feat: add --gen-patch-path to choose where the patch is written

Cargo.toml

@@ -21,7 +21,7 @@ strip = "symbols"
 
 [workspace.dependencies]
 anyhow = "1.0.104"
-clap = { version = "4.6.7", features = ["derive"] }
+clap = { version = "4.6.7", features = ["derive", "env"] }
 colored = "3.1.1"
 log = { version = "0.4.34", features = ["std"] }
 mockito = "1.7.2"

cpp-linter/src/cli/mod.rs

@@ -8,9 +8,7 @@ use std::str::FromStr;
 #[cfg(feature = "bin")]
 use clang_tools_manager::{RequestedVersion, logger::CLI_HELP_STYLE};
 #[cfg(feature = "bin")]
-use clap::{
-    ArgAction, Args, Parser, Subcommand, ValueEnum, builder::FalseyValueParser, value_parser,
-};
+use clap::{ArgAction, Args, Parser, Subcommand, ValueEnum, builder::FalseyValueParser};
 
 mod structs;
 pub use structs::{ClangParams, FeedbackInput, LinesChangedOnly, ThreadComments};
@@ -410,13 +408,15 @@ pub struct TidyOptions {
     /// attempted through all parent paths of the first input file. See [LLVM docs about
     /// setup tooling](https://clang.llvm.org/docs/HowToSetupToolingForLLVM.html)
     /// for an example of setting up Clang Tooling on a source tree.
-    #[cfg_attr(feature = "bin", arg(
-        short = 'p',
-        long,
-        value_name = "PATH",
-        value_parser = value_parser!(PathBuf),
-        help_heading = "Clang-tidy options",
-    ))]
+    #[cfg_attr(
+        feature = "bin",
+        arg(
+            short = 'p',
+            long,
+            value_name = "PATH",
+            help_heading = "Clang-tidy options",
+        )
+    )]
     pub database: Option<PathBuf>,
 
     /// A string of extra arguments passed to clang-tidy for use as compiler arguments.
@@ -502,13 +502,15 @@ pub struct FeedbackOptions {
     ///
     /// If the given path is relative, then it shall be relative to the given
     /// [`--repo-root`](#-r-repo-root) path.
-    #[cfg_attr(feature = "bin", arg(
-        short = 'o',
-        long,
-        value_name = "PATH",
-        value_parser = value_parser!(PathBuf),
-        help_heading = "Feedback options",
-    ))]
+    #[cfg_attr(
+        feature = "bin",
+        arg(
+            short = 'o',
+            long,
+            value_name = "PATH",
+            help_heading = "Feedback options",
+        )
+    )]
     pub summary_output_file: Option<PathBuf>,
 
     /// Provide this option with a path to which clang-tidy diagnostics are
@@ -528,12 +530,10 @@ pub struct FeedbackOptions {
     /// > In GitHub Actions, the SARIF file can be uploaded to
     /// > [GitHub code scanning](https://docs.github.com/en/code-security/code-scanning/integrating-with-code-scanning/uploading-a-sarif-file-to-github)
     /// > with the `github/codeql-action/upload-sarif` action.
-    #[cfg_attr(feature = "bin", arg(
-        long,
-        value_name = "PATH",
-        value_parser = value_parser!(PathBuf),
-        help_heading = "Feedback options",
-    ))]
+    #[cfg_attr(
+        feature = "bin",
+        arg(long, value_name = "PATH", help_heading = "Feedback options",)
+    )]
     pub sarif_file: Option<PathBuf>,
 
     /// Set this option to false to disable the use of
@@ -593,6 +593,33 @@ pub struct FeedbackOptions {
         help_heading = "Feedback options",
     ))]
     pub gen_patch: bool,
