diff --git a/cpp-linter/src/cli/mod.rs b/cpp-linter/src/cli/mod.rs index 3464abc9..d2f85129 100644 --- a/cpp-linter/src/cli/mod.rs +++ b/cpp-linter/src/cli/mod.rs @@ -494,6 +494,19 @@ pub struct FeedbackOptions { ))] pub step_summary: bool, + /// Provide this option with a path to which a summary comment is written. + /// + /// 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", + ))] + pub summary_output_file: Option, + /// Set this option to false to disable the use of /// file annotations as feedback. #[cfg_attr(feature = "bin", arg( diff --git a/cpp-linter/src/cli/structs.rs b/cpp-linter/src/cli/structs.rs index c997e76f..e2e1be18 100644 --- a/cpp-linter/src/cli/structs.rs +++ b/cpp-linter/src/cli/structs.rs @@ -299,6 +299,9 @@ pub struct FeedbackInput { /// Whether to post a step summary comment. pub step_summary: bool, + /// An optional file path to which a summary comment is written. + pub summary_output_file: Option, + /// Whether to post file annotations. pub file_annotations: bool, @@ -330,6 +333,7 @@ impl From<&Cli> for FeedbackInput { pr_review: args.feedback_options.pr_review, passive_reviews: args.feedback_options.passive_reviews, repo_root: args.source_options.repo_root.clone(), + summary_output_file: args.feedback_options.summary_output_file.clone(), } } } @@ -346,6 +350,7 @@ impl Default for FeedbackInput { pr_review: false, passive_reviews: false, repo_root: PathBuf::from("."), + summary_output_file: None, } } } diff --git a/cpp-linter/src/common_fs.rs b/cpp-linter/src/common_fs.rs index de8c73df..01b01a9a 100644 --- a/cpp-linter/src/common_fs.rs +++ b/cpp-linter/src/common_fs.rs @@ -391,6 +391,7 @@ impl FileObj { /// This function can error if the given path fails to be canonicalized. /// This may happen if the path does not exist or a non-final path /// component is not a directory. +#[cfg(any(test, feature = "bin"))] pub(crate) fn mk_path_abs>(path: P) -> Result { let abs_path = path.as_ref().canonicalize()?; let abs_path_str = abs_path.to_string_lossy(); diff --git a/cpp-linter/src/error.rs b/cpp-linter/src/error.rs index 6b1c45ee..d934ff7b 100644 --- a/cpp-linter/src/error.rs +++ b/cpp-linter/src/error.rs @@ -49,6 +49,27 @@ pub enum ClientError { /// Error to propagate a [`FileObjError`] encountered during file processing. #[error(transparent)] FileObjError(#[from] FileObjError), + + /// Error when failing to write a given summary output file. + #[error("Failed to write summary comment to file '{file_path:?}': {source}")] + SummaryOutputFileWriteFailed { + /// The path to the summary output file that failed to be written. + file_path: std::path::PathBuf, + + /// The underlying error from trying to write the summary output file. + #[source] + source: std::io::Error, + }, + + /// Error when failing to determine the parent directory of a given file path. + #[error("Failed to create a parent directory for the path '{file_path:?}'")] + MkDirFailed { + /// The path to the summary output file that failed to be written. + file_path: std::path::PathBuf, + /// The underlying error from trying to create the parent directory. + #[source] + source: std::io::Error, + }, } /// Errors related to invoking clang tools and processing their output. diff --git a/cpp-linter/src/rest_client.rs b/cpp-linter/src/rest_client.rs index b75fe390..a710759b 100644 --- a/cpp-linter/src/rest_client.rs +++ b/cpp-linter/src/rest_client.rs @@ -127,7 +127,7 @@ impl RestClient { let annotations = Self::make_annotations(files, &feedback_inputs.style)?; self.client.write_file_annotations(&annotations)?; } - if feedback_inputs.step_summary { + if feedback_inputs.step_summary || feedback_inputs.summary_output_file.is_some() { let summary = Self::make_comment( files, format_checks_failed, @@ -135,7 +135,28 @@ impl RestClient { &clang_versions, None, )?; - self.client.append_step_summary(&summary)?; + if feedback_inputs.step_summary { + self.client.append_step_summary(&summary)?; + } + if let Some(summary_output_file) = feedback_inputs.summary_output_file { + let output_file = if summary_output_file.is_absolute() { + summary_output_file + } else { + feedback_inputs.repo_root.join(&summary_output_file) + }; + if let Some(parent) = output_file.parent() { + std::fs::create_dir_all(parent).map_err(|e| ClientError::MkDirFailed { + file_path: output_file.clone(), + source: e, + })?; + } + std::fs::write(&output_file, &summary).map_err(|e| { + ClientError::SummaryOutputFileWriteFailed { + file_path: output_file, + source: e, + } + })?; + } comment = Some(summary); } let output_vars = [ diff --git a/cpp-linter/tests/comments.rs b/cpp-linter/tests/comments.rs index 746f2840..4ee3fbbb 100644 --- a/cpp-linter/tests/comments.rs +++ b/cpp-linter/tests/comments.rs @@ -1,7 +1,8 @@ #![cfg(feature = "bin")] use chrono::Utc; -use cpp_linter::cli::ThreadComments; +use cpp_linter::rest_client::USER_OUTREACH; use cpp_linter::run::run_main; +use cpp_linter::{cli::ThreadComments, rest_client::COMMENT_MARKER}; use git_bot_feedback::LinesChangedOnly; use mockito::Matcher; use std::{env, fmt::Display, fs, io::Write, path::Path}; @@ -19,6 +20,8 @@ const MOCK_ASSETS_PATH: &str = "tests/comment_test_assets/"; const RESET_RATE_LIMIT_HEADER: &str = "x-ratelimit-reset"; const REMAINING_RATE_LIMIT_HEADER: &str = "x-ratelimit-remaining"; +const SUMMARY_OUT_FILE_NAME: &str = "summary_output.md"; + #[derive(PartialEq, Clone, Copy, Debug)] enum EventType { Push, @@ -44,6 +47,7 @@ struct TestParams { pub fail_dismissal: bool, pub fail_posting: bool, pub bad_existing_comments: bool, + pub relative_summary_out_file: bool, } impl Default for TestParams { @@ -58,6 +62,7 @@ impl Default for TestParams { fail_dismissal: false, fail_posting: false, bad_existing_comments: false, + relative_summary_out_file: false, } } } @@ -242,6 +247,12 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { ); } + let summary_out_file_path = if test_params.relative_summary_out_file { + std::path::PathBuf::from(SUMMARY_OUT_FILE_NAME) + } else { + tmp_dir.path().join(SUMMARY_OUT_FILE_NAME) + }; + let mut args = vec![ "cpp-linter".to_string(), "-v=debug".to_string(), @@ -254,6 +265,10 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { "-p=build".to_string(), "-i=build/**".to_string(), format!("--repo-root={}", tmp_dir.path().to_str().unwrap()), + format!( + "--summary-output-file={}", + summary_out_file_path.to_str().unwrap() + ), ]; if test_params.force_lgtm { args.push("-e=c".to_string()); @@ -297,6 +312,15 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) { std::fs::read_to_string(patch_path).expect("Failed to read generated patch file."); println!("Generated patch content:\n{patch_content}"); } + + let summary_out_file_abs_path = if test_params.relative_summary_out_file { + tmp_dir.path().join(SUMMARY_OUT_FILE_NAME) + } else { + summary_out_file_path.clone() + }; + let summary_content = std::fs::read_to_string(&summary_out_file_abs_path).unwrap(); + assert!(summary_content.contains(COMMENT_MARKER)); + assert!(summary_content.contains(USER_OUTREACH)); } async fn test_comment(test_params: &TestParams) { @@ -473,3 +497,12 @@ async fn bad_existing_comments() { }) .await; } + +#[tokio::test] +async fn relative_summary_out_file() { + test_comment(&TestParams { + relative_summary_out_file: true, + ..Default::default() + }) + .await; +} diff --git a/docs/cli.yml b/docs/cli.yml index 7f9bb3be..a7184f12 100644 --- a/docs/cli.yml +++ b/docs/cli.yml @@ -31,6 +31,8 @@ inputs: minimum-version: '1.6.1' step-summary: minimum-version: '1.6.0' + summary-output-file: + minimum-version: '1.13.0' file-annotations: minimum-version: '1.4.3' database: @@ -47,8 +49,6 @@ inputs: passive-reviews: minimum-version: '1.10.0' required-permission: 'pull-requests: write #pull-request-reviews' - jobs: - minimum-version: '1.8.1' diff-base: minimum-version: '1.12.0' ignore-index: @@ -60,3 +60,5 @@ outputs: minimum-version: '1.6.2' clang-format-checks-failed: minimum-version: '1.6.2' + fix-patch-path: + minimum-version: '2.0.0'