cpp_linter/clang_tools/

clang_tidy.rs

//! This module holds functionality specific to running clang-tidy and parsing it's
//! output.

use std::{
    env::consts::OS,
    fs,
    path::{Path, PathBuf},
    process::Command,
    sync::{Arc, Mutex, MutexGuard},
};

// non-std crates
use clang_tools_manager::utils::normalize_path;
use regex::Regex;
use serde::Deserialize;

// project-specific modules/crates
use crate::{cli::ClangParams, common_fs::FileObj, error::ClangCaptureError};

/// Used to deserialize a json compilation database's translation unit.
///
/// The only purpose this serves is to normalize relative paths for build systems that
/// use/need relative paths (ie ninja).
#[derive(Deserialize, Debug, Clone)]
pub struct CompilationUnit {
    /// The directory of the build environment
    directory: String,

    /// The file path of the translation unit.
    ///
    /// Sometimes, this is relative to the build [`CompilationUnit::directory`].
    ///
    /// This is typically the path that clang-tidy uses in its stdout (for a dry run).
    /// So, having this information helps with matching clang-tidy's stdout with the
    /// repository files.
    file: String,
}

/// A structure that represents a single notification parsed from clang-tidy's stdout.
#[derive(Debug, Clone)]
pub struct TidyNotification {
    /// The file's path and name (supposedly relative to the repository root folder).
    pub filename: String,

    /// The line number from which the notification originated.
    pub line: u32,

    /// The column offset on the line from which the notification originated.
    pub cols: u32,

    /// The severity (ie error/warning/note) of the [`TidyNotification::diagnostic`]
    /// that caused the notification.
    pub severity: String,

    /// A helpful message explaining why the notification exists.
    pub rationale: String,

    /// The diagnostic name as used when configuring clang-tidy.
    pub diagnostic: String,

    /// A code block that points directly to the origin of the notification.
    ///
    /// Sometimes, this code block doesn't exist. Sometimes, it contains suggested
    /// fixes/advice. This information is purely superfluous.
    pub suggestion: Vec<String>,

    /// The list of line numbers that had fixes applied via `clang-tidy --fix-error`.
    pub fixed_lines: Vec<u32>,
}

impl TidyNotification {
    /// Get a markdown-formatted link to the clang-tidy documentation page for [`Self::diagnostic`].
    pub fn diagnostic_link(&self) -> String {
        if self.diagnostic.starts_with("clang-diagnostic-") {
            // clang-diagnostic-* diagnostics are compiler diagnostics and don't have
            // dedicated clang-tidy documentation pages, so return the name as-is.
            return self.diagnostic.clone();
        }
        if let Some((category, name)) = if self.diagnostic.starts_with("clang-analyzer-") {
            self.diagnostic
                .strip_prefix("clang-analyzer-")
                .map(|n| ("clang-analyzer", n))
        } else {
            self.diagnostic.split_once('-')
        } {
            // In production, both category and name should be non-empty strings.
            // Clang does not actually have a diagnostic name whose category or name is empty.
            debug_assert!(!category.is_empty() && !name.is_empty());
            format!(
                "[{}](https://clang.llvm.org/extra/clang-tidy/checks/{category}/{name}.html)",
                self.diagnostic
            )
        } else {
            self.diagnostic.clone()
        }
    }
}

/// A struct to hold notification from clang-tidy about a single file
#[derive(Debug, Clone)]
pub struct TidyAdvice {
    /// A list of notifications parsed from clang-tidy stdout.
    pub notes: Vec<TidyNotification>,
}

impl TidyAdvice {
    pub(crate) fn get_suggestion_help(&self, start_line: u32, end_line: u32) -> String {
        let mut diagnostics = vec![];
        for note in &self.notes {
            for fixed_line in &note.fixed_lines {
                if (start_line..=end_line).contains(fixed_line) {
                    diagnostics.push(format!(
                        "- {} [{}]\n",
                        note.rationale,
                        note.diagnostic_link()
                    ));
                }
            }
        }
        format!(
            "### clang-tidy {}\n{}",
            if diagnostics.is_empty() {
                "suggestion"
            } else {
                "diagnostic(s)"
            },
            diagnostics.join("")
        )
    }
}

/// A regex pattern to capture the clang-tidy note header.
const NOTE_HEADER: &str = r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+),?[^\]]*\]$";

