Skip to content

Commit da846d7

Browse files
committed
feat: add fix-patch-path output variable
resolves #376 If there are any auto-fixes available, then a patch full of them is created and its path is set to a new output `fix-patch-path` output variable. If there were no auto-fixes, then the output variable is not set, which is treated as an empty in `steps.linter.outputs.fix-patch-path` context).
1 parent f352be2 commit da846d7

6 files changed

Lines changed: 98 additions & 5 deletions

File tree

cpp-linter/src/cli/structs.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -233,6 +233,10 @@ pub struct ClangParams {
233233
impl ClangParams {
234234
/// The directory name to use for caching clang-tidy and clang-format results.
235235
pub(crate) const CACHE_DIR: &str = ".cpp-linter-cache";
236+
237+
/// The file name for aggregating auto-fixes into a unified patch.
238+
pub(crate) const AUTO_FIX_PATCH: &str = "auto_fix.patch";
239+
236240
pub(crate) fn get_cache_path(&self) -> PathBuf {
237241
self.repo_root.join(Self::CACHE_DIR).join("patched")
238242
}

cpp-linter/src/common_fs.rs

Lines changed: 55 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
use std::{
44
fmt::Debug,
55
fs,
6+
io::Write,
67
num::NonZeroU32,
78
ops::RangeInclusive,
89
path::{Path, PathBuf},
@@ -16,7 +17,7 @@ use crate::{
1617
clang_tools::{
1718
ReviewComments, Suggestion, clang_format::FormatAdvice, clang_tidy::TidyAdvice, make_patch,
1819
},
19-
cli::LinesChangedOnly,
20+
cli::{ClangParams, LinesChangedOnly},
2021
error::FileObjError,
2122
};
2223

@@ -162,6 +163,57 @@ impl FileObj {
162163
false
163164
}
164165

166+
/// If the file has a cached fixes, then append them to a unified patched file.
167+
///
168+
/// This is the alternative to [`FileObj::make_suggestions_from_patch()`] when
169+
/// a PR review is not being posted. Both function have to create a patch by
170+
/// reading the original file and patched file (in cache), but
171+
/// [`FileObj::make_suggestions_from_patch()`] does more with the diff than this function.
172+
pub fn maybe_append_patch(&self, repo_root: &Path) -> Result<(), FileObjError> {
173+
let patched = match &self.patched_path {
174+
Some(patched_path) if patched_path.exists() => {
175+
fs::read_to_string(patched_path).map_err(FileObjError::ReadFile)?
176+
}
177+
_ => return Ok(()),
178+
};
179+
let original_content =
180+
fs::read_to_string(repo_root.join(&self.name)).map_err(FileObjError::ReadFile)?;
181+
let (diff, input) = make_patch(patched.as_str(), &original_content);
182+
let file_name = self.name.to_string_lossy().replace("\\", "/");
183+
Self::append_patch(&file_name, &input, &diff, repo_root)?;
184+
Ok(())
185+
}
186+
187+
/// write fixes to a unified patch file in the cache directory.
188+
fn append_patch(
189+
file_name: &str,
190+
input: &InternedInput<&str>,
191+
diff: &Diff,
192+
repo_root: &Path,
193+
) -> Result<(), FileObjError> {
194+
let printer = BasicLineDiffPrinter(&input.interner);
195+
let mut diff_config = UnifiedDiffConfig::default();
196+
diff_config.context_len(0);
197+
let unified_diff = diff.unified_diff(&printer, diff_config, input).to_string();
198+
if !unified_diff.is_empty() {
199+
let patch_path_parent = repo_root.join(ClangParams::CACHE_DIR);
200+
fs::create_dir_all(&patch_path_parent).map_err(FileObjError::MkDirFailed)?;
201+
let patch_file_path = patch_path_parent.join(ClangParams::AUTO_FIX_PATCH);
202+
let mut patch_file = fs::OpenOptions::new()
203+
.append(true)
204+
.create(true)
205+
.truncate(false)
206+
.open(&patch_file_path)
207+
.map_err(FileObjError::OpenPatchFileFailed)?;
208+
patch_file
209+
.write_all(
210+
format!("--- a/{file_name}\n+++ b/{file_name}\n{unified_diff}",).as_bytes(),
211+
)
212+
.map_err(FileObjError::WritePatchFailed)?;
213+
}
214+
Ok(())
215+
}
216+
165217
/// Create a list of [`Suggestion`](struct@crate::clang_tools::Suggestion) from a
166218
/// generated diff and store them in the given
167219
/// [`ReviewComments`](struct@crate::clang_tools::ReviewComments).
@@ -183,7 +235,8 @@ impl FileObj {
183235
let original_content =
184236
fs::read_to_string(repo_root.join(&self.name)).map_err(FileObjError::ReadFile)?;
185237
let (diff, input) = make_patch(patched.as_str(), &original_content);
186-
let file_name = self.name.to_str().unwrap_or_default().replace("\\", "/");
238+
let file_name = self.name.to_string_lossy().replace("\\", "/");
239+
Self::append_patch(&file_name, &input, &diff, repo_root)?;
187240

188241
self.get_suggestions(review_comments, &diff, &input, summary_only)
189242
.map_err(FileObjError::DisplayStringFailed)?;

cpp-linter/src/error.rs

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,18 @@ pub enum FileObjError {
1717
/// Error when failing to generate a patch for a file.
1818
#[error("Failed to print a hunk to a string buffer: {0}")]
1919
DisplayStringFailed(#[source] std::fmt::Error),
20+
21+
/// Error when failing to create the cache directory for the patch file.
22+
#[error("Failed to create cache directory for the patch file: {0}")]
23+
MkDirFailed(#[source] std::io::Error),
24+
25+
/// Error when failing to open the patch file for writing.
26+
#[error("Failed to open patch file for writing: {0}")]
27+
OpenPatchFileFailed(#[source] std::io::Error),
28+
29+
/// Error when failing to write to the patch file.
30+
#[error("Failed to write to the patch file: {0}")]
31+
WritePatchFailed(#[source] std::io::Error),
2032
}
2133

2234
/// Errors related to the REST client used for posting feedback and special logging.

cpp-linter/src/rest_client.rs

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ use crate::{
1818
clang_format::{summarize_style, tally_format_advice},
1919
clang_tidy::tally_tidy_advice,
2020
},
21-
cli::{FeedbackInput, ThreadComments},
21+
cli::{ClangParams, FeedbackInput, ThreadComments},
2222
common_fs::FileObj,
2323
error::ClientError,
2424
};
@@ -232,6 +232,23 @@ impl RestClient {
232232
summary_only,
233233
);
234234
self.client.post_pr_review(&options).await?;
235+
} else {
236+
for file in files {
237+
let file = file
238+
.lock()
239+
.map_err(|e| ClientError::MutexPoisoned(e.to_string()))?;
240+
file.maybe_append_patch(&feedback_inputs.repo_root)?;
241+
}
242+
}
243+
let auto_fix_patch_patch = feedback_inputs
244+
.repo_root
245+
.join(ClangParams::CACHE_DIR)
246+
.join(ClangParams::AUTO_FIX_PATCH);
247+
if auto_fix_patch_patch.exists() {
248+
self.client.write_output_variables(&[OutputVariable {
249+
name: "fix-patch-path".to_string(),
250+
value: auto_fix_patch_patch.to_string_lossy().replace("\\", "/"),
251+
}])?;
235252
}
236253
Ok(format_checks_failed + tidy_checks_failed)
237254
}

cpp-linter/tests/comments.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
252252
format!("--thread-comments={}", test_params.thread_comments),
253253
format!("--no-lgtm={}", test_params.no_lgtm),
254254
"-p=build".to_string(),
255-
"-i=build".to_string(),
255+
"-i=build/**".to_string(),
256256
format!("--repo-root={}", tmp_dir.path().to_str().unwrap()),
257257
];
258258
if test_params.force_lgtm {
@@ -274,6 +274,13 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
274274
for mock in mocks {
275275
mock.assert();
276276
}
277+
let patch_path = tmp_dir
278+
.path()
279+
.join(".cpp-linter-cache")
280+
.join("auto_fix.patch");
281+
let patch_content =
282+
std::fs::read_to_string(patch_path).expect("Failed to read generated patch file.");
283+
println!("Generated patch content:\n{patch_content}");
277284
}
278285

279286
async fn test_comment(test_params: &TestParams) {

cpp-linter/tests/reviews.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,7 @@ async fn setup(lib_root: &Path, tmp_dir: &TempDir, test_params: &TestParams) {
231231
format!("--passive-reviews={}", test_params.passive_reviews),
232232
format!("--no-lgtm={}", test_params.no_lgtm),
233233
"-p=build".to_string(),
234-
"-i=build".to_string(),
234+
"-i=build/**".to_string(),
235235
format!("--repo-root={}", tmp_dir.path().to_str().unwrap()),
236236
];
237237
if test_params.force_lgtm {

0 commit comments

Comments
 (0)