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..de8c73d 100644 --- a/cpp-linter/src/common_fs.rs +++ b/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>(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 { + #![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"\\?\")); + } } 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..5935c61 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()); @@ -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); + } }