/// Parses clang-tidy stdout.
///
/// Here it helps to have the JSON database deserialized for normalizing paths present
/// in the notifications.
fn parse_tidy_output(
    tidy_stdout: &[u8],
    database_json: &Option<Vec<CompilationUnit>>,
    repo_root: &Path,
) -> Result<Vec<TidyNotification>, ClangCaptureError> {
    let note_header = Regex::new(NOTE_HEADER)?;
    let fixed_note = Regex::new(r"^.+:(\d+):\d+:\snote: FIX-IT applied suggested code changes$")?;
    let mut found_fix = false;
    let mut notification = None;
    let mut result = Vec::new();
    for line in String::from_utf8(tidy_stdout.to_vec())
        .map_err(|e| ClangCaptureError::NonUtf8Output {
            task: "convert clang-tidy stdout".to_string(),
            source: e,
        })?
        .lines()
    {
        if let Some(captured) = note_header.captures(line) {
            // First check that the diagnostic name is a actual diagnostic name.
            // Sometimes clang-tidy uses square brackets to enclose additional context
            // about the diagnostic rationale. For example: '[with auto = typename ...]'
            // We need to ignore such cases as they do not start a diagnostic report.
            if captured
                .get(6)
                .is_some_and(|diag| !diag.as_str().contains(' ') && diag.as_str().contains('-'))
            {
                // starting a new TidyNotification; store the previous one (if any) to results
                if let Some(note) = notification {
                    result.push(note);
                }

                // normalize the filename path and try to make it relative to the repo root
                let mut filename = PathBuf::from(&captured[1]);
                // if database was given try to use that first
                if let Some(db_json) = &database_json {
                    let mut found_unit = false;
                    for unit in db_json {
                        let unit_path =
                            PathBuf::from_iter([unit.directory.as_str(), unit.file.as_str()]);
                        if unit_path == filename {
                            filename =
                                normalize_path(&PathBuf::from_iter([&unit.directory, &unit.file]));
                            found_unit = true;
                            break;
                        }
                    }
                    if !found_unit {
                        // file was not a named unit in the database;
                        // try to normalize path as if relative to working directory.
                        // NOTE: This shouldn't happen with a properly formed JSON database
                        filename = normalize_path(&PathBuf::from_iter([repo_root, &filename]));
                    }
                } else {
                    // still need to normalize the relative path despite missing database info.
                    // let's assume the file is relative to current working directory.
                    filename = normalize_path(&PathBuf::from_iter([repo_root, &filename]));
                }

                if filename.is_absolute()
                    && let Ok(file_n) = filename.strip_prefix(repo_root)
                {
                    // if this filename can't be made into a relative path, then it is
                    // likely not a member of the project's sources (ie /usr/include/stdio.h)
                    filename = file_n.to_path_buf();
                }

                notification = Some(TidyNotification {
                    filename: filename.to_string_lossy().to_string().replace('\\', "/"),
                    line: captured[2].parse()?,
                    cols: captured[3].parse()?,
                    severity: String::from(&captured[4]),
                    rationale: String::from(&captured[5]).trim().to_string(),
                    diagnostic: String::from(&captured[6]),
                    suggestion: Vec::new(),
                    fixed_lines: Vec::new(),
                });
                // begin capturing subsequent lines as suggestions
                found_fix = false;
            }
        } else if let Some(capture) = fixed_note.captures(line) {
            let fixed_line = capture[1].parse()?;
            if let Some(note) = &mut notification
                && !note.fixed_lines.contains(&fixed_line)
            {
                note.fixed_lines.push(fixed_line);
            }
            // Suspend capturing subsequent lines as suggestions until
            // a new notification is constructed. If we found a note about applied fixes,
            // then the lines of suggestions for that notification have already been parsed.
            found_fix = true;
        } else if !found_fix && let Some(note) = &mut notification {
            // append lines of code that are part of
            // the previous line's notification
            note.suggestion.push(line.to_string());
        }
    }
    if let Some(note) = notification {
        result.push(note);
    }
    Ok(result)
}

/// Get a total count of clang-tidy advice from the given list of [FileObj]s.
pub fn tally_tidy_advice(files: &[Arc<Mutex<FileObj>>]) -> Result<u64, String> {
    let mut total = 0;
    for file in files {
        let file = file.lock().map_err(|e| e.to_string())?;
        if let Some(advice) = &file.tidy_advice {
            for tidy_note in &advice.notes {
                let file_path = PathBuf::from(&tidy_note.filename);
                if file_path == file.name {
                    total += 1;
                }
            }
        }
    }
    Ok(total)
}

