feat: add --fix as alias to new --gen-patch/-F arg

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,13 +449,30 @@ 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")));
             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) {

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))
 }

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

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![
@@ -306,6 +307,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![
@@ -346,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]
@@ -355,6 +378,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 +449,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 +510,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 +549,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![

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();
@@ -378,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")));
@@ -429,6 +431,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 +476,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!(

cpp-linter/src/cli/mod.rs

@@ -576,6 +576,23 @@ 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,
+        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",
+    ))]
+    pub gen_patch: bool,
 }
 
 /// Parses a single value of the `--extensions` option.

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,
         }
     }
 }