From 594f0dea8435f20c72431023c89fe0f37b82f8b1 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 23 Jun 2026 06:14:59 -0700 Subject: [PATCH 1/2] fix: make repo-root absolute path Keeping the repo-root path relative breaks parsing of absolute paths in clang-tidy output. This didn't surface in the tests because they all use a compilation database generated for ninja (which uses relative paths). But in the test repo, we are using CMake which generates absolute paths in the compilation database. Remember, clang-tidy outputs filenames however they are stated in the compilation database. If not using a compilation database, then clang-tidy just outputs the filename as it was given in the CLI. --- I also removed an artifact debug log from developing solution in #386 --- cpp-linter/src/clang_tools/clang_tidy.rs | 4 ---- cpp-linter/src/common_fs.rs | 23 +++++++++++++++++++++++ cpp-linter/src/git.rs | 9 ++------- cpp-linter/src/run.rs | 13 ++++++++++--- 4 files changed, 35 insertions(+), 14 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index 4d97816..5239ff3 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -395,10 +395,6 @@ pub fn run_clang_tidy( &clang_params.database_json, &clang_params.repo_root, )?; - logs.push(( - log::Level::Debug, - format!("Parsed {} clang-tidy notifications", notes.len()), - )); let tidy_advice = TidyAdvice { notes }; file.patched_path = Some(cache_patch_path.to_path_buf()); diff --git a/cpp-linter/src/common_fs.rs b/cpp-linter/src/common_fs.rs index 8e148ba..911cea9 100644 --- a/cpp-linter/src/common_fs.rs +++ b/cpp-linter/src/common_fs.rs @@ -383,6 +383,20 @@ impl FileObj { } } +/// Make the given `path` absolute. +/// +/// This function will canonicalize the path and remove any `\\?\` prefix +/// returned by Windows API, which is really only designed for other Windows API. +/// +/// This function can error if the given path fails to be canonicalized. +/// This may happen if the path does not exist or a non-final path +/// component is not a directory. +pub(crate) fn mk_path_abs>(path: P) -> Result { + let abs_path = path.as_ref().canonicalize()?; + let abs_path_str = abs_path.to_string_lossy(); + Ok(PathBuf::from(abs_path_str.trim_start_matches(r"\\?\"))) +} + #[cfg(test)] mod test { use std::path::PathBuf; @@ -438,4 +452,13 @@ mod test { let file_obj = FileObj::new(PathBuf::from("tests/demo/demo.cpp")); assert!(!file_obj.is_line_in_diff(&42)); } + + #[test] + fn canonical_path() { + let file_obj = FileObj::new(PathBuf::from("tests/demo/demo.cpp")); + let canonical_path = super::mk_path_abs(&file_obj.name).unwrap(); + assert!(canonical_path.is_file()); + println!("Canonical path: {}", canonical_path.display()); + assert!(!canonical_path.to_string_lossy().starts_with(r"\\?\")); + } } diff --git a/cpp-linter/src/git.rs b/cpp-linter/src/git.rs index 2407a18..e38de77 100644 --- a/cpp-linter/src/git.rs +++ b/cpp-linter/src/git.rs @@ -10,7 +10,6 @@ mod test { use std::{ env::{self, current_dir, set_current_dir}, - fs, process::Command, }; @@ -41,12 +40,8 @@ mod test { } } if let Some(patch) = patch_path { - let canonical_patch_path = fs::canonicalize(patch).unwrap(); - let patch_path = canonical_patch_path - .to_str() - .unwrap() - // on Windows, canonical paths can have a prefix of "\\?\" which git does not recognize - .trim_start_matches("\\\\?\\"); + let canonical_patch_path = crate::common_fs::mk_path_abs(patch).unwrap(); + let patch_path = canonical_patch_path.to_str().unwrap(); let ok = Command::new("git") .args(["apply", "--index", patch_path]) .current_dir(path) diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index ac60573..835bcff 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -20,7 +20,7 @@ use log::{LevelFilter, set_max_level}; use crate::{ clang_tools::capture_clang_tools_output, cli::{ClangParams, Cli, CliCommand, FeedbackInput, LinesChangedOnly}, - common_fs::FileObj, + common_fs::{FileObj, mk_path_abs}, rest_client::RestClient, }; use git_bot_feedback::FileFilter; @@ -47,7 +47,7 @@ const VERSION: &str = env!("CARGO_PKG_VERSION"); /// alias ("path/to/cpp-linter.exe"). Thus, the parser in [`crate::cli`] will halt on an error /// because it is not configured to handle positional arguments. pub async fn run_main(args: Vec) -> Result<()> { - let cli = Cli::parse_from(args); + let mut cli = Cli::parse_from(args); if matches!(cli.commands, Some(CliCommand::Version)) || cli.general_options.version == RequestedVersion::NoValue @@ -81,7 +81,14 @@ pub async fn run_main(args: Vec) -> Result<()> { .collect::>(), None, ); - let gitmodules = cli.source_options.repo_root.join(".gitmodules"); + let repo_root_abs = mk_path_abs(&cli.source_options.repo_root).with_context(|| { + format!( + "Failed to canonicalize the repo root path: {}", + cli.source_options.repo_root.to_string_lossy() + ) + })?; + let gitmodules = repo_root_abs.join(".gitmodules"); + cli.source_options.repo_root = repo_root_abs; file_filter.parse_submodules(Some(gitmodules.as_path())); if let Some(files) = &cli.not_ignored { file_filter.not_ignored.extend(files.clone()); From caf7b2ffedf2c0101d981155daceb87a128f48b4 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 23 Jun 2026 14:42:43 -0700 Subject: [PATCH 2/2] fix lint and add test for coverage --- cpp-linter/src/common_fs.rs | 1 + cpp-linter/src/run.rs | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/cpp-linter/src/common_fs.rs b/cpp-linter/src/common_fs.rs index 911cea9..de8c73d 100644 --- a/cpp-linter/src/common_fs.rs +++ b/cpp-linter/src/common_fs.rs @@ -399,6 +399,7 @@ pub(crate) fn mk_path_abs>(path: P) -> Result