From ceafaa1293f90d5d9ce3b42f2224cb236ed1fb86 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Fri, 3 Oct 2025 21:24:03 +0900 Subject: [PATCH 1/3] handle argfile expansion errors gracefully fix a comment --- crates/ruff/src/main.rs | 59 +++++++++++++++------------ crates/ruff/tests/integration_test.rs | 17 ++++++++ 2 files changed, 51 insertions(+), 25 deletions(-) diff --git a/crates/ruff/src/main.rs b/crates/ruff/src/main.rs index 62e4f85b6e4ba..d79bf89652592 100644 --- a/crates/ruff/src/main.rs +++ b/crates/ruff/src/main.rs @@ -1,6 +1,7 @@ use std::io::Write; use std::process::ExitCode; +use anyhow::Context; use clap::Parser; use colored::Colorize; @@ -39,39 +40,47 @@ pub fn main() -> ExitCode { } let args = wild::args_os(); - let args = argfile::expand_args_from(args, argfile::parse_fromfile, argfile::PREFIX).unwrap(); + let args = match argfile::expand_args_from(args, argfile::parse_fromfile, argfile::PREFIX) + .map_err(anyhow::Error::from) + .context("Failed to read CLI arguments from files") + { + Ok(args) => args, + Err(err) => return report_error(&err), + }; let args = Args::parse_from(args); match run(args) { Ok(code) => code.into(), - Err(err) => { - { - // Exit "gracefully" on broken pipe errors. - // - // See: https://github.com/BurntSushi/ripgrep/blob/bf63fe8f258afc09bae6caa48f0ae35eaf115005/crates/core/main.rs#L47C1-L61C14 - for cause in err.chain() { - if let Some(ioerr) = cause.downcast_ref::() { - if ioerr.kind() == std::io::ErrorKind::BrokenPipe { - return ExitCode::from(0); - } - } - } - - // Use `writeln` instead of `eprintln` to avoid panicking when the stderr pipe is broken. - let mut stderr = std::io::stderr().lock(); + Err(err) => report_error(&err), + } +} - // This communicates that this isn't a linter error but ruff itself hard-errored for - // some reason (e.g. failed to resolve the configuration) - writeln!(stderr, "{}", "ruff failed".red().bold()).ok(); - // Currently we generally only see one error, but e.g. with io errors when resolving - // the configuration it is help to chain errors ("resolving configuration failed" -> - // "failed to read file: subdir/pyproject.toml") - for cause in err.chain() { - writeln!(stderr, " {} {cause}", "Cause:".bold()).ok(); +fn report_error(err: &anyhow::Error) -> ExitCode { + { + // Exit "gracefully" on broken pipe errors. + // + // See: https://github.com/BurntSushi/ripgrep/blob/bf63fe8f258afc09bae6caa48f0ae35eaf115005/crates/core/main.rs#L47C1-L61C14 + for cause in err.chain() { + if let Some(ioerr) = cause.downcast_ref::() { + if ioerr.kind() == std::io::ErrorKind::BrokenPipe { + return ExitCode::from(0); } } - ExitStatus::Error.into() + } + + // Use `writeln` instead of `eprintln` to avoid panicking when the stderr pipe is broken. + let mut stderr = std::io::stderr().lock(); + + // This communicates that this isn't a linter error but ruff itself hard-errored for + // some reason (e.g. failed to resolve the configuration) + writeln!(stderr, "{}", "ruff failed".red().bold()).ok(); + // Currently we generally only see one error, but e.g. with io errors when resolving + // the configuration it is help to chain errors ("resolving configuration failed" -> + // "failed to read file: subdir/pyproject.toml") + for cause in err.chain() { + writeln!(stderr, " {} {cause}", "Cause:".bold()).ok(); } } + ExitStatus::Error.into() } diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 4babbb39862f9..43fa7e88cbc7d 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -1688,6 +1688,23 @@ fn check_input_from_argfile() -> Result<()> { Ok(()) } +#[test] +// Regression test for https://github.com/astral-sh/ruff/issues/20655 +fn missing_argfile_reports_error() { + let mut cmd = RuffCheck::default().filename("@!.txt").build(); + assert_cmd_snapshot!(cmd, @r" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Failed to read CLI arguments from files + Cause: failed to open file `!.txt` + Cause: No such file or directory (os error 2) + "); +} + #[test] fn check_hints_hidden_unsafe_fixes() { let mut cmd = RuffCheck::default() From 235d2db455dc693e7bf889db80f4e93c2c14e0e8 Mon Sep 17 00:00:00 2001 From: Takayuki Maeda Date: Fri, 3 Oct 2025 21:57:47 +0900 Subject: [PATCH 2/3] apply a snapshot filter --- crates/ruff/tests/integration_test.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/ruff/tests/integration_test.rs b/crates/ruff/tests/integration_test.rs index 43fa7e88cbc7d..edca13cd7290b 100644 --- a/crates/ruff/tests/integration_test.rs +++ b/crates/ruff/tests/integration_test.rs @@ -1692,7 +1692,10 @@ fn check_input_from_argfile() -> Result<()> { // Regression test for https://github.com/astral-sh/ruff/issues/20655 fn missing_argfile_reports_error() { let mut cmd = RuffCheck::default().filename("@!.txt").build(); - assert_cmd_snapshot!(cmd, @r" + insta::with_settings!({filters => vec![ + ("The system cannot find the file specified.", "No such file or directory") + ]}, { + assert_cmd_snapshot!(cmd, @r" success: false exit_code: 2 ----- stdout ----- @@ -1703,6 +1706,7 @@ fn missing_argfile_reports_error() { Cause: failed to open file `!.txt` Cause: No such file or directory (os error 2) "); + }); } #[test] From d9f0afd3c0429d63bb8bef17b889eae3b8ba11c0 Mon Sep 17 00:00:00 2001 From: Brent Westbrook <36778786+ntBre@users.noreply.github.com> Date: Fri, 3 Oct 2025 09:32:05 -0400 Subject: [PATCH 3/3] call context directly --- crates/ruff/src/main.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/crates/ruff/src/main.rs b/crates/ruff/src/main.rs index d79bf89652592..5ec78c9e25c31 100644 --- a/crates/ruff/src/main.rs +++ b/crates/ruff/src/main.rs @@ -41,7 +41,6 @@ pub fn main() -> ExitCode { let args = wild::args_os(); let args = match argfile::expand_args_from(args, argfile::parse_fromfile, argfile::PREFIX) - .map_err(anyhow::Error::from) .context("Failed to read CLI arguments from files") { Ok(args) => args,