diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index c564d2c8..5c57ac11 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -563,19 +563,21 @@ pub struct FeedbackOptions { pub file_annotations: bool, /// Set to `true` to enable Pull Request reviews. + /// + /// v1's `--tidy-review` and `--format-review` are aliases of this option. + /// It can be given more than once; reviews are enabled if any value is `true`. #[cfg_attr(feature = "bin", arg( short = 'P', long, - default_value_t = false, default_missing_value = "true", num_args = 0..=1, - action = ArgAction::Set, + action = ArgAction::Append, value_parser = FalseyValueParser::new(), help_heading = "Feedback options", aliases = ["format-review", "tidy-review"], short_aliases = ['d', 'm'], ))] - pub pr_review: bool, + pub pr_review: Vec, /// Set to `true` to prevent Pull Request reviews from /// approving or requesting changes. @@ -636,6 +638,13 @@ pub struct FeedbackOptions { pub gen_patch_path: Option, } +impl FeedbackOptions { + /// Are Pull Request reviews enabled by any given `--pr-review` value (or its aliases)? + pub fn is_pr_review(&self) -> bool { + self.pr_review.iter().any(|enabled| *enabled) + } +} + /// Parses a single value of the `--extensions` option. /// /// Surrounding whitespace and a leading `.` are removed. @@ -722,6 +731,25 @@ mod test { assert!(Cli::try_parse_from(vec!["cpp-linter", "--mod-sys", "--no-mod-sys"]).is_err()); } + #[test] + fn pr_review_aliases_can_be_combined() { + for (args, expected) in [ + (vec![], false), + (vec!["-P"], true), + (vec!["--pr-review=false"], false), + (vec!["--tidy-review=true", "--format-review=false"], true), + ( + vec!["--tidy-review", "false", "--format-review", "false"], + false, + ), + (vec!["-d", "-m"], true), + (vec!["--format-review=false", "--pr-review"], true), + ] { + let cli = Cli::parse_from([vec!["cpp-linter"], args.clone()].concat()); + assert_eq!(cli.feedback_options.is_pr_review(), expected, "{args:?}"); + } + } + #[test] fn deprecated_jobs_takes_an_integer() { let args = Cli::parse_from(vec!["cpp-linter", "-j", "4"]); diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index cc3d5cca..e0738a16 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -299,7 +299,7 @@ impl From<&Cli> for ClangParams { tidy_filter, format_filter, repo_root: args.source_options.repo_root.clone(), - gen_patch: args.feedback_options.pr_review + gen_patch: args.feedback_options.is_pr_review() || args.feedback_options.gen_patch || args.feedback_options.gen_patch_path.is_some(), } @@ -360,7 +360,7 @@ impl From<&Cli> for FeedbackInput { step_summary: args.feedback_options.step_summary, thread_comments: args.feedback_options.thread_comments.clone(), file_annotations: args.feedback_options.file_annotations, - pr_review: args.feedback_options.pr_review, + pr_review: args.feedback_options.is_pr_review(), passive_reviews: args.feedback_options.passive_reviews, repo_root: args.source_options.repo_root.clone(), summary_output_file: args diff --git a/cpp-linter/src/run.rs b/cpp-linter/src/run.rs index 2949d7eb..f40651be 100644 --- a/cpp-linter/src/run.rs +++ b/cpp-linter/src/run.rs @@ -78,7 +78,7 @@ pub async fn run_main(args: Vec) -> Result<()> { ); } let is_pr = rest_api_client.is_pr(); - if !is_pr && cli.feedback_options.pr_review && rest_api_client.client_kind() == "local" { + if !is_pr && cli.feedback_options.is_pr_review() && rest_api_client.client_kind() == "local" { log::warn!("Pull request reviews are only posted when running in a supported CI platform."); } @@ -138,7 +138,7 @@ pub async fn run_main(args: Vec) -> Result<()> { FileObj::new(file_path) }) .collect(); - if is_pr && cli.feedback_options.pr_review { + if is_pr && cli.feedback_options.is_pr_review() { let changed_files = rest_api_client .get_list_of_changed_files( &file_filter,