+
+    /// Write the patch file of applied clang fixes to this path.
+    ///
+    /// Setting this implies [`--gen-patch`](#-f-gen-patch).
+    /// A relative path is relative to the current working directory.
+    /// If not set, the patch is written to `.cpp-linter-cache/auto-fix.patch`
+    /// in the [`--repo-root`](#-r-repo-root) folder.
+    ///
+    /// This can also be set with the `GEN_PATCH_PATH` environment variable,
+    /// so the same variable can be used to apply the patch afterward:
+    ///
+    /// ```shell
+    /// export GEN_PATCH_PATH=fixes.patch
+    /// cpp-linter
+    /// git apply "$GEN_PATCH_PATH"
+    /// ```
+    #[cfg_attr(
+        feature = "bin",
+        arg(
+            long,
+            value_name = "PATH",
+            env = "GEN_PATCH_PATH",
+            help_heading = "Feedback options",
+            verbatim_doc_comment,
+        )
+    )]
+    pub gen_patch_path: Option<PathBuf>,
 }
 
 /// Parses a single value of the `--extensions` option.

cpp-linter/src/cli/structs.rs

@@ -1,4 +1,7 @@
-use std::{fmt::Display, path::PathBuf};
+use std::{
+    fmt::Display,
+    path::{Path, PathBuf},
+};
 
 #[cfg(feature = "bin")]
 use clap::{ValueEnum, builder::PossibleValue};
@@ -237,6 +240,11 @@ impl ClangParams {
     /// The file name for aggregating auto-fixes into a unified patch.
     pub(crate) const AUTO_FIX_PATCH: &str = "auto-fix.patch";
 
+    /// The default path of the generated patch file, in the cache directory of `repo_root`.
+    pub(crate) fn default_patch_path(repo_root: &Path) -> PathBuf {
+        repo_root.join(Self::CACHE_DIR).join(Self::AUTO_FIX_PATCH)
+    }
+
     pub(crate) fn get_cache_path(&self) -> PathBuf {
         self.repo_root.join(Self::CACHE_DIR).join("patched")
     }
@@ -289,7 +297,9 @@ impl From<&Cli> for ClangParams {
             tidy_filter,
             format_filter,
             repo_root,
-            gen_patch: args.feedback_options.pr_review || args.feedback_options.gen_patch,
+            gen_patch: args.feedback_options.pr_review
+                || args.feedback_options.gen_patch
+                || args.feedback_options.gen_patch_path.is_some(),
         }
     }
 }
@@ -333,6 +343,9 @@ pub struct FeedbackInput {
     ///
     /// Only used when `pr_review` is false.
     pub gen_patch: bool,
+
+    /// The path of the generated patch file.
+    pub patch_path: PathBuf,
 }
 
 #[cfg(feature = "bin")]
@@ -350,7 +363,12 @@ 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,
+            gen_patch: args.feedback_options.gen_patch
+                || args.feedback_options.gen_patch_path.is_some(),
+            patch_path: match &args.feedback_options.gen_patch_path {
+                Some(path) => std::path::absolute(path).unwrap_or_else(|_| path.to_path_buf()),
+                None => ClangParams::default_patch_path(&args.source_options.repo_root),
+            },
         }
     }
 }
@@ -370,6 +388,7 @@ impl Default for FeedbackInput {
             summary_output_file: None,
             sarif_file: None,
             gen_patch: false,
+            patch_path: ClangParams::default_patch_path(Path::new(".")),
         }
     }
 }