/// RAII guard that restores a file's original content on drop.
///
/// This is a best-effort when an error gets propagated before
/// we can explicitly restore a file's contents.
struct RestoreOnDrop<'a> {
    path: &'a Path,
    content: String,
    // lock:
    armed: bool,
}

impl Drop for RestoreOnDrop<'_> {
    fn drop(&mut self) {
        if self.armed {
            // We have to ignore any error here and hope for the best.
            let _ = fs::write(self.path, &self.content);
        }
    }
}

/// Run clang-tidy, then parse and return it's output.
pub fn run_clang_tidy(
    file: &mut MutexGuard<FileObj>,
    clang_params: &ClangParams,
) -> Result<Vec<(log::Level, String)>, ClangCaptureError> {
    let cmd_path = clang_params
        .clang_tidy_command
        .as_ref()
        .ok_or(ClangCaptureError::ToolPathUnknown("clang-tidy"))?;
    let mut cmd = Command::new(cmd_path);
    cmd.current_dir(&clang_params.repo_root);
    let mut logs = vec![];
    if !clang_params.tidy_checks.is_empty() {
        cmd.args(["-checks", &clang_params.tidy_checks]);
    }
    if let Some(db) = &clang_params.database {
        cmd.args(["-p", &db.to_string_lossy()]);
    }
    for arg in &clang_params.extra_args {
        cmd.args(["--extra-arg", format!("\"{}\"", arg).as_str()]);
    }
    let file_name = file.name.to_string_lossy().to_string();
    let ranges = file.get_ranges(&clang_params.lines_changed_only);
    if !ranges.is_empty() {
        let filter = format!(
            "[{{\"name\":{:?},\"lines\":{:?}}}]",
            &file_name.replace('/', if OS == "windows" { "\\" } else { "/" }),
            ranges
                .iter()
                .map(|r| [r.start(), r.end()])
                .collect::<Vec<_>>()
        );
        cmd.args(["--line-filter", filter.as_str()]);
    }
    let repo_file_path = clang_params.repo_root.join(&file.name);
    cmd.arg("--fix-errors");
    if !clang_params.style.is_empty() {
        cmd.args(["--format-style", clang_params.style.as_str()]);
    }
    cmd.arg(file.name.to_string_lossy().as_ref());
    logs.push((
        log::Level::Info,
        format!(
            "Running \"{} {}\"",
            cmd.get_program().to_string_lossy(),
            cmd.get_args()
                .map(|x| x.to_string_lossy())
                .collect::<Vec<_>>()
                .join(" ")
        ),
    ));
    let cache_path = clang_params.get_cache_path();
    let cache_patch_path = cache_path.join(&file.name);
    fs::create_dir_all(
        cache_patch_path
            .parent()
            .ok_or(ClangCaptureError::UnknownCacheParentPath)?,
    )
    .map_err(ClangCaptureError::MkDirFailed)?;
    let mut drop_guard = RestoreOnDrop {
        content: fs::read_to_string(&repo_file_path).map_err(|e| {
            ClangCaptureError::ReadFileFailed {
                file_name: file_name.clone(),
                source: e,
            }
        })?,
        armed: true,
        path: repo_file_path.as_path(),
    };
    // run clang-tidy to patch the file in-place.
    let output = cmd
        .output()
        .map_err(|e| ClangCaptureError::FailedToRunCommand {
            task: format!("execute clang-tidy on file {file_name}"),
            source: e,
        })?;
    // move the patched file content into cache
    fs::copy(&repo_file_path, &cache_patch_path).map_err(|e| {
        ClangCaptureError::WriteFileFailed {
            file_name: cache_patch_path.to_string_lossy().to_string(),
            source: e,
        }
    })?;
    // restore the original file content to avoid unexpected behavior in later steps of dev workflow
    fs::write(&repo_file_path, &drop_guard.content).map_err(|e| {
        ClangCaptureError::WriteFileFailed {
            file_name: file_name.clone(),
            source: e,
        }
    })?;
    // disarm the drop guard since we've already restored the original content
    drop_guard.armed = false;

    logs.push((
        log::Level::Debug,
        format!(
            "Output from clang-tidy:\n{}",
            String::from_utf8_lossy(&output.stdout)
        ),
    ));
    if !output.stderr.is_empty() {
        logs.push((
            log::Level::Debug,
            format!(
                "clang-tidy made the following summary:\n{}",
                String::from_utf8_lossy(&output.stderr)
            ),
        ));
    }

    let notes = parse_tidy_output(
        &output.stdout,
        &clang_params.database_json,
        &clang_params.repo_root,
    )?;

    let tidy_advice = TidyAdvice { notes };
    file.patched_path = Some(cache_patch_path.to_path_buf());
    file.tidy_advice = Some(tidy_advice);
    Ok(logs)
}

