From fedeef88bd96cb06286f0f884610ad059f4c6906 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 20 Oct 2026 01:08:10 -0700 Subject: [PATCH 1/4] feat: add `--fix` as alias to new `--gen-patch/-F` arg resolves #500 --- cpp-linter/src/clang_tools/clang_format.rs | 15 +++++++++ cpp-linter/src/clang_tools/clang_tidy/mod.rs | 4 ++- .../clang_tools/clang_tidy/other_files/mod.rs | 8 +++-- .../clang_tidy/other_files/test.rs | 9 ++++++ cpp-linter/src/clang_tools/clang_tidy/test.rs | 32 +++++++++++++++++++ cpp-linter/src/cli/mod.rs | 15 +++++++++ cpp-linter/src/cli/structs.rs | 14 ++++++++ cpp-linter/src/rest_client.rs | 6 +++- 8 files changed, 98 insertions(+), 5 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs index f9c7f620..5845a9d5 100644 --- a/cpp-linter/src/clang_tools/clang_format.rs +++ b/cpp-linter/src/clang_tools/clang_format.rs @@ -454,6 +454,21 @@ mod tests { let err = run_clang_format(&mut file.lock().unwrap(), &clang_params).unwrap_err(); assert!(matches!(err, ClangCaptureError::WriteFileFailed { .. })); } + + #[test] + fn no_gen_patch() { + let tmp_workspace = setup_tmp_workspace(); + let tools_dir = tempfile::tempdir().unwrap(); + let fake_format = make_fake_tool(tools_dir.path(), "clang-format", "echo 'void main() {}'"); + let mut clang_params = make_clang_params(tmp_workspace.path(), fake_format); + clang_params.gen_patch = false; + let file = Mutex::new(FileObj::new(PathBuf::from("demo/demo.cpp"))); + let mut file = file.lock().unwrap(); + let _ = run_clang_format(&mut file, &clang_params).unwrap(); + assert!(file.patched_path.is_none()); + assert!(file.format_advice.is_some()); + assert!(!file.format_advice.as_ref().unwrap().replacements.is_empty()); + } } fn formalize_style(style: &str, expected: &str) { diff --git a/cpp-linter/src/clang_tools/clang_tidy/mod.rs b/cpp-linter/src/clang_tools/clang_tidy/mod.rs index a245e6e4..cc49ae54 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/mod.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/mod.rs @@ -738,7 +738,9 @@ pub fn run_clang_tidy( let mut paths = FixPaths::new(&target, &working_dir); let (patched, tidy_fixes) = apply_fixes(&fixes, &mut paths, &original_content, &mut notes, &mut logs); - file.patched_path = Some(write_cache(clang_params, &file.name, &patched)?); + if clang_params.gen_patch { + file.patched_path = Some(write_cache(clang_params, &file.name, &patched)?); + } file.tidy_advice = Some(TidyAdvice { notes }); Ok((logs, tidy_fixes)) } diff --git a/cpp-linter/src/clang_tools/clang_tidy/other_files/mod.rs b/cpp-linter/src/clang_tools/clang_tidy/other_files/mod.rs index da194c6a..6cfd920f 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/other_files/mod.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/other_files/mod.rs @@ -292,12 +292,14 @@ pub fn apply_other_file_fixes( }, ) in pending { - let content = analyzed.content(index, clang_params)?; let mut file = files[index] .lock() .map_err(|_| ClangCaptureError::MutexPoisoned)?; - let patched = patch_content(content, &replacements, &file.name, &mut logs[index]); - file.patched_path = Some(write_cache(clang_params, &file.name, &patched)?); + if clang_params.gen_patch { + let content = analyzed.content(index, clang_params)?; + let patched = patch_content(content, &replacements, &file.name, &mut logs[index]); + file.patched_path = Some(write_cache(clang_params, &file.name, &patched)?); + } let name = file.name.to_string_lossy().replace('\\', "/"); let advice = file .tidy_advice diff --git a/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs b/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs index d25c86b7..323ce9c3 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs @@ -114,6 +114,7 @@ async fn fix_analyzed_header() { tidy_checks: "-*,readability-non-const-parameter".to_string(), clang_tidy_command: Some(get_tidy_exe()), repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }); let files = as_files(vec![ @@ -170,6 +171,7 @@ fn not_analyzed() { let tmp = workspace(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let mut source = FileObj::new(PathBuf::from("fix.cpp")); @@ -241,6 +243,7 @@ fn outside_changed_lines() { let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), lines_changed_only: LinesChangedOnly::On, + gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -271,6 +274,7 @@ fn out_of_bounds() { let tmp = workspace(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -306,6 +310,7 @@ fn merge_with_own_fixes() { fs::write(tmp.path().join("other.cpp"), SOURCE).unwrap(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -355,6 +360,7 @@ fn remove_changed_line() { let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), lines_changed_only: LinesChangedOnly::On, + gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -425,6 +431,7 @@ fn analyzed_file_missing() { let tmp = workspace(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -485,6 +492,7 @@ fn overlapping_fixes() { fs::write(tmp.path().join("other.cpp"), SOURCE).unwrap(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -523,6 +531,7 @@ fn overlaps_own_fix() { let tmp = workspace(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let files = as_files(vec![ diff --git a/cpp-linter/src/clang_tools/clang_tidy/test.rs b/cpp-linter/src/clang_tools/clang_tidy/test.rs index ebcaf0ac..2bfa5e9a 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/test.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/test.rs @@ -144,6 +144,7 @@ fn use_extra_args() { clang_tidy_command: Some(exe_path), clang_format_command: None, repo_root: tmp_workspace.path().to_path_buf(), + gen_patch: false, }; let mut file_lock = arc_file.lock().unwrap(); fs::create_dir_all(clang_params.get_cache_path()).unwrap(); @@ -429,6 +430,7 @@ fn fixes_do_not_modify_worktree() { tidy_checks: "-*,readability-non-const-parameter".to_string(), clang_tidy_command: Some(get_tidy_exe()), repo_root: tmp_workspace.path().to_path_buf(), + gen_patch: true, ..Default::default() }; let file = Mutex::new(FileObj::new(PathBuf::from("fix.cpp"))); @@ -473,6 +475,36 @@ fn fixes_do_not_modify_worktree() { assert!(header_fix.path.ends_with("fix.h")); } +#[test] +fn fixes_not_cached_when_gen_patch_false() { + let tmp_workspace = tempfile::TempDir::with_prefix("cpp-linter-unit-tests_").unwrap(); + fs::write(tmp_workspace.path().join("fix.h"), FIX_HEADER).unwrap(); + fs::write(tmp_workspace.path().join("fix.cpp"), FIX_SOURCE).unwrap(); + let clang_params = ClangParams { + tidy_checks: "-*,readability-non-const-parameter".to_string(), + clang_tidy_command: Some(get_tidy_exe()), + repo_root: tmp_workspace.path().to_path_buf(), + gen_patch: false, + ..Default::default() + }; + let file = Mutex::new(FileObj::new(PathBuf::from("fix.cpp"))); + let mut file = file.lock().unwrap(); + let (_logs, fixes) = run_clang_tidy(&mut file, &clang_params).unwrap(); + + // patched_path is None because gen_patch is false + assert!(file.patched_path.is_none()); + + // but notes and advice are still populated + let notes = &file.tidy_advice.as_ref().unwrap().notes; + let note = notes + .iter() + .find(|n| n.diagnostic == "readability-non-const-parameter") + .unwrap(); + assert_eq!(note.fixed_lines, vec![2]); + assert_eq!(fixes.own.len(), 1); + assert_eq!(fixes.others.len(), 1); +} + fn dummy_fixes(target: &Path) -> String { let target = target.to_string_lossy(); format!( diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index b00c87a2..8e7a38f7 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -576,6 +576,21 @@ pub struct FeedbackOptions { help_heading = "Feedback options", ))] pub passive_reviews: bool, + + /// Set to `true` to generate a patch file of applied clang fixes. + /// + /// The path to the generated patch file is printed in the logs and + /// set as the output variable `fix-patch-path` in CI workflows. + #[cfg_attr(feature = "bin", arg( + short = 'F', + long, + default_value_t = false, + aliases = ["fix"], + action = ArgAction::Set, + value_parser = FalseyValueParser::new(), + help_heading = "Feedback options", + ))] + pub gen_patch: bool, } /// Parses a single value of the `--extensions` option. diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index 732261bb..9c2a91fb 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -222,6 +222,12 @@ pub struct ClangParams { /// /// A project-specific cache folder is created in this path. pub repo_root: PathBuf, + + /// Whether to generate a patch file of applied clang fixes. + /// + /// This should be asserted if posting PR reviews or generating a patch file + /// without posing a PR review. + pub gen_patch: bool, } impl ClangParams { @@ -283,6 +289,7 @@ impl From<&Cli> for ClangParams { tidy_filter, format_filter, repo_root, + gen_patch: args.feedback_options.pr_review || args.feedback_options.gen_patch, } } } @@ -321,6 +328,11 @@ pub struct FeedbackInput { /// The root of the repository, used to locate relative file paths in processing. pub repo_root: PathBuf, + + /// Whether to generate a patch file of applied clang fixes. + /// + /// Only used when `pr_review` is false. + pub gen_patch: bool, } #[cfg(feature = "bin")] @@ -338,6 +350,7 @@ impl From<&Cli> for FeedbackInput { repo_root: args.source_options.repo_root.clone(), summary_output_file: args.feedback_options.summary_output_file.clone(), sarif_file: args.feedback_options.sarif_file.clone(), + gen_patch: args.feedback_options.gen_patch, } } } @@ -356,6 +369,7 @@ impl Default for FeedbackInput { repo_root: PathBuf::from("."), summary_output_file: None, sarif_file: None, + gen_patch: false, } } } diff --git a/cpp-linter/src/rest_client.rs b/cpp-linter/src/rest_client.rs index c3399ba0..7724ef73 100644 --- a/cpp-linter/src/rest_client.rs +++ b/cpp-linter/src/rest_client.rs @@ -261,7 +261,7 @@ impl RestClient { } else { self.client.post_pr_review(&options).await?; } - } else { + } else if feedback_inputs.gen_patch { for file in files { let file = file .lock() @@ -480,6 +480,10 @@ impl RestClient { } line_buf.clear(); } + log::info!( + "Patch file written to {}", + auto_fix_patch_path.to_string_lossy().replace("\\", "/") + ); } Ok(()) } From c0f9dd1c1072a0a0b0ca328203a46775ffe9574e Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:58:15 -0700 Subject: [PATCH 2/4] fix: Respect `gen_patch` when caching clang-format output (#512) With patch generation disabled, clang-format still wrote a cached patched file and set `patched_path`, contradicting the expected behavior. Gate cache creation on `gen_patch` while continuing to collect format advice. - **Behavior:** `gen_patch = false` leaves `patched_path` unset; format advice remains available. - **Implementation:** Cache clang-format output only when patch generation is enabled. ```rust if clang_params.gen_patch { // write formatted output to cache and set patched_path } ``` --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: 2bndy5 <14963867+2bndy5@users.noreply.github.com> Co-authored-by: Brendan <2bndy5@gmail.com> --- cpp-linter/src/clang_tools/clang_format.rs | 36 ++++++++++--------- cpp-linter/src/clang_tools/clang_tidy/test.rs | 3 +- 2 files changed, 22 insertions(+), 17 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_format.rs b/cpp-linter/src/clang_tools/clang_format.rs index 5845a9d5..64051b33 100644 --- a/cpp-linter/src/clang_tools/clang_format.rs +++ b/cpp-linter/src/clang_tools/clang_format.rs @@ -186,20 +186,22 @@ pub fn run_clang_format( } else { // clang-tidy was not run on this file, // so just use the clang-format fixes as the patched content. - let cache_format_fixes = cache_path.join(&file.name); - fs::create_dir_all( - cache_format_fixes - .parent() - .ok_or(ClangCaptureError::UnknownCacheParentPath)?, - ) - .map_err(ClangCaptureError::MkDirFailed)?; - fs::write(&cache_format_fixes, &output.stdout).map_err(|e| { - ClangCaptureError::WriteFileFailed { - file_name: cache_format_fixes.to_string_lossy().to_string(), - source: e, - } - })?; - file.patched_path = Some(cache_format_fixes); + if clang_params.gen_patch { + let cache_format_fixes = cache_path.join(&file.name); + fs::create_dir_all( + cache_format_fixes + .parent() + .ok_or(ClangCaptureError::UnknownCacheParentPath)?, + ) + .map_err(ClangCaptureError::MkDirFailed)?; + fs::write(&cache_format_fixes, &output.stdout).map_err(|e| { + ClangCaptureError::WriteFileFailed { + file_name: cache_format_fixes.to_string_lossy().to_string(), + source: e, + } + })?; + file.patched_path = Some(cache_format_fixes); + } } file.format_advice = Some(format_advice); @@ -447,7 +449,8 @@ mod tests { let tmp_workspace = setup_tmp_workspace(); let tools_dir = tempfile::tempdir().unwrap(); let fake_format = make_fake_tool(tools_dir.path(), "clang-format", ""); - let clang_params = make_clang_params(tmp_workspace.path(), fake_format); + let mut clang_params = make_clang_params(tmp_workspace.path(), fake_format); + clang_params.gen_patch = true; // a directory where the patched file should be cached fs::create_dir_all(clang_params.get_cache_path().join("demo/demo.cpp")).unwrap(); let file = Mutex::new(FileObj::new(PathBuf::from("demo/demo.cpp"))); @@ -459,7 +462,8 @@ mod tests { fn no_gen_patch() { let tmp_workspace = setup_tmp_workspace(); let tools_dir = tempfile::tempdir().unwrap(); - let fake_format = make_fake_tool(tools_dir.path(), "clang-format", "echo 'void main() {}'"); + let fake_format = + make_fake_tool(tools_dir.path(), "clang-format", "echo 'void main() {}'"); let mut clang_params = make_clang_params(tmp_workspace.path(), fake_format); clang_params.gen_patch = false; let file = Mutex::new(FileObj::new(PathBuf::from("demo/demo.cpp"))); diff --git a/cpp-linter/src/clang_tools/clang_tidy/test.rs b/cpp-linter/src/clang_tools/clang_tidy/test.rs index 2bfa5e9a..7c51d1f6 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/test.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/test.rs @@ -379,7 +379,8 @@ fn tidy_cache_not_writable() { let tmp_workspace = crate::test_common::setup_tmp_workspace(); let tools_dir = tempfile::tempdir().unwrap(); let fake_tidy = crate::test_common::make_fake_tool(tools_dir.path(), "clang-tidy", ""); - let clang_params = make_clang_params(tmp_workspace.path(), Some(fake_tidy)); + let mut clang_params = make_clang_params(tmp_workspace.path(), Some(fake_tidy)); + clang_params.gen_patch = true; // a directory where the patched file should be cached fs::create_dir_all(clang_params.get_cache_path().join("demo/demo.cpp")).unwrap(); let file = Mutex::new(FileObj::new(PathBuf::from("demo/demo.cpp"))); From 7969b682027da1e998c2a0d04cf6474a2b255f48 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 20 Oct 2026 17:48:43 -0700 Subject: [PATCH 3/4] test: restore coverage and ensure patch is generated when expected. --- cpp-linter/src/cli/mod.rs | 4 +++- cpp-linter/tests/comments.rs | 9 ++++++++- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index 8e7a38f7..ef7e52e6 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -584,8 +584,10 @@ pub struct FeedbackOptions { #[cfg_attr(feature = "bin", arg( short = 'F', long, - default_value_t = false, aliases = ["fix"], + default_value_t = false, + default_missing_value = "true", + num_args = 0..=1, action = ArgAction::Set, value_parser = FalseyValueParser::new(), help_heading = "Feedback options", diff --git a/cpp-linter/tests/comments.rs b/cpp-linter/tests/comments.rs index 7e58de57..7f19db70 100644 --- a/cpp-linter/tests/comments.rs +++ b/cpp-linter/tests/comments.rs @@ -50,6 +50,7 @@ struct TestParams { pub fail_posting: bool, pub bad_existing_comments: bool, pub relative_out_files: bool, + pub gen_patch: bool, } impl Default for TestParams { @@ -65,6 +66,7 @@ impl Default for TestParams { fail_posting: false, bad_existing_comments: false, relative_out_files: false, + gen_patch: false, } } } @@ -278,6 +280,7 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { "--summary-output-file={}", summary_out_file_path.to_str().unwrap() ), + format!("--gen-patch={}", test_params.gen_patch), format!("--sarif-file={}", sarif_file_path.to_str().unwrap()), ]; if test_params.force_lgtm { @@ -310,7 +313,9 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { .trim() .to_string() }); - if let Some(patch_path_from_output) = patch_path_from_output { + if test_params.gen_patch + && let patch_path_from_output = patch_path_from_output.unwrap() + { // verify patch path exists and print its content println!("Patch path from GITHUB_OUTPUT: {patch_path_from_output}"); let patch_path = Path::new(&patch_path_from_output); @@ -390,6 +395,7 @@ async fn new_push_all_lines() { test_comment(&TestParams { lines_changed_only: LinesChangedOnly::Off, no_lgtm: false, + gen_patch: true, ..Default::default() }) .await; @@ -399,6 +405,7 @@ async fn new_push_all_lines() { async fn new_push_changed_lines() { test_comment(&TestParams { no_lgtm: false, + gen_patch: true, ..Default::default() }) .await; From e159c8d6a6108de69031700d5cef00111536c304 Mon Sep 17 00:00:00 2001 From: Brendan <2bndy5@gmail.com> Date: Tue, 20 Oct 2026 19:40:24 -0700 Subject: [PATCH 4/4] review changes only show "fix avail in patch" in comment when a patch was generated. add a test to cover a closing brace for a hard-to-reach path --- .../clang_tidy/other_files/test.rs | 24 ++++++++++++++++--- cpp-linter/src/rest_client.rs | 19 +++++++++++---- 2 files changed, 35 insertions(+), 8 deletions(-) diff --git a/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs b/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs index 323ce9c3..47ad8170 100644 --- a/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs +++ b/cpp-linter/src/clang_tools/clang_tidy/other_files/test.rs @@ -171,7 +171,6 @@ fn not_analyzed() { let tmp = workspace(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), - gen_patch: true, ..Default::default() }; let mut source = FileObj::new(PathBuf::from("fix.cpp")); @@ -243,7 +242,6 @@ fn outside_changed_lines() { let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), lines_changed_only: LinesChangedOnly::On, - gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -274,7 +272,6 @@ fn out_of_bounds() { let tmp = workspace(); let clang_params = ClangParams { repo_root: tmp.path().to_path_buf(), - gen_patch: true, ..Default::default() }; let files = as_files(vec![ @@ -351,6 +348,27 @@ fn merge_with_own_fixes() { assert_eq!(notes[0].fixed_lines, vec![2]); } +/// An accepted fix records its note, but writes no patch when `gen_patch` is false. +#[test] +fn accepted_fix_without_patch() { + let tmp = workspace(); + let clang_params = ClangParams { + repo_root: tmp.path().to_path_buf(), + gen_patch: false, + ..Default::default() + }; + let files = as_files(vec![ + FileObj::new(PathBuf::from("fix.cpp")), + FileObj::new(PathBuf::from("fix.h")), + ]); + let tidy_fixes = [(0, header_fixes(tmp.path()))]; + let mut logs = vec![vec![], vec![]]; + apply_other_file_fixes(&files, &tidy_fixes, &clang_params, &mut logs).unwrap(); + assert!(patched(&files[1]).is_none()); + assert!(messages(&logs[0])[0].starts_with("Applying the fix")); + assert_eq!(header_notes(&files[1]).len(), 1); +} + /// A fix that removes a whole changed line (including its line break) /// does not reach into the next line. #[test] diff --git a/cpp-linter/src/rest_client.rs b/cpp-linter/src/rest_client.rs index 7724ef73..fff5d545 100644 --- a/cpp-linter/src/rest_client.rs +++ b/cpp-linter/src/rest_client.rs @@ -147,6 +147,7 @@ impl RestClient { tidy_checks_failed, &clang_versions, None, + feedback_inputs.gen_patch || feedback_inputs.pr_review, )?; if feedback_inputs.step_summary { self.client.append_step_summary(&summary)?; @@ -280,6 +281,7 @@ impl RestClient { tidy_checks_failed, &clang_versions, Some(u16::MAX as u64), + feedback_inputs.gen_patch || feedback_inputs.pr_review, )?); } let options = ThreadCommentOptions { @@ -400,6 +402,7 @@ impl RestClient { tidy_checks_failed: u64, clang_versions: &ClangVersions, max_len: Option, + patch_exists: bool, ) -> Result { let mut comment = format!("{COMMENT_MARKER}# Cpp-Linter Report "); let mut remaining_length = @@ -429,6 +432,7 @@ impl RestClient { tidy_checks_failed, &tidy_version.to_string(), &mut remaining_length, + patch_exists, )?; } } else { @@ -541,6 +545,7 @@ fn make_tidy_comment( tidy_checks_failed: u64, version_used: &String, remaining_length: &mut u64, + patch_exists: bool, ) -> Result<(), ClientError> { let opener = format!( "\n
clang-tidy (v{version_used}) reports: {tidy_checks_failed} concern(s)\n\n" @@ -556,7 +561,7 @@ fn make_tidy_comment( for tidy_note in &tidy_advice.notes { let file_path = PathBuf::from(&tidy_note.filename); if file_path == file.name { - let uses_auto_fix = !tidy_note.fixed_lines.is_empty(); + let uses_auto_fix = !tidy_note.fixed_lines.is_empty() && patch_exists; let mut tmp_note = format!("- {}\n\n", tidy_note.filename); tmp_note.push_str(&format!( " {filename}:{line}:{cols}: {severity}: [{diagnostic}]{auto_fixable}{skipped_fixes}\n > {rationale}\n{concerned_code}", @@ -795,6 +800,7 @@ mod test { 1, &"1.2.3".to_string(), &mut remaining_length, + true, ) .unwrap(); assert!(comment.contains( @@ -876,7 +882,8 @@ mod test { #[test] fn comment_without_tidy_concerns() { let files = make_dummy_files("warning"); - let comment = RestClient::make_comment(&files, 1, 0, &dummy_versions(), None).unwrap(); + let comment = + RestClient::make_comment(&files, 1, 0, &dummy_versions(), None, false).unwrap(); assert!(comment.contains("clang-format (v1.2.3) reports")); assert!(!comment.contains("clang-tidy (v1.2.3) reports")); } @@ -954,15 +961,16 @@ mod test { let files = make_dummy_files("warning"); crate::test_common::poison_mutex(&files[0]); // poisoned while making the clang-format comment - assert!(RestClient::make_comment(&files, 1, 1, &dummy_versions(), None).is_err()); + assert!(RestClient::make_comment(&files, 1, 1, &dummy_versions(), None, false).is_err()); // poisoned while making the clang-tidy comment - assert!(RestClient::make_comment(&files, 0, 1, &dummy_versions(), None).is_err()); + assert!(RestClient::make_comment(&files, 0, 1, &dummy_versions(), None, false).is_err()); } #[test] fn comment_too_long_for_tidy_note() { let files = make_dummy_files("warning"); - let unlimited = RestClient::make_comment(&files, 0, 1, &dummy_versions(), None).unwrap(); + let unlimited = + RestClient::make_comment(&files, 0, 1, &dummy_versions(), None, false).unwrap(); assert!(unlimited.contains("A test dummy rationale")); // one byte short of the room needed for the clang-tidy note let limited = RestClient::make_comment( @@ -971,6 +979,7 @@ mod test { 1, &dummy_versions(), Some(unlimited.len() as u64 - 1), + false, ) .unwrap(); assert!(!limited.contains("A test dummy rationale"));