@@ -380,7 +399,9 @@ mod test {
 
     use clap::{Parser, ValueEnum};
 
-    use super::{ClangParams, Cli, LinesChangedOnly, ThreadComments};
+    use std::path::PathBuf;
+
+    use super::{ClangParams, Cli, FeedbackInput, LinesChangedOnly, ThreadComments};
 
     #[test]
     fn parse_positional() {
@@ -443,6 +464,45 @@ mod test {
         }
     }
 
+    #[test]
+    fn default_patch_path() {
+        let cli = Cli::parse_from(["cpp-linter", "--repo-root", "sub"]);
+        let feedback_input = FeedbackInput::from(&cli);
+        assert!(!feedback_input.gen_patch);
+        assert_eq!(
+            feedback_input.patch_path,
+            PathBuf::from("sub/.cpp-linter-cache/auto-fix.patch")
+        );
+    }
+
+    #[test]
+    fn gen_patch_path_implies_gen_patch() {
+        let cli = Cli::parse_from(["cpp-linter", "--gen-patch-path", "fixes.patch"]);
+        let feedback_input = FeedbackInput::from(&cli);
+        assert!(feedback_input.gen_patch);
+        // a relative path is relative to the working directory
+        assert_eq!(
+            feedback_input.patch_path,
+            std::env::current_dir().unwrap().join("fixes.patch")
+        );
+        assert!(ClangParams::from(&cli).gen_patch);
+    }
+
+    #[test]
+    fn gen_patch_path_from_env() {
+        unsafe {
+            std::env::set_var("GEN_PATCH_PATH", "from-env.patch");
+        }
+        let cli = Cli::parse_from(["cpp-linter"]);
+        unsafe {
+            std::env::remove_var("GEN_PATCH_PATH");
+        }
+        assert_eq!(
+            cli.feedback_options.gen_patch_path,
+            Some(PathBuf::from("from-env.patch"))
+        );
+    }
+
     #[test]
     fn absolute_db_path() {
         let tmp_dir = tempfile::tempdir().unwrap();

cpp-linter/src/common_fs.rs

@@ -17,7 +17,7 @@ use crate::{
     clang_tools::{
         ReviewComments, Suggestion, clang_format::FormatAdvice, clang_tidy::TidyAdvice, make_patch,
     },
-    cli::{ClangParams, LinesChangedOnly},
+    cli::LinesChangedOnly,
     error::FileObjError,
 };
 
@@ -169,7 +169,11 @@ impl FileObj {
     /// a PR review is not being posted. Both function have to create a patch by
     /// reading the original file and patched file (in cache), but
     /// [`FileObj::make_suggestions_from_patch()`] does more with the diff than this function.
-    pub fn maybe_append_patch(&self, repo_root: &Path) -> Result<(), FileObjError> {
+    pub fn maybe_append_patch(
+        &self,
+        repo_root: &Path,
+        patch_path: &Path,
+    ) -> Result<(), FileObjError> {
         let patched = match &self.patched_path {
             Some(patched_path) if patched_path.exists() => {
                 fs::read_to_string(patched_path).map_err(FileObjError::ReadFile)?
@@ -180,29 +184,28 @@ impl FileObj {
             fs::read_to_string(repo_root.join(&self.name)).map_err(FileObjError::ReadFile)?;
         let (diff, input) = make_patch(patched.as_str(), &original_content);
         let file_name = self.name.to_string_lossy().replace("\\", "/");
-        Self::append_patch(&file_name, &input, &diff, repo_root)?;
+        Self::append_patch(&file_name, &input, &diff, patch_path)?;
         Ok(())
     }
 
-    /// write fixes to a unified patch file in the cache directory.
+    /// write fixes to the unified patch file at `patch_path`.
     fn append_patch(
         file_name: &str,
         input: &InternedInput<&str>,
         diff: &Diff,
-        repo_root: &Path,
+        patch_path: &Path,
     ) -> Result<(), FileObjError> {
         let printer = BasicLineDiffPrinter(&input.interner);
         let diff_config = UnifiedDiffConfig::default();
         let unified_diff = diff.unified_diff(&printer, diff_config, input).to_string();
         if !unified_diff.is_empty() {
-            let patch_path_parent = repo_root.join(ClangParams::CACHE_DIR);
-            fs::create_dir_all(&patch_path_parent).map_err(FileObjError::MkDirFailed)?;
-            let patch_file_path = patch_path_parent.join(ClangParams::AUTO_FIX_PATCH);
+            let patch_path_parent = patch_path.parent().unwrap_or(Path::new(""));
+            fs::create_dir_all(patch_path_parent).map_err(FileObjError::MkDirFailed)?;
             let mut patch_file = fs::OpenOptions::new()
                 .append(true)
                 .create(true)
                 .truncate(false)
-                .open(&patch_file_path)
+                .open(patch_path)
                 .map_err(FileObjError::OpenPatchFileFailed)?;
             patch_file
                 .write_all(
@@ -224,6 +227,7 @@ impl FileObj {
         review_comments: &mut ReviewComments,
         summary_only: bool,
         repo_root: &Path,
+        patch_path: &Path,
     ) -> Result<(), FileObjError> {
         let patched = match &self.patched_path {
             Some(patched_path) if patched_path.exists() => {
@@ -235,7 +239,7 @@ impl FileObj {
             fs::read_to_string(repo_root.join(&self.name)).map_err(FileObjError::ReadFile)?;
         let (diff, input) = make_patch(patched.as_str(), &original_content);
         let file_name = self.name.to_string_lossy().replace("\\", "/");
-        Self::append_patch(&file_name, &input, &diff, repo_root)?;
+        Self::append_patch(&file_name, &input, &diff, patch_path)?;
 
         self.get_suggestions(review_comments, &diff, &input, summary_only)
             .map_err(FileObjError::DisplayStringFailed)?;
@@ -425,7 +429,10 @@ mod test {
     use tempfile::{NamedTempFile, TempDir};
 
     use super::FileObj;
-    use crate::{clang_tools::ReviewComments, cli::LinesChangedOnly};
+    use crate::{
+        clang_tools::ReviewComments,
+        cli::{ClangParams, LinesChangedOnly},
+    };
 
     // *********************** tests for FileObj::get_ranges()
 
@@ -508,7 +515,12 @@ mod test {
 
         let mut review_comments = ReviewComments::default();
         file_obj
-            .make_suggestions_from_patch(&mut review_comments, false, repo_root.path())
+            .make_suggestions_from_patch(
+                &mut review_comments,
+                false,
+                repo_root.path(),
+                &ClangParams::default_patch_path(repo_root.path()),
+            )
             .unwrap();
 
         assert_eq!(review_comments.comments.len(), 1);
@@ -568,7 +580,12 @@ mod test {
 
         let mut review_comments = ReviewComments::default();
         file_obj
-            .make_suggestions_from_patch(&mut review_comments, false, repo_root.path())
+            .make_suggestions_from_patch(
+                &mut review_comments,
+                false,
+                repo_root.path(),
+                &ClangParams::default_patch_path(repo_root.path()),
+            )
             .unwrap();
         assert_eq!(review_comments.comments.len(), 2);
         // 1 fix and 2 unfixed diagnostics, all within the diff (in 2 comments)
@@ -599,7 +616,12 @@ mod test {
 
         // Processing the same file again merges duplicate suggestions
         file_obj
-            .make_suggestions_from_patch(&mut review_comments, false, repo_root.path())
+            .make_suggestions_from_patch(
+                &mut review_comments,
+                false,
+                repo_root.path(),
+                &ClangParams::default_patch_path(repo_root.path()),
+            )
             .unwrap();
         assert_eq!(review_comments.comments.len(), 2);
         assert_eq!(
@@ -613,7 +635,12 @@ mod test {
         // Only counts the unfixed diagnostics in summary-only mode
         let mut summary_comments = ReviewComments::default();
         file_obj
-            .make_suggestions_from_patch(&mut summary_comments, true, repo_root.path())
+            .make_suggestions_from_patch(
+                &mut summary_comments,
+                true,
+                repo_root.path(),
+                &ClangParams::default_patch_path(repo_root.path()),
+            )
             .unwrap();
         assert!(summary_comments.comments.is_empty());
         assert_eq!(summary_comments.tool_total, 3);