From d89f39b7c5a67167b96b523b8c1db712e5c63c6a Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 26 Nov 2024 15:53:18 -0800 Subject: [PATCH 1/3] prevent tidy diagnostics outside of PR's diff --- cpp-linter/src/clang_tools/mod.rs | 4 ++-- cpp-linter/src/common_fs/mod.rs | 27 ++++++++++++++++++++++++--- 2 files changed, 26 insertions(+), 5 deletions(-) diff --git a/cpp-linter/src/clang_tools/mod.rs b/cpp-linter/src/clang_tools/mod.rs index c05bbd5..c2be8b2 100644 --- a/cpp-linter/src/clang_tools/mod.rs +++ b/cpp-linter/src/clang_tools/mod.rs @@ -180,7 +180,7 @@ pub async fn capture_clang_tools_output( ); clang_versions.tidy_version = Some(version_found); clang_params.clang_tidy_command = Some(exe_path); - }; + } if !clang_params.style.is_empty() { let exe_path = get_clang_tool_exe("clang-format", version)?; let version_found = capture_clang_version(&exe_path)?; @@ -190,7 +190,7 @@ pub async fn capture_clang_tools_output( ); clang_versions.format_version = Some(version_found); clang_params.clang_format_command = Some(exe_path); - }; + } // parse database (if provided) to match filenames when parsing clang-tidy's stdout if let Some(db_path) = &clang_params.database { diff --git a/cpp-linter/src/common_fs/mod.rs b/cpp-linter/src/common_fs/mod.rs index 447e69d..c97d89c 100644 --- a/cpp-linter/src/common_fs/mod.rs +++ b/cpp-linter/src/common_fs/mod.rs @@ -119,6 +119,20 @@ impl FileObj { None } + /// Similar to [`FileObj::is_hunk_in_diff()`] but looks for a single line instead of + /// an entire [`DiffHunk`]. + /// + /// This is a private function it is because only used in + /// [`FileObj::make_suggestions_from_patch()`]. + fn is_line_in_diff(&self, line: &u32) -> bool { + for range in &self.diff_chunks { + if range.contains(line) { + return true; + } + } + false + } + /// Create a list of [`Suggestion`](struct@crate::clang_tools::Suggestion) from a /// generated [`Patch`](struct@git2::Patch) and store them in the given /// [`ReviewComments`](struct@crate::clang_tools::ReviewComments). @@ -161,10 +175,10 @@ impl FileObj { // Count of clang-tidy diagnostics that had no fixes applied let mut total = 0; for note in &advice.notes { - if note.fixed_lines.is_empty() { + if note.fixed_lines.is_empty() && self.is_line_in_diff(¬e.line) { // notification had no suggestion applied in `patched` let mut suggestion = format!( - "### clang-tidy diagnostic\n**{file_name}:{}:{}** {}: [{}]\n> {}", + "### clang-tidy diagnostic\n**{file_name}:{}:{}** {}: [{}]\n\n> {}\n", ¬e.line, ¬e.cols, ¬e.severity, @@ -173,7 +187,8 @@ impl FileObj { ); if !note.suggestion.is_empty() { suggestion.push_str( - format!("```{file_ext}\n{}```", ¬e.suggestion.join("\n")).as_str(), + format!("\n```{file_ext}\n{}\n```\n", ¬e.suggestion.join("\n")) + .as_str(), ); } total += 1; @@ -342,4 +357,10 @@ mod test { let ranges = file_obj.get_ranges(&LinesChangedOnly::On); assert_eq!(ranges, vec![4..=5, 9..=9]); } + + #[test] + fn line_not_in_diff() { + let file_obj = FileObj::new(PathBuf::from("tests/demo/demo.cpp")); + assert!(!file_obj.is_line_in_diff(&42)); + } } From 172373c349aa7a988dde7f5563bc77d88fda0dc5 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 26 Nov 2024 17:58:18 -0800 Subject: [PATCH 2/3] cover missing lines --- cpp-linter/src/run.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index 8dd16d0..b4c65dc 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -221,4 +221,20 @@ mod test { .await; assert!(result.is_err()); } + + // just for completion. this test is not practical use case + #[tokio::test] + async fn no_analysis() { + env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests + let result = run_main(vec![ + "cpp-linter".to_string(), + "-l".to_string(), + "false".to_string(), + "--style".to_string(), + String::new(), + "--tidy-checks=-*".to_string(), + ]) + .await; + assert!(result.is_ok()); + } } From 1e6ede593dd85497bf1d7fb5802a3e45c39d640d Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 26 Nov 2024 18:16:14 -0800 Subject: [PATCH 3/3] fix comments (per AI advice) --- cpp-linter/src/common_fs/mod.rs | 2 +- cpp-linter/src/run.rs | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/cpp-linter/src/common_fs/mod.rs b/cpp-linter/src/common_fs/mod.rs index c97d89c..00bf62b 100644 --- a/cpp-linter/src/common_fs/mod.rs +++ b/cpp-linter/src/common_fs/mod.rs @@ -122,7 +122,7 @@ impl FileObj { /// Similar to [`FileObj::is_hunk_in_diff()`] but looks for a single line instead of /// an entire [`DiffHunk`]. /// - /// This is a private function it is because only used in + /// This is a private function because it is only used in /// [`FileObj::make_suggestions_from_patch()`]. fn is_line_in_diff(&self, line: &u32) -> bool { for range in &self.diff_chunks { diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index b4c65dc..d7fe857 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -222,7 +222,8 @@ mod test { assert!(result.is_err()); } - // just for completion. this test is not practical use case + // Verifies that the system gracefully handles cases where all analysis is disabled. + // This ensures no diagnostic comments are generated when analysis is explicitly skipped. #[tokio::test] async fn no_analysis() { env::remove_var("GITHUB_OUTPUT"); // avoid writing to GH_OUT in parallel-running tests