fix: make repo-root path absolute (#387)

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 the solution for
#386

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Improvements**
* Enhanced error handling for repository root paths with improved error
messages when invalid paths are provided.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

Author: 2bndy5

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());

cpp-linter/src/common_fs.rs

@@ -383,8 +383,23 @@ 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<P: AsRef<Path>>(path: P) -> Result<PathBuf, std::io::Error> {
+    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 {
+    #![allow(clippy::unwrap_used)]
     use std::path::PathBuf;
 
     use super::FileObj;
@@ -438,4 +453,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"\\?\"));
+    }
 }

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)

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<String>) -> 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<String>) -> Result<()> {
             .collect::<Vec<&str>>(),
         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());
@@ -274,7 +281,6 @@ pub(crate) mod test {
             "-l".to_string(),
             "false".to_string(),
             "-V".to_string(),
-            "-i=target|benches/libgit2".to_string(),
             "--repo-root".to_string(),
             tmp_workspace.path().to_str().unwrap().to_string(),
         ])
@@ -295,7 +301,6 @@ pub(crate) mod test {
             "--lines-changed-only".to_string(),
             "false".to_string(),
             "-v".to_string(),
-            "--ignore=target|benches/libgit2".to_string(),
             "--repo-root".to_string(),
             tmp_workspace.path().to_str().unwrap().to_string(),
         ])
@@ -321,4 +326,17 @@ pub(crate) mod test {
         .unwrap();
         drop(tmp_gh_out);
     }
+
+    #[tokio::test]
+    async fn bad_repo_root() {
+        let tmp_gh_out = setup_tmp_gh_out_path();
+        run_main(vec![
+            "cpp-linter".to_string(),
+            "--repo-root".to_string(),
+            "non-existent_path".to_string(),
+        ])
+        .await
+        .unwrap_err();
+        drop(tmp_gh_out);
+    }
 }