Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions cpp-linter/src/clang_tools/clang_tidy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
24 changes: 24 additions & 0 deletions cpp-linter/src/common_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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"\\?\"));
}
}
9 changes: 2 additions & 7 deletions cpp-linter/src/git.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ mod test {

use std::{
env::{self, current_dir, set_current_dir},
fs,
process::Command,
};

Expand Down Expand Up @@ -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)
Expand Down
28 changes: 23 additions & 5 deletions cpp-linter/src/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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(),
])
Expand All @@ -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(),
])
Expand All @@ -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);
}
}
Loading