Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions cpp-linter/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>,

/// Set this option to false to disable the use of
/// file annotations as feedback.
#[cfg_attr(feature = "bin", arg(
Expand Down
5 changes: 5 additions & 0 deletions cpp-linter/src/cli/structs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<PathBuf>,

/// Whether to post file annotations.
pub file_annotations: bool,

Expand Down Expand Up @@ -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(),
}
}
}
Expand All @@ -346,6 +350,7 @@ impl Default for FeedbackInput {
pr_review: false,
passive_reviews: false,
repo_root: PathBuf::from("."),
summary_output_file: None,
}
}
}
Expand Down
1 change: 1 addition & 0 deletions cpp-linter/src/common_fs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<P: AsRef<Path>>(path: P) -> Result<PathBuf, std::io::Error> {
let abs_path = path.as_ref().canonicalize()?;
let abs_path_str = abs_path.to_string_lossy();
Expand Down
21 changes: 21 additions & 0 deletions cpp-linter/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
25 changes: 23 additions & 2 deletions cpp-linter/src/rest_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,15 +127,36 @@ 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,
tidy_checks_failed,
&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)
};
Comment thread
coderabbitai[bot] marked this conversation as resolved.
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 = [
Expand Down
35 changes: 34 additions & 1 deletion cpp-linter/tests/comments.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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,
Expand All @@ -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 {
Expand All @@ -58,6 +62,7 @@ impl Default for TestParams {
fail_dismissal: false,
fail_posting: false,
bad_existing_comments: false,
relative_summary_out_file: false,
}
}
}
Expand Down Expand Up @@ -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(),
Expand All @@ -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());
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
}
6 changes: 4 additions & 2 deletions docs/cli.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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'
Loading