#[cfg(test)]
mod test {
    #![allow(clippy::unwrap_used, clippy::expect_used)]

    use std::{
        env, fs,
        path::PathBuf,
        str::FromStr,
        sync::{Arc, Mutex},
    };

    use clang_tools_manager::RequestedVersion;
    use regex::Regex;

    use crate::{
        clang_tools::{ClangTool, clang_tidy::parse_tidy_output},
        cli::{ClangParams, LinesChangedOnly},
        common_fs::FileObj,
    };

    use super::{NOTE_HEADER, RestoreOnDrop, TidyNotification, run_clang_tidy};

    #[test]
    fn clang_diagnostic_link() {
        let note = TidyNotification {
            filename: String::from("some_src.cpp"),
            line: 1504,
            cols: 9,
            rationale: String::from("file not found"),
            severity: String::from("error"),
            diagnostic: String::from("clang-diagnostic-error"),
            suggestion: vec![],
            fixed_lines: vec![],
        };
        assert_eq!(note.diagnostic_link(), note.diagnostic);
    }

    #[test]
    fn clang_analyzer_link() {
        let note = TidyNotification {
            filename: String::from("some_src.cpp"),
            line: 1504,
            cols: 9,
            rationale: String::from(
                "Dereference of null pointer (loaded from variable 'pipe_num')",
            ),
            severity: String::from("warning"),
            diagnostic: String::from("clang-analyzer-core.NullDereference"),
            suggestion: vec![],
            fixed_lines: vec![],
        };
        let expected = format!(
            "[{}](https://clang.llvm.org/extra/clang-tidy/checks/{}/{}.html)",
            note.diagnostic, "clang-analyzer", "core.NullDereference",
        );
        assert_eq!(note.diagnostic_link(), expected);
    }

    #[test]
    fn invalid_diagnostic_link() {
        let expected = "no_diagnostic_name".to_string();
        let note = TidyNotification {
            filename: String::from("some_src.cpp"),
            line: 1504,
            cols: 9,
            rationale: String::from("some rationale"),
            severity: String::from("warning"),
            diagnostic: expected.clone(),
            suggestion: vec![],
            fixed_lines: vec![],
        };
        assert_eq!(note.diagnostic_link(), expected);
    }

    // ***************** test for regex parsing of clang-tidy stdout

    #[test]
    fn test_capture() {
        let src = "tests/demo/demo.hpp:11:11: \
        warning: use a trailing return type for this function \
        [modernize-use-trailing-return-type,-warnings-as-errors]";
        let pat = Regex::new(NOTE_HEADER).unwrap();
        let cap = pat.captures(src).unwrap();
        assert_eq!(
            cap.get(0).unwrap().as_str(),
            format!(
                "{}:{}:{}: {}:{}[{},-warnings-as-errors]",
                cap.get(1).unwrap().as_str(),
                cap.get(2).unwrap().as_str(),
                cap.get(3).unwrap().as_str(),
                cap.get(4).unwrap().as_str(),
                cap.get(5).unwrap().as_str(),
                cap.get(6).unwrap().as_str()
            )
            .as_str()
        )
    }

    #[test]
    fn use_extra_args() {
        let exe_path = ClangTool::ClangTidy
            .get_exe_path(
                &RequestedVersion::from_str(
                    env::var("CLANG_VERSION").unwrap_or("".to_string()).as_str(),
                )
                .unwrap(),
            )
            .unwrap();
        let tmp_workspace = crate::test_common::setup_tmp_workspace();
        let file = FileObj::new(PathBuf::from("demo/demo.cpp"));
        let arc_file = Arc::new(Mutex::new(file));
        let extra_args = vec!["-std=c++17".to_string(), "-Wall".to_string()];
        let clang_params = ClangParams {
            style: "".to_string(),
            tidy_checks: "".to_string(), // use .clang-tidy config file
            lines_changed_only: LinesChangedOnly::Off,
            database: None,
            extra_args: extra_args.clone(), // <---- the reason for this test
            database_json: None,
            format_filter: None,
            tidy_filter: None,
            clang_tidy_command: Some(exe_path),
            clang_format_command: None,
            repo_root: tmp_workspace.path().to_path_buf(),
        };
        let mut file_lock = arc_file.lock().unwrap();
        fs::create_dir_all(clang_params.get_cache_path()).unwrap();
        let logs = run_clang_tidy(&mut file_lock, &clang_params)
            .unwrap()
            .into_iter()
            .filter_map(|(_lvl, msg)| {
                if msg.contains("Running ") {
                    Some(msg)
                } else {
                    None
                }
            })
            .collect::<Vec<String>>();
        let args = &logs
            .first()
            .expect("expected a log message about invoked clang-tidy command")
            .split(' ')
            .collect::<Vec<&str>>();
        for arg in &extra_args {
            let extra_arg = format!("\"{arg}\"");
            assert!(args.contains(&extra_arg.as_str()));
        }
    }

