From 3bccf51eb54edd476009502f0373cdf0853b5783 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Wed, 10 Jun 2026 05:38:33 -0700 Subject: [PATCH 01/13] fix: do not change directory to `repo-root` ref: - cpp-linter/cpp-linter#144 - cpp-linter/cpp-linter#146 Let cpp-linter stay in working directory. Instead of changing into the given `--repo-root`, this patch uses the value as a scope of file system focus. - All clang tools are invoked in the given `--repo-root`. - All discovered files are stripped of `--repo-root` - Any local caching done is prefixed with `--repo-root` value. `--repo-root` value has always been expected sa a relative path, but this should also work if given an absolute path too. --- cpp-linter/src/clang_tools/clang_format.rs | 13 +++--- cpp-linter/src/clang_tools/clang_tidy.rs | 3 +- cpp-linter/src/clang_tools/mod.rs | 2 + cpp-linter/src/cli/structs.rs | 5 +-- cpp-linter/src/git.rs | 1 + cpp-linter/src/rest_client/mod.rs | 7 +++- cpp-linter/src/run.rs | 45 ++++++++------------- cpp-linter/tests/paginated_changed_files.rs | 8 +++- 8 files changed, 45 insertions(+), 39 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs index 0ef78100..90f84fd2 100644 --- a/cpp-linter/src/clang_tools/clang_format.rs +++ b/cpp-linter/src/clang_tools/clang_format.rs @@ -13,7 +13,7 @@ use gix_imara_diff::{Diff, InternedInput}; use log::Level; // project-specific crates/modules -use super::MakeSuggestions; +use super::{CACHE_DIR, MakeSuggestions}; use crate::{cli::ClangParams, common_fs::FileObj, error::ClangCaptureError}; #[derive(Debug, Clone, PartialEq, Eq, Default)] @@ -73,13 +73,14 @@ pub fn run_clang_format( .as_ref() .ok_or(ClangCaptureError::ToolPathUnknown("clang-format"))?; let mut cmd = Command::new(cmd_path); + cmd.current_dir(&clang_params.repo_root); let mut logs = vec![]; cmd.args(["--style", &clang_params.style]); let ranges = file.get_ranges(&clang_params.lines_changed_only); for range in &ranges { cmd.arg(format!("--lines={}:{}", range.start(), range.end())); } - let cache_path = clang_params.project_cache_dir.join("patches"); + let cache_path = clang_params.repo_root.join(CACHE_DIR).join("patches"); let cache_format_fixes = cache_path.join(file.name.with_added_extension("format")); fs::create_dir_all( cache_format_fixes @@ -125,9 +126,11 @@ pub fn run_clang_format( // use a diff between patched and original contents to get format results let original_contents = - fs::read_to_string(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed { - file_name: file_name.clone(), - source: e, + fs::read_to_string(clang_params.repo_root.join(&file.name)).map_err(|e| { + ClangCaptureError::ReadFileFailed { + file_name: file_name.clone(), + source: e, + } })?; let patched_contents = String::from_utf8(output.stdout.to_vec()).map_err(|e| { ClangCaptureError::NonUtf8Output { diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index 111767e0..1fdb022b 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -273,6 +273,7 @@ pub fn run_clang_tidy( .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]); @@ -496,7 +497,7 @@ mod test { format_review: false, clang_tidy_command: Some(exe_path), clang_format_command: None, - project_cache_dir: PathBuf::from(".").join(".cpp-linter-cache"), + repo_root: PathBuf::from(".").join(".cpp-linter-cache"), }; let mut file_lock = arc_file.lock().unwrap(); let logs = run_clang_tidy(&mut file_lock, &clang_params) diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index 62127d1b..bac0e0c4 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -27,6 +27,8 @@ use clang_format::run_clang_format; pub mod clang_tidy; use clang_tidy::{CompilationUnit, run_clang_tidy}; +pub const CACHE_DIR: &str = ".cpp-linter-cache"; + /// This creates a task to run clang-tidy and clang-format on a single file. /// /// Returns a Future that infallibly resolves to a 2-tuple that contains diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index 48e473f9..4ed17016 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -178,7 +178,7 @@ pub struct ClangParams { pub format_filter: Option, pub tidy_review: bool, pub format_review: bool, - pub project_cache_dir: PathBuf, + pub repo_root: PathBuf, } #[cfg(feature = "bin")] @@ -216,8 +216,7 @@ impl From<&Cli> for ClangParams { format_filter, tidy_review: args.feedback_options.tidy_review, format_review: args.feedback_options.format_review, - project_cache_dir: PathBuf::from(&args.source_options.repo_root) - .join(".cpp-linter-cache"), + repo_root: PathBuf::from(&args.source_options.repo_root), } } } diff --git a/cpp-linter/src/git.rs b/cpp-linter/src/git.rs index a77cf7d0..309a2b49 100644 --- a/cpp-linter/src/git.rs +++ b/cpp-linter/src/git.rs @@ -101,6 +101,7 @@ mod test { &LinesChangedOnly::Off.into(), &base_diff, ignore_staged, + ".", ) .await .unwrap() diff --git a/cpp-linter/src/rest_client/mod.rs b/cpp-linter/src/rest_client/mod.rs index cec9af4e..8b1344be 100644 --- a/cpp-linter/src/rest_client/mod.rs +++ b/cpp-linter/src/rest_client/mod.rs @@ -54,6 +54,7 @@ impl RestClient { lines_changed_only: &LinesChangedOnly, base_diff: &Option, ignore_index: bool, + repo_root: &str, ) -> Result, ClientError> { let files = self .client @@ -72,8 +73,12 @@ impl RestClient { .iter() .map(|hunk| hunk.start..=hunk.end) .collect(); + let file_path = PathBuf::from(file_name); FileObj::from( - PathBuf::from(&file_name), + file_path + .strip_prefix(repo_root) + .map(PathBuf::from) + .unwrap_or(file_path), diff_lines.added_lines.clone(), diff_chunks, ) diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index 9746a74f..63f2dc50 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -6,7 +6,7 @@ #![cfg(feature = "bin")] use std::{ env, - path::{Path, PathBuf}, + path::PathBuf, sync::{Arc, Mutex}, }; @@ -18,7 +18,7 @@ use log::{LevelFilter, set_max_level}; // project specific modules/crates use crate::{ - clang_tools::capture_clang_tools_output, + clang_tools::{CACHE_DIR, capture_clang_tools_output}, cli::{ClangParams, Cli, CliCommand, FeedbackInput, LinesChangedOnly}, common_fs::FileObj, logger, @@ -59,15 +59,6 @@ pub async fn run_main(args: Vec) -> Result<()> { logger::try_init(); - if cli.source_options.repo_root != "." { - env::set_current_dir(Path::new(&cli.source_options.repo_root)).map_err(|e| { - anyhow!( - "'{}' is inaccessible or does not exist: {e:?}", - cli.source_options.repo_root - ) - })?; - } - let mut rest_api_client = RestClient::new()?; set_max_level( if cli.general_options.verbosity.is_debug() || rest_api_client.is_debug_enabled() { @@ -120,14 +111,23 @@ pub async fn run_main(args: Vec) -> Result<()> { &cli.source_options.lines_changed_only.clone().into(), &cli.source_options.diff_base, cli.source_options.ignore_index, + &cli.source_options.repo_root, ) .await? } else { // walk the folder and look for files with specified extensions according to ignore values. let mut all_files: Vec = file_filter - .walk_dir(".")? + .walk_dir(&cli.source_options.repo_root)? .into_iter() - .map(|file_name| FileObj::new(PathBuf::from(&file_name))) + .map(|file_name| { + let file_path = PathBuf::from(&file_name); + FileObj::new( + file_path + .strip_prefix(&cli.source_options.repo_root) + .map(PathBuf::from) + .unwrap_or(file_path), + ) + }) .collect(); if is_pr && (cli.feedback_options.tidy_review || cli.feedback_options.format_review) { let changed_files = rest_api_client @@ -136,6 +136,7 @@ pub async fn run_main(args: Vec) -> Result<()> { &LinesChangedOnly::Off.into(), &cli.source_options.diff_base, cli.source_options.ignore_index, + &cli.source_options.repo_root, ) .await?; for changed_file in changed_files { @@ -160,11 +161,12 @@ pub async fn run_main(args: Vec) -> Result<()> { let mut clang_params = ClangParams::from(&cli); // mkdir -p .cpp-linter-cache/ - std::fs::create_dir_all(&clang_params.project_cache_dir) + let cache_dir = clang_params.repo_root.join(CACHE_DIR); + std::fs::create_dir_all(&cache_dir) .with_context(|| "Failed to create a local cache directory.")?; // add gitignore file in project cache dir std::fs::write( - clang_params.project_cache_dir.join(".gitignore"), + cache_dir.join(".gitignore"), "# Automatically created by cpp-linter\n*\n", ) .with_context(|| "Failed to write .cpp-linter-cache/.gitignore file")?; @@ -296,17 +298,4 @@ 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(), - "some-non-existent-dir".to_string(), - ]) - .await - .unwrap_err(); - drop(tmp_gh_out); - } } diff --git a/cpp-linter/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs index f8b57001..464ccb0e 100644 --- a/cpp-linter/tests/paginated_changed_files.rs +++ b/cpp-linter/tests/paginated_changed_files.rs @@ -140,7 +140,13 @@ async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) { let file_filter = FileFilter::new(&[], &["cpp", "hpp"], None); let files = client - .get_list_of_changed_files(&file_filter, &LinesChangedOnly::Off, &None::, false) + .get_list_of_changed_files( + &file_filter, + &LinesChangedOnly::Off, + &None::, + false, + ".", + ) .await; match files { Err(e) => { From 9fcf74db43f841ea86d678ad13ca9b57a14be9ee Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Wed, 10 Jun 2026 05:45:53 -0700 Subject: [PATCH 02/13] missed a test --- cpp-linter/src/clang_tools/clang_tidy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index 1fdb022b..c6049490 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -497,7 +497,7 @@ mod test { format_review: false, clang_tidy_command: Some(exe_path), clang_format_command: None, - repo_root: PathBuf::from(".").join(".cpp-linter-cache"), + repo_root: PathBuf::from("."), }; let mut file_lock = arc_file.lock().unwrap(); let logs = run_clang_tidy(&mut file_lock, &clang_params) From fc628c5c438f1f47f2617582dfe041050f76cbce Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Wed, 10 Jun 2026 19:45:31 -0700 Subject: [PATCH 03/13] adjust integration tests to rely on `--repo-root` using an absolute path --- cpp-linter/src/clang_tools/clang_tidy.rs | 30 +++++++++++---------- cpp-linter/src/cli/structs.rs | 3 +++ cpp-linter/src/common_fs/mod.rs | 11 ++++++-- cpp-linter/src/git.rs | 3 ++- cpp-linter/src/rest_client/mod.rs | 10 ++++--- cpp-linter/src/run.rs | 7 ++--- cpp-linter/tests/comments.rs | 9 +++---- cpp-linter/tests/common/mod.rs | 22 +++++++-------- cpp-linter/tests/paginated_changed_files.rs | 14 +++++----- cpp-linter/tests/reviews.rs | 9 +++---- 10 files changed, 67 insertions(+), 51 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index c6049490..510f8a18 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -2,9 +2,9 @@ //! output. use std::{ - env::{consts::OS, current_dir}, + env::consts::OS, fs, - path::PathBuf, + path::{Path, PathBuf}, process::Command, sync::{Arc, Mutex, MutexGuard}, }; @@ -144,13 +144,13 @@ const NOTE_HEADER: &str = r"^(.+):(\d+):(\d+):\s(\w+):(.*)\[([a-zA-Z\d\-\.]+),?[ fn parse_tidy_output( tidy_stdout: &[u8], database_json: &Option>, + repo_root: &Path, ) -> Result { 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(); - let cur_dir = current_dir().map_err(ClangCaptureError::UnknownWorkingDirectory)?; for line in String::from_utf8(tidy_stdout.to_vec()) .map_err(|e| ClangCaptureError::NonUtf8Output { task: "convert clang-tidy stdout".to_string(), @@ -191,16 +191,16 @@ fn parse_tidy_output( // 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([&cur_dir, &filename])); + 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([&cur_dir, &filename])); + filename = normalize_path(&PathBuf::from_iter([repo_root, &filename])); } - debug_assert!(filename.is_absolute()); + if filename.is_absolute() - && let Ok(file_n) = filename.strip_prefix(&cur_dir) + && 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) @@ -297,16 +297,17 @@ pub fn run_clang_tidy( ); cmd.args(["--line-filter", filter.as_str()]); } + let repo_file_path = PathBuf::from_iter([&clang_params.repo_root, &file.name]); let original_content = if !clang_params.tidy_review { None } else { cmd.arg("--fix-errors"); - Some( - fs::read_to_string(&file.name).map_err(|e| ClangCaptureError::ReadFileFailed { + Some(fs::read_to_string(&repo_file_path).map_err(|e| { + ClangCaptureError::ReadFileFailed { file_name: file_name.clone(), source: e, - })?, - ) + } + })?) }; if !clang_params.style.is_empty() { cmd.args(["--format-style", clang_params.style.as_str()]); @@ -348,6 +349,7 @@ pub fn run_clang_tidy( file.tidy_advice = Some(parse_tidy_output( &output.stdout, &clang_params.database_json, + &clang_params.repo_root, )?); if clang_params.tidy_review && let Some(original_content) = &original_content @@ -356,14 +358,14 @@ pub fn run_clang_tidy( // cache file changes in a buffer and restore the original contents for further analysis tidy_advice.patched = Some( - fs::read(&file_name).map_err(|e| ClangCaptureError::ReadFileFailed { + fs::read(&repo_file_path).map_err(|e| ClangCaptureError::ReadFileFailed { file_name: file_name.clone(), source: e, })?, ); } // original_content is guaranteed to be Some() value at this point - fs::write(&file_name, original_content).map_err(|e| { + fs::write(&repo_file_path, original_content).map_err(|e| { ClangCaptureError::WriteFileFailed { file_name, source: e, @@ -565,7 +567,7 @@ TrenchBroom/TrenchBroom/common/test/src/mdl/tst_ReadFreeImageTexture.cpp:44:48: 44 | return diskFS.openFile(name) | kdl::and_then([](const auto& file) { | ^ "#; - let advice = parse_tidy_output(tidy_out.as_bytes(), &None).unwrap(); + let advice = parse_tidy_output(tidy_out.as_bytes(), &None, &PathBuf::from(".")).unwrap(); assert_eq!(advice.notes.len(), 4); for note in advice.notes { assert!(note.diagnostic.contains('-')); diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index 4ed17016..7f60ccb2 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -232,6 +232,7 @@ pub struct FeedbackInput { pub tidy_review: bool, pub format_review: bool, pub passive_reviews: bool, + pub repo_root: PathBuf, } #[cfg(feature = "bin")] @@ -247,6 +248,7 @@ impl From<&Cli> for FeedbackInput { tidy_review: args.feedback_options.tidy_review, format_review: args.feedback_options.format_review, passive_reviews: args.feedback_options.passive_reviews, + repo_root: PathBuf::from(&args.source_options.repo_root), } } } @@ -263,6 +265,7 @@ impl Default for FeedbackInput { tidy_review: false, format_review: false, passive_reviews: false, + repo_root: PathBuf::from("."), } } } diff --git a/cpp-linter/src/common_fs/mod.rs b/cpp-linter/src/common_fs/mod.rs index 51ab07fb..d6c87fe8 100644 --- a/cpp-linter/src/common_fs/mod.rs +++ b/cpp-linter/src/common_fs/mod.rs @@ -1,6 +1,11 @@ //! A module to hold all common file system functionality. -use std::{fmt::Debug, fs, ops::RangeInclusive, path::PathBuf}; +use std::{ + fmt::Debug, + fs, + ops::RangeInclusive, + path::{Path, PathBuf}, +}; use gix_imara_diff::Hunk; @@ -140,8 +145,10 @@ impl FileObj { &self, review_comments: &mut ReviewComments, summary_only: bool, + repo_root: &Path, ) -> Result<(), FileObjError> { - let original_content = fs::read_to_string(&self.name).map_err(FileObjError::ReadFile)?; + let original_content = + fs::read_to_string(repo_root.join(&self.name)).map_err(FileObjError::ReadFile)?; let file_name = self.name.to_str().unwrap_or_default().replace("\\", "/"); if let Some(advice) = &self.format_advice { let patched = fs::read_to_string(&advice.patched).map_err(FileObjError::ReadFile)?; diff --git a/cpp-linter/src/git.rs b/cpp-linter/src/git.rs index 309a2b49..03c7cb44 100644 --- a/cpp-linter/src/git.rs +++ b/cpp-linter/src/git.rs @@ -9,6 +9,7 @@ mod test { use std::{ env::{self, current_dir, set_current_dir}, fs, + path::PathBuf, process::Command, }; @@ -101,7 +102,7 @@ mod test { &LinesChangedOnly::Off.into(), &base_diff, ignore_staged, - ".", + &PathBuf::from("."), ) .await .unwrap() diff --git a/cpp-linter/src/rest_client/mod.rs b/cpp-linter/src/rest_client/mod.rs index 8b1344be..e330af87 100644 --- a/cpp-linter/src/rest_client/mod.rs +++ b/cpp-linter/src/rest_client/mod.rs @@ -1,6 +1,6 @@ use std::{ env, - path::PathBuf, + path::{Path, PathBuf}, sync::{Arc, Mutex}, }; @@ -54,7 +54,7 @@ impl RestClient { lines_changed_only: &LinesChangedOnly, base_diff: &Option, ignore_index: bool, - repo_root: &str, + repo_root: &Path, ) -> Result, ClientError> { let files = self .client @@ -181,7 +181,11 @@ impl RestClient { let file = file .lock() .map_err(|e| ClientError::MutexPoisoned(e.to_string()))?; - file.make_suggestions_from_patch(&mut review_comments, summary_only)?; + file.make_suggestions_from_patch( + &mut review_comments, + summary_only, + &feedback_inputs.repo_root, + )?; } let mut options = ReviewOptions { diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index 63f2dc50..2aa5e6d0 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -101,6 +101,7 @@ pub async fn run_main(args: Vec) -> Result<()> { } rest_api_client.start_log_group("Get list of specified source files"); + let repo_root_path = PathBuf::from(&cli.source_options.repo_root); let files = if !matches!(cli.source_options.lines_changed_only, LinesChangedOnly::Off) || cli.source_options.files_changed_only { @@ -111,7 +112,7 @@ pub async fn run_main(args: Vec) -> Result<()> { &cli.source_options.lines_changed_only.clone().into(), &cli.source_options.diff_base, cli.source_options.ignore_index, - &cli.source_options.repo_root, + &repo_root_path, ) .await? } else { @@ -123,7 +124,7 @@ pub async fn run_main(args: Vec) -> Result<()> { let file_path = PathBuf::from(&file_name); FileObj::new( file_path - .strip_prefix(&cli.source_options.repo_root) + .strip_prefix(&repo_root_path) .map(PathBuf::from) .unwrap_or(file_path), ) @@ -136,7 +137,7 @@ pub async fn run_main(args: Vec) -> Result<()> { &LinesChangedOnly::Off.into(), &cli.source_options.diff_base, cli.source_options.ignore_index, - &cli.source_options.repo_root, + &repo_root_path, ) .await?; for changed_file in changed_files { diff --git a/cpp-linter/tests/comments.rs b/cpp-linter/tests/comments.rs index b77aa13d..45656745 100644 --- a/cpp-linter/tests/comments.rs +++ b/cpp-linter/tests/comments.rs @@ -5,7 +5,7 @@ use cpp_linter::run::run_main; use git_bot_feedback::LinesChangedOnly; use mockito::Matcher; use std::{env, fmt::Display, io::Write, path::Path}; -use tempfile::NamedTempFile; +use tempfile::{NamedTempFile, TempDir}; mod common; use common::{create_test_space, mock_server}; @@ -62,7 +62,7 @@ impl Default for TestParams { } } -async fn setup(lib_root: &Path, test_params: &TestParams) { +async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { let mut event_payload_path = NamedTempFile::new_in("./").unwrap(); let tmp_out_file = NamedTempFile::new().unwrap(); unsafe { @@ -253,6 +253,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) { format!("--no-lgtm={}", test_params.no_lgtm), "-p=build".to_string(), "-i=build".to_string(), + format!("--repo-root={}", tmp_dir.path().to_str().unwrap()), ]; if test_params.force_lgtm { args.push("-e=c".to_string()); @@ -278,9 +279,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) { async fn test_comment(test_params: &TestParams) { let tmp_dir = create_test_space(true); let lib_root = env::current_dir().unwrap(); - env::set_current_dir(tmp_dir.path()).unwrap(); - setup(&lib_root, test_params).await; - env::set_current_dir(lib_root.as_path()).unwrap(); + setup(&lib_root, &tmp_dir, test_params).await; drop(tmp_dir); } diff --git a/cpp-linter/tests/common/mod.rs b/cpp-linter/tests/common/mod.rs index b73e99d6..10a72618 100644 --- a/cpp-linter/tests/common/mod.rs +++ b/cpp-linter/tests/common/mod.rs @@ -21,7 +21,7 @@ use tempfile::TempDir; /// The returned directory object will automatically delete the /// temporary folder when it is dropped out of scope. pub fn create_test_space(setup_meson: bool) -> TempDir { - let tmp = TempDir::new().unwrap(); + let tmp = TempDir::with_prefix("cpp-linter-test").unwrap(); fs::create_dir(tmp.path().join("src")).unwrap(); let src = fs::read_dir("tests/demo").unwrap(); for file in src { @@ -48,11 +48,10 @@ pub fn create_test_space(setup_meson: bool) -> TempDir { "demo.cpp", "demo.hpp", ]); - let output = cmd.output().expect("Failed to run 'meson init'"); - println!( - "meson init stdout:\n{}", - String::from_utf8(output.stdout.to_vec()).unwrap() - ); + let status = cmd.status().expect("Failed to run 'meson init'"); + if !status.success() { + panic!("Failed to initialize meson project with 'meson init'"); + } let meson_build_dir = tmp.path().join("build"); let mut cmd = Command::new("meson"); cmd.args([ @@ -61,13 +60,12 @@ pub fn create_test_space(setup_meson: bool) -> TempDir { meson_build_dir.as_path().to_str().unwrap(), test_dir.to_str().unwrap(), ]); - let output = cmd - .output() + let status = cmd + .status() .expect("Failed to generate build assets with 'meson setup'"); - println!( - "meson setup stdout:\n{}", - String::from_utf8(output.stdout.to_vec()).unwrap() - ); + if !status.success() { + panic!("Failed to generate build assets with 'meson setup'"); + } let db = fs::File::open(meson_build_dir.join("compile_commands.json")) .expect("Failed to open compilation database"); for line in io::BufReader::new(db).lines().map_while(Result::ok) { diff --git a/cpp-linter/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs index 464ccb0e..647e82c1 100644 --- a/cpp-linter/tests/paginated_changed_files.rs +++ b/cpp-linter/tests/paginated_changed_files.rs @@ -7,7 +7,11 @@ use mockito::Matcher; use tempfile::{NamedTempFile, TempDir}; use cpp_linter::{logger, rest_client::RestClient}; -use std::{env, io::Write, path::Path}; +use std::{ + env, + io::Write, + path::{Path, PathBuf}, +}; #[derive(PartialEq, Default)] enum EventType { @@ -32,7 +36,7 @@ const RESET_RATE_LIMIT_HEADER: &str = "x-ratelimit-reset"; const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining"; const MALFORMED_RESPONSE_PAYLOAD: &str = "{\"message\":\"Resource not accessible by integration\"}"; -async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) { +async fn get_paginated_changes(lib_root: &Path, _tmp_dir: &TempDir, test_params: &TestParams) { let tmp = TempDir::new().expect("Failed to create a temp dir for test"); let mut event_payload = NamedTempFile::new_in(tmp.path()) .expect("Failed to spawn a tmp file for test event payload"); @@ -145,7 +149,7 @@ async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) { &LinesChangedOnly::Off, &None::, false, - ".", + &PathBuf::from("."), ) .await; match files { @@ -177,9 +181,7 @@ async fn get_paginated_changes(lib_root: &Path, test_params: &TestParams) { async fn test_get_changes(test_params: &TestParams) { let tmp_dir = create_test_space(false); let lib_root = env::current_dir().unwrap(); - env::set_current_dir(tmp_dir.path()).unwrap(); - get_paginated_changes(&lib_root, test_params).await; - env::set_current_dir(lib_root.as_path()).unwrap(); + get_paginated_changes(&lib_root, &tmp_dir, test_params).await; drop(tmp_dir); } diff --git a/cpp-linter/tests/reviews.rs b/cpp-linter/tests/reviews.rs index b13c16ac..3d6134d7 100644 --- a/cpp-linter/tests/reviews.rs +++ b/cpp-linter/tests/reviews.rs @@ -7,7 +7,7 @@ use cpp_linter::{ use git_bot_feedback::LinesChangedOnly; use mockito::Matcher; use std::{env, io::Write, path::Path}; -use tempfile::NamedTempFile; +use tempfile::{NamedTempFile, TempDir}; mod common; use common::{create_test_space, mock_server}; @@ -70,7 +70,7 @@ fn generate_tool_summary(review_enabled: bool, force_lgtm: bool, tool_name: &str } } -async fn setup(lib_root: &Path, test_params: &TestParams) { +async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { let mut event_payload_path = NamedTempFile::new_in("./").unwrap(); let event_payload = if test_params.bad_pr_info { "".to_string() @@ -239,6 +239,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) { format!("--no-lgtm={}", test_params.no_lgtm), "-p=build".to_string(), "-i=build".to_string(), + format!("--repo-root={}", tmp_dir.path().to_str().unwrap()), ]; if test_params.force_lgtm { if test_params.tidy_review { @@ -272,9 +273,7 @@ async fn setup(lib_root: &Path, test_params: &TestParams) { async fn test_review(test_params: &TestParams) { let tmp_dir = create_test_space(true); let lib_root = env::current_dir().unwrap(); - env::set_current_dir(tmp_dir.path()).unwrap(); - setup(&lib_root, test_params).await; - env::set_current_dir(lib_root.as_path()).unwrap(); + setup(&lib_root, &tmp_dir, test_params).await; drop(tmp_dir); } From a6d069e5afe50999ceb450d52660c614c3d00ddc Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Wed, 10 Jun 2026 20:09:42 -0700 Subject: [PATCH 04/13] finish refactoring paginated_files test --- cpp-linter/tests/paginated_changed_files.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/cpp-linter/tests/paginated_changed_files.rs b/cpp-linter/tests/paginated_changed_files.rs index 647e82c1..7563a10b 100644 --- a/cpp-linter/tests/paginated_changed_files.rs +++ b/cpp-linter/tests/paginated_changed_files.rs @@ -7,11 +7,7 @@ use mockito::Matcher; use tempfile::{NamedTempFile, TempDir}; use cpp_linter::{logger, rest_client::RestClient}; -use std::{ - env, - io::Write, - path::{Path, PathBuf}, -}; +use std::{env, io::Write, path::Path}; #[derive(PartialEq, Default)] enum EventType { @@ -36,9 +32,8 @@ const RESET_RATE_LIMIT_HEADER: &str = "x-ratelimit-reset"; const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining"; const MALFORMED_RESPONSE_PAYLOAD: &str = "{\"message\":\"Resource not accessible by integration\"}"; -async fn get_paginated_changes(lib_root: &Path, _tmp_dir: &TempDir, test_params: &TestParams) { - let tmp = TempDir::new().expect("Failed to create a temp dir for test"); - let mut event_payload = NamedTempFile::new_in(tmp.path()) +async fn get_paginated_changes(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { + let mut event_payload = NamedTempFile::new_in(tmp_dir.path()) .expect("Failed to spawn a tmp file for test event payload"); unsafe { env::set_var("GITHUB_ACTIONS", "true"); @@ -88,7 +83,6 @@ async fn get_paginated_changes(lib_root: &Path, _tmp_dir: &TempDir, test_params: unsafe { env::set_var("GITHUB_API_URL", server.url()); } - env::set_current_dir(tmp.path()).unwrap(); logger::try_init(); log::set_max_level(log::LevelFilter::Debug); let gh_client = RestClient::new(); @@ -149,7 +143,7 @@ async fn get_paginated_changes(lib_root: &Path, _tmp_dir: &TempDir, test_params: &LinesChangedOnly::Off, &None::, false, - &PathBuf::from("."), + tmp_dir.path(), ) .await; match files { From 81a51bcc4462e18894a4d0836d6fa65dfb69ef4f Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Wed, 10 Jun 2026 20:19:06 -0700 Subject: [PATCH 05/13] apply repo-root to database path --- cpp-linter/src/clang_tools/mod.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index bac0e0c4..b6de9afd 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -140,7 +140,15 @@ pub async fn capture_clang_tools_output( // parse database (if provided) to match filenames when parsing clang-tidy's stdout if let Some(db_path) = &clang_params.database - && let Ok(db_str) = fs::read(db_path.join("compile_commands.json")) + && let Ok(db_str) = fs::read( + if db_path.is_relative() { + clang_params.repo_root.join(db_path) + } else { + db_path.to_path_buf() + } + .join(db_path) + .join("compile_commands.json"), + ) { clang_params.database_json = Some( // A compilation database should be UTF-8 encoded, but file paths are not; use lossy conversion. From 9b74fbd86f799110c99b8f25643f0eb00f31cb01 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Wed, 10 Jun 2026 20:26:48 -0700 Subject: [PATCH 06/13] be consistent --- cpp-linter/src/clang_tools/clang_tidy.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp-linter/src/clang_tools/clang_tidy.rs b/cpp-linter/src/clang_tools/clang_tidy.rs index 510f8a18..bc7201b4 100644 --- a/cpp-linter/src/clang_tools/clang_tidy.rs +++ b/cpp-linter/src/clang_tools/clang_tidy.rs @@ -297,7 +297,7 @@ pub fn run_clang_tidy( ); cmd.args(["--line-filter", filter.as_str()]); } - let repo_file_path = PathBuf::from_iter([&clang_params.repo_root, &file.name]); + let repo_file_path = clang_params.repo_root.join(&file.name); let original_content = if !clang_params.tidy_review { None } else { From 586a2471fd6f2a1313c170232f80b490d2c3185d Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 01:49:56 -0700 Subject: [PATCH 07/13] more review changes --- Cargo.lock | 4 +-- cpp-linter/Cargo.toml | 2 +- cpp-linter/src/clang_tools/mod.rs | 41 +++++++++++++++++-------------- cpp-linter/src/cli/mod.rs | 7 +++--- cpp-linter/src/cli/structs.rs | 18 ++++++++++++-- 5 files changed, 45 insertions(+), 27 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d4a8c68b..59254ca6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -732,9 +732,9 @@ dependencies = [ [[package]] name = "git-bot-feedback" -version = "0.5.5" +version = "0.5.6" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "2188a95438d5b5a329cd55f422c88cdc6efd8a191f88e1167270bb2ff8bcaee3" +checksum = "87c5dcb915266b670fbc0f47f7240cfe832a0e9363e7f509ef38dec960b80824" dependencies = [ "async-trait", "chrono", diff --git a/cpp-linter/Cargo.toml b/cpp-linter/Cargo.toml index 6ea5939c..41c18957 100644 --- a/cpp-linter/Cargo.toml +++ b/cpp-linter/Cargo.toml @@ -33,7 +33,7 @@ thiserror = { workspace = true } tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } [dependencies.git-bot-feedback] -version = "0.5.5" +version = "0.5.6" features = ["file-changes"] # path = "../../git-bot-feedback" # git = "https://github.com/2bndy5/git-bot-feedback" diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index b6de9afd..63a44bb5 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -17,15 +17,16 @@ use tokio::task::JoinSet; // project-specific modules/crates use super::common_fs::FileObj; -use crate::error::{ClangCaptureError, ClangTaskError}; use crate::{ + clang_tools::clang_tidy::CompilationUnit, cli::ClangParams, + error::{ClangCaptureError, ClangTaskError}, rest_client::{RestClient, USER_OUTREACH}, }; pub mod clang_format; use clang_format::run_clang_format; pub mod clang_tidy; -use clang_tidy::{CompilationUnit, run_clang_tidy}; +use clang_tidy::run_clang_tidy; pub const CACHE_DIR: &str = ".cpp-linter-cache"; @@ -137,23 +138,27 @@ pub async fn capture_clang_tools_output( clang_versions.format_version = Some(tool_info.version); clang_params.clang_format_command = Some(tool_info.path); } - - // parse database (if provided) to match filenames when parsing clang-tidy's stdout - if let Some(db_path) = &clang_params.database - && let Ok(db_str) = fs::read( - if db_path.is_relative() { - clang_params.repo_root.join(db_path) - } else { - db_path.to_path_buf() + if let Some(db_path) = &clang_params.database { + let db_path = db_path.join("compile_commands.json"); + match fs::read_to_string(&db_path) { + Ok(db_str) => match serde_json::from_str::>(&db_str) { + Ok(db_json) => { + clang_params.database_json = Some(db_json); + } + Err(e) => { + log::warn!( + "Failed to parse compilation database JSON at {}: {e:?}", + db_path.to_string_lossy() + ); + } + }, + Err(e) => { + log::warn!( + "Failed to read compilation database file at {}: {e:?}", + db_path.to_string_lossy() + ); } - .join(db_path) - .join("compile_commands.json"), - ) - { - clang_params.database_json = Some( - // A compilation database should be UTF-8 encoded, but file paths are not; use lossy conversion. - serde_json::from_str::>(&String::from_utf8_lossy(&db_str))?, - ) + } }; let mut executors = JoinSet::new(); diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index e66cf796..7b7c5841 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -153,14 +153,13 @@ pub struct SourceOptions { /// The relative path to the repository root directory. /// - /// This path is relative to the runner's `GITHUB_WORKSPACE` - /// environment variable (or the current working directory if - /// not using a CI runner). + /// This path should be relative to the current + /// working directory. It can also be an absolute path. #[cfg_attr( feature = "bin", arg(short, long, default_value = ".", help_heading = "Source options") )] - pub repo_root: String, + pub repo_root: PathBuf, /// This controls what part of the files are analyzed. #[cfg_attr( diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index 7f60ccb2..d18994fe 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -1,3 +1,4 @@ +#[cfg(feature = "bin")] use std::{fmt::Display, path::PathBuf}; #[cfg(feature = "bin")] @@ -203,10 +204,23 @@ impl From<&Cli> for ClangParams { let ignore_format: Vec<&str> = ignore_format.iter().map(|s| s.as_str()).collect(); FileFilter::new(&ignore_format, &extensions, Some("clang-format")) }); + let repo_root = args.source_options.repo_root.clone(); + let database = args + .tidy_options + .database + .as_ref() + .map(PathBuf::from) + .map(|db| { + if db.is_relative() { + repo_root.join(db) + } else { + db + } + }); ClangParams { tidy_checks: args.tidy_options.tidy_checks.clone(), lines_changed_only: args.source_options.lines_changed_only.clone(), - database: args.tidy_options.database.clone(), + database, extra_args: args.tidy_options.extra_arg.clone(), database_json: None, style: args.format_options.style.clone(), @@ -216,7 +230,7 @@ impl From<&Cli> for ClangParams { format_filter, tidy_review: args.feedback_options.tidy_review, format_review: args.feedback_options.format_review, - repo_root: PathBuf::from(&args.source_options.repo_root), + repo_root, } } } From 42314b2d75b6cc4e9f67a325d18080f62fcb807f Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 02:04:22 -0700 Subject: [PATCH 08/13] rs doc fixes --- cpp-linter/src/clang_tools/clang_format.rs | 2 +- cpp-linter/src/cli/structs.rs | 1 - cpp-linter/src/common_fs/mod.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs index 90f84fd2..fb10d909 100644 --- a/cpp-linter/src/clang_tools/clang_format.rs +++ b/cpp-linter/src/clang_tools/clang_format.rs @@ -18,7 +18,7 @@ use crate::{cli::ClangParams, common_fs::FileObj, error::ClangCaptureError}; #[derive(Debug, Clone, PartialEq, Eq, Default)] pub struct FormatAdvice { - /// A list of [`Replacement`]s that clang-tidy wants to make. + /// A list of line ranges that clang-format wants to replace. pub replacements: Vec>, pub patched: PathBuf, diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index d18994fe..5de07184 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -1,4 +1,3 @@ -#[cfg(feature = "bin")] use std::{fmt::Display, path::PathBuf}; #[cfg(feature = "bin")] diff --git a/cpp-linter/src/common_fs/mod.rs b/cpp-linter/src/common_fs/mod.rs index d6c87fe8..cd78b340 100644 --- a/cpp-linter/src/common_fs/mod.rs +++ b/cpp-linter/src/common_fs/mod.rs @@ -136,7 +136,7 @@ impl FileObj { } /// Create a list of [`Suggestion`](struct@crate::clang_tools::Suggestion) from a - /// generated [`Patch`](struct@git2::Patch) and store them in the given + /// generated diff and store them in the given /// [`ReviewComments`](struct@crate::clang_tools::ReviewComments). /// /// The suggestions will also include diagnostics from clang-tidy that From 9ab50f34de71d4656293b07b7a88ca5c259001f8 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 02:06:46 -0700 Subject: [PATCH 09/13] fix pure insertions about clang-format replacements Follow up to #344 --- cpp-linter/src/clang_tools/clang_format.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs index fb10d909..35d51fdf 100644 --- a/cpp-linter/src/clang_tools/clang_format.rs +++ b/cpp-linter/src/clang_tools/clang_format.rs @@ -146,7 +146,7 @@ pub fn run_clang_format( .hunks() .filter_map(|hunk| { let replacement = if hunk.is_pure_insertion() { - RangeInclusive::new(hunk.after.start, hunk.after.end.saturating_sub(1)) + RangeInclusive::new(hunk.after.start, hunk.after.start) } else { RangeInclusive::new(hunk.before.start, hunk.before.end.saturating_sub(1)) }; From 12660a99ae51ad88ed3dad442f78305003abc239 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 02:14:23 -0700 Subject: [PATCH 10/13] AI suggested doc comment fix --- cpp-linter/src/cli/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index 7b7c5841..7a4a68bc 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -151,7 +151,7 @@ pub struct SourceOptions { )] pub extensions: Vec, - /// The relative path to the repository root directory. + /// The path to the repository root directory. /// /// This path should be relative to the current /// working directory. It can also be an absolute path. From 4baa52298b178decc110f28806dbca0b79779364 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 02:26:46 -0700 Subject: [PATCH 11/13] upgrade codecov action to v7 fixes GPG problems in v6 since they migrated --- .github/workflows/run-dev-tests.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/run-dev-tests.yml b/.github/workflows/run-dev-tests.yml index 8d1208b2..39cf8385 100644 --- a/.github/workflows/run-dev-tests.yml +++ b/.github/workflows/run-dev-tests.yml @@ -163,9 +163,8 @@ jobs: - name: Generate Coverage lcov report run: nur test lcov - - uses: codecov/codecov-action@e79a6962e0d4c0c17b229090214935d2e33f8354 # v6.0.1 + - uses: codecov/codecov-action@fb8b3582c8e4def4969c97caa2f19720cb33a72f # v7.0.0 with: token: ${{secrets.CODECOV_TOKEN}} files: lcov.info fail_ci_if_error: true # optional (default = false) - use_pypi: true # workaround unexpected GPG key changes (temporarily) From 7c71a75c6576e92afc7b8125a92fa25702dfe48a Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 03:54:52 -0700 Subject: [PATCH 12/13] add more tests --- cpp-linter/src/clang_tools/mod.rs | 41 +++++++++++++++++++++++++++++++ cpp-linter/src/cli/structs.rs | 19 +++++++++++++- 2 files changed, 59 insertions(+), 1 deletion(-) diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index 63a44bb5..e5312ccb 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -397,3 +397,44 @@ pub trait MakeSuggestions { *tool_total += hunks_in_patch; } } + +#[cfg(test)] +mod tests { + #![allow(clippy::unwrap_used)] + + use std::{env, fs, path::Path}; + + use super::*; + #[cfg(feature = "bin")] + use crate::logger::try_init; + + async fn test_db_parse>(path: P) -> Result { + let clang_params = ClangParams { + database: Some(path.as_ref().to_path_buf()), + repo_root: PathBuf::from("."), + ..Default::default() + }; + let version = RequestedVersion::default(); + // We don't need to use any specific git REST API client for this. + unsafe { + env::remove_var("GITHUB_ACTIONS"); + } + let rest_client = RestClient::new().unwrap(); + #[cfg(feature = "bin")] + try_init(); + capture_clang_tools_output(&[], &version, clang_params, &rest_client).await + } + + #[tokio::test] + async fn bad_db_path() { + test_db_parse("nonexistent/path").await.unwrap(); + } + + #[tokio::test] + async fn bad_db_json() { + let tmp_dir = tempfile::tempdir().unwrap(); + let db_path = tmp_dir.path().join("compile_commands.json"); + fs::write(&db_path, "not a valid json").unwrap(); + test_db_parse(tmp_dir.path()).await.unwrap(); + } +} diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index 5de07184..dea13104 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -288,8 +288,9 @@ mod test { #![allow(clippy::unwrap_used)] use clap::{Parser, ValueEnum}; + use std::path::PathBuf; - use super::{Cli, LinesChangedOnly, ThreadComments}; + use super::{ClangParams, Cli, LinesChangedOnly, ThreadComments}; #[test] fn parse_positional() { @@ -328,4 +329,20 @@ mod test { ThreadComments::Off ); } + + #[test] + fn relative_db_path() { + let cli = Cli::parse_from([ + "cpp-linter", + "--database", + "path/to/compile_commands.json", + "--repo-root", + "relative", + ]); + let clang_params = ClangParams::from(&cli); + assert_eq!( + clang_params.database, + Some(PathBuf::from("relative/path/to/compile_commands.json")) + ); + } } From dca82d964a942b6a8a0da0b83654018526b86390 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Thu, 11 Jun 2026 04:41:02 -0700 Subject: [PATCH 13/13] last review change --- cpp-linter/src/cli/structs.rs | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index dea13104..572234be 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -261,7 +261,7 @@ impl From<&Cli> for FeedbackInput { tidy_review: args.feedback_options.tidy_review, format_review: args.feedback_options.format_review, passive_reviews: args.feedback_options.passive_reviews, - repo_root: PathBuf::from(&args.source_options.repo_root), + repo_root: args.source_options.repo_root.clone(), } } } @@ -288,7 +288,6 @@ mod test { #![allow(clippy::unwrap_used)] use clap::{Parser, ValueEnum}; - use std::path::PathBuf; use super::{ClangParams, Cli, LinesChangedOnly, ThreadComments}; @@ -331,18 +330,10 @@ mod test { } #[test] - fn relative_db_path() { - let cli = Cli::parse_from([ - "cpp-linter", - "--database", - "path/to/compile_commands.json", - "--repo-root", - "relative", - ]); + fn absolute_db_path() { + let tmp_dir = tempfile::tempdir().unwrap(); + let cli = Cli::parse_from(["cpp-linter", "--database", tmp_dir.path().to_str().unwrap()]); let clang_params = ClangParams::from(&cli); - assert_eq!( - clang_params.database, - Some(PathBuf::from("relative/path/to/compile_commands.json")) - ); + assert_eq!(clang_params.database, Some(tmp_dir.path().to_path_buf())); } }