Skip to content

Commit c70ab7d

Browse files
fables-talesclaude
andcommitted
Fix --check mode to exit non-zero on syntax errors
Previously, --check would exit with code 0 even when syntax errors were encountered in input files. This changes the check mode to track errors and exit with code 1 (SyntaxError) when any syntax errors occur. Also adds tests for --check with syntax errors for both file and stdin input. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 6221b00 commit c70ab7d

File tree

2 files changed

+61
-11
lines changed

2 files changed

+61
-11
lines changed

src/main.rs

Lines changed: 21 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -347,16 +347,23 @@ fn main() {
347347
match opts {
348348
CommandlineOpts { check: true, .. } => {
349349
let text_diffs: Arc<Mutex<Vec<String>>> = Arc::new(Mutex::new(Vec::new()));
350-
351-
iterate_formatted(&opts, &|(file_path, before, after)| match after {
352-
None => {}
353-
Some(fmtted) => {
354-
let diff = TextDiff::from_lines(before, &fmtted);
355-
let path_string = file_path.to_str().unwrap();
356-
text_diffs.lock().unwrap().push(format!(
357-
"{}",
358-
diff.unified_diff().header(path_string, path_string)
359-
));
350+
let errors_count: Arc<Mutex<usize>> = Arc::new(Mutex::new(0));
351+
352+
iterate_input_files(&opts, &|(file_path, before)| {
353+
match rubyfmt_string(&opts, before) {
354+
Ok(None) => {}
355+
Ok(Some(fmtted)) => {
356+
let diff = TextDiff::from_lines(before, &fmtted);
357+
let path_string = file_path.to_str().unwrap();
358+
text_diffs.lock().unwrap().push(format!(
359+
"{}",
360+
diff.unified_diff().header(path_string, path_string)
361+
));
362+
}
363+
Err(e) => {
364+
handle_rubyfmt_error(e, &file_path.display().to_string(), ErrorExit::NoExit);
365+
*errors_count.lock().unwrap() += 1;
366+
}
360367
}
361368
});
362369

@@ -370,7 +377,10 @@ fn main() {
370377
diffs_reported += 1
371378
}
372379
}
373-
if diffs_reported > 0 {
380+
let errors = *errors_count.lock().unwrap();
381+
if errors > 0 {
382+
exit(rubyfmt::FormatError::SyntaxError as i32);
383+
} else if diffs_reported > 0 {
374384
exit(rubyfmt::FormatError::DiffDetected as i32);
375385
} else {
376386
exit(0)

tests/cli_interface_test.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,46 @@ fn format_input_file_without_changes() {
499499
assert_eq!("a(1, 2, 3)\n", read_to_string(file_one.path()).unwrap());
500500
}
501501

502+
#[test]
503+
fn test_check_flag_with_syntax_error() {
504+
let mut file = NamedTempFile::new().unwrap();
505+
// Use actually invalid Ruby syntax (incomplete def statement)
506+
writeln!(file, "def (").unwrap();
507+
508+
let output = Command::cargo_bin("rubyfmt-main")
509+
.unwrap()
510+
.arg(file.path())
511+
.arg("--check")
512+
.assert()
513+
.code(1)
514+
.failure();
515+
516+
let stderr = String::from_utf8_lossy(&output.get_output().stderr);
517+
assert!(
518+
stderr.contains("syntax error"),
519+
"Expected stderr to contain 'syntax error', got: {}",
520+
stderr
521+
);
522+
}
523+
524+
#[test]
525+
fn test_check_flag_stdin_with_syntax_error() {
526+
let output = Command::cargo_bin("rubyfmt-main")
527+
.unwrap()
528+
.arg("--check")
529+
.write_stdin("def (")
530+
.assert()
531+
.code(1)
532+
.failure();
533+
534+
let stderr = String::from_utf8_lossy(&output.get_output().stderr);
535+
assert!(
536+
stderr.contains("syntax error"),
537+
"Expected stderr to contain 'syntax error', got: {}",
538+
stderr
539+
);
540+
}
541+
502542
#[test]
503543
fn test_format_respects_opt_in_header() {
504544
let mut file_one = NamedTempFile::new().unwrap();

0 commit comments

Comments
 (0)