    #[test]
    fn skip_parse_tidy_diagnostic_rationale() {
        let tidy_out = r#"
TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:46:19: error: use of undeclared identifier 'readFreeImageTexture' [clang-diagnostic-error]
   46 |            return readFreeImageTexture(reader);
      |                   ^
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:659:32: note: in instantiation of function template specialization 'tb::mdl::(anonymous namespace)::loadTexture(const std::string &)::(anonymous class)::operator()<std::shared_ptr<tb::fs::File>>' requested here
  659 |     using Fn_Result = decltype(f(std::declval<Value&&>()));
      |                                ^
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:2949:29: note: in instantiation of function template specialization 'kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>::and_then<(lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
 2949 |   return std::forward<R>(r).and_then(t.and_then);
      |                             ^
TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:32: note: in instantiation of function template specialization 'kdl::detail::operator|<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
      |                                ^
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:661:19: error: static assertion failed due to requirement 'is_result_v<int>': Function must return a result type [clang-diagnostic-error]
  661 |     static_assert(is_result_v<Fn_Result>, "Function must return a result type");
      |                   ^~~~~~~~~~~~~~~~~~~~~~
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:2949:29: note: in instantiation of function template specialization 'kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>::and_then<(lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
 2949 |   return std::forward<R>(r).and_then(t.and_then);
      |                             ^
TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:32: note: in instantiation of function template specialization 'kdl::detail::operator|<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
      |                                ^
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:663:77: error: no type named 'type' in 'kdl::detail::chain_results<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, int>' [clang-diagnostic-error]
  663 |     using Cm_Result = typename detail::chain_results<My_Result, Fn_Result>::type;
      |                       ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:667:48: error: no matching function for call to object of type 'const (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)' [clang-diagnostic-error]
  667 |         [&](value_type&& v) { return Cm_Result{f(std::move(v))}; },
      |                                                ^
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:667:29: note: while substituting into a lambda expression here
  667 |         [&](value_type&& v) { return Cm_Result{f(std::move(v))}; },
      |                             ^
TrenchBroom/TrenchBroom/lib/KdLib/include/kd/result.h:2949:29: note: in instantiation of function template specialization 'kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>::and_then<(lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
 2949 |   return std::forward<R>(r).and_then(t.and_then);
      |                             ^
TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:32: note: in instantiation of function template specialization 'kdl::detail::operator|<kdl::result<std::shared_ptr<tb::fs::File>, kdl::result_error>, (lambda at TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48)>' requested here
   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
      |                                ^
TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48: note: candidate template ignored: substitution failure [with file:auto = typename std::remove_reference<shared_ptr<File> &>::type]
   44 |   return diskFS.openFile(name) | kdl::and_then([](const auto& file) {
      |                                                ^
"#;
        let advice = parse_tidy_output(tidy_out.as_bytes(), &None, &PathBuf::from(".")).unwrap();
        assert_eq!(advice.len(), 4);
        for note in advice {
            assert!(note.diagnostic.contains('-'));
            assert!(!note.diagnostic.contains(' '));
        }
    }

    #[test]
    fn restore_on_drop_fires() {
        let tmp = tempfile::tempdir().unwrap();
        let file_path = tmp.path().join("test_file.txt");
        let original = "original content";
        fs::write(&file_path, original).unwrap();

        {
            let guard = RestoreOnDrop {
                path: &file_path,
                content: original.to_string(),
                armed: true,
            };
            // Simulate clang-tidy mutating the file
            fs::write(&file_path, "patched content").unwrap();
            // Explicit drop triggers restoration
            drop(guard);
        }

        assert_eq!(fs::read_to_string(&file_path).unwrap(), original);
    }
}