Skip to content

Commit 59f2564

Browse files
kylehgcclaude
andcommitted
fix(prettier): enforce the failure invariant at every call site
Review round: one invariant — a run that signals failure never yields the success line. Failure is non-zero exit, or prettier's own 'Code style issues found' verdict for callers that have no exit code to give. - rtk format prettier had result.exit_code in hand and discarded it at the filter dispatch; pass it through filter_prettier_output_with_exit - guard the previously unguarded 'All matched files use Prettier' early return (stray success summary beside a non-zero exit) - the pipe path (no exit code, text only) is covered by the textual verdict - add the checklist-required count_tokens >=60% savings test Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6f4d9f7 commit 59f2564

2 files changed

Lines changed: 60 additions & 4 deletions

File tree

‎src/cmds/js/prettier_cmd.rs‎

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ pub fn filter_prettier_output(output: &str) -> String {
3333
filter_prettier_output_with_exit(output, 0)
3434
}
3535

36-
fn filter_prettier_output_with_exit(output: &str, exit_code: i32) -> String {
36+
pub(crate) fn filter_prettier_output_with_exit(output: &str, exit_code: i32) -> String {
3737
// Prettier colors the "[warn]" marker even when piped, so the prefix
3838
// match below must see the literal marker, not "[\x1b[33mwarn\x1b[39m]".
3939
let output = &strip_ansi(output);
@@ -42,6 +42,12 @@ fn filter_prettier_output_with_exit(output: &str, exit_code: i32) -> String {
4242
return "Error: prettier produced no output".to_string();
4343
}
4444

45+
// One invariant: a run that signals failure never yields the success
46+
// line. Non-zero exit covers callers that have an exit code; prettier's
47+
// own "Code style issues found" verdict covers callers that don't
48+
// (pipe mode, where the filter sees only text).
49+
let failed = exit_code != 0 || output.contains("Code style issues found");
50+
4551
let mut files_to_format: Vec<String> = Vec::new();
4652
let mut files_checked = 0;
4753
let mut is_check_mode = true;
@@ -90,7 +96,7 @@ fn filter_prettier_output_with_exit(output: &str, exit_code: i32) -> String {
9096
}
9197

9298
// Check if all files are formatted
93-
if files_to_format.is_empty() && output.contains("All matched files use Prettier") {
99+
if !failed && files_to_format.is_empty() && output.contains("All matched files use Prettier") {
94100
return "Prettier: All files formatted correctly".to_string();
95101
}
96102

@@ -104,7 +110,7 @@ fn filter_prettier_output_with_exit(output: &str, exit_code: i32) -> String {
104110
if is_check_mode {
105111
// Check mode: show files that need formatting
106112
if files_to_format.is_empty() {
107-
if exit_code != 0 {
113+
if failed {
108114
// Prettier failed but no file list could be parsed (unknown
109115
// extension, unexpected output shape). Never claim success on
110116
// a failing check — pass the real output through.
@@ -247,6 +253,56 @@ Code style issues found in the above file(s). Forgot to run Prettier?
247253
assert!(result.contains("All files formatted correctly"));
248254
}
249255

256+
#[test]
257+
fn test_filter_check_failure_textual_verdict_no_exit_code() {
258+
// Pipe/format callers may have no exit code to give, but prettier's
259+
// own "Code style issues found" verdict still marks the run failing.
260+
let output = "Checking formatting...\n\
261+
[warn] src/component.vue\n\
262+
[warn] Code style issues found in the above file. Run Prettier with --write to fix.";
263+
let result = filter_prettier_output(output);
264+
assert!(
265+
!result.contains("All files formatted correctly"),
266+
"failing check reported as success: {}",
267+
result
268+
);
269+
}
270+
271+
#[test]
272+
fn test_filter_check_failure_stray_success_summary() {
273+
// Non-zero exit wins over a success summary line in the output.
274+
let output = "Checking formatting...\nAll matched files use Prettier code style!";
275+
let result = filter_prettier_output_with_exit(output, 1);
276+
assert!(
277+
!result.contains("All files formatted correctly"),
278+
"failing check reported as success: {}",
279+
result
280+
);
281+
}
282+
283+
fn count_tokens(s: &str) -> usize {
284+
s.split_whitespace().count()
285+
}
286+
287+
#[test]
288+
fn test_filter_check_savings_over_60_percent() {
289+
let mut output = String::from("Checking formatting...\n");
290+
for i in 0..100 {
291+
output.push_str(&format!("[warn] src/components/file{}.ts\n", i));
292+
}
293+
output.push_str(
294+
"[warn] Code style issues found in the above files. Run Prettier with --write to fix.",
295+
);
296+
let filtered = filter_prettier_output_with_exit(&output, 1);
297+
let (input, kept) = (count_tokens(&output), count_tokens(&filtered));
298+
assert!(
299+
kept * 100 <= input * 40,
300+
"savings below 60%: {} -> {} tokens",
301+
input,
302+
kept
303+
);
304+
}
305+
250306
#[test]
251307
fn test_filter_check_failure_ansi_colored_warn() {
252308
// Real prettier 3.x output captured through rtk's pipes on Windows:

‎src/cmds/system/format_cmd.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@ pub fn run(args: &[String], verbose: u8) -> Result<i32> {
119119

120120
// Dispatch to appropriate filter based on formatter
121121
let filtered = match formatter.as_str() {
122-
"prettier" => prettier_cmd::filter_prettier_output(&raw),
122+
"prettier" => prettier_cmd::filter_prettier_output_with_exit(&raw, result.exit_code),
123123
"ruff" => ruff_cmd::filter_ruff_format(&raw),
124124
"black" => filter_black_output(&raw),
125125
_ => raw.trim().to_string(),

0 commit comments

Comments
 (0)