From d0c8eaa0923352ccc4ec30c9fac1f573f20998b3 Mon Sep 17 00:00:00 2001 From: Max Mynter <32773644+maxmynter@users.noreply.github.com> Date: Tue, 1 Apr 2025 08:17:07 -0400 Subject: [PATCH] Error instead of `panic!` when running Ruff from a deleted directory (#16903) (#17054) Closes #16903 ## Summary Check if the current working directory exist. If not, provide an error instead of panicking. Fixed a stale comment in `resolve_default_files`. ## Test Plan I added a test to the `resolve_files.rs`. Manual testing follows steps of #16903 : - Terminal 1 ```bash mkdir tmp cd tmp ``` - Terminal 2 ```bash rm -rf tmp ``` - Terminal 1 ```bash ruff check ``` ## Open Issues / Questions to Reviewer All tests pass when executed with `cargo nextest run`. However, with `cargo test` the parallelization makes the other tests fail as we change the `pwd`. Serial execution with `cargo test` seems to require [another dependency or some workarounds](https://stackoverflow.com/questions/51694017/how-can-i-avoid-running-some-tests-in-parallel). Do you think an additional dependency or test complexity is worth testing this small edge case, do you have another implementation idea, or should i rather remove the test? --- P.S.: I'm currently participating in a batch at the [Recurse Center](https://www.recurse.com/) and would love to contribute more for the next six weeks to improve my Rust. Let me know if you're open to mentoring/reviewing and/or if you have specific areas where help would be most valued. --------- Co-authored-by: Micha Reiser --- crates/ruff/src/lib.rs | 2 +- crates/ruff/src/resolve.rs | 22 +++++++++------------ crates/ruff/tests/direxist_guard.rs | 30 +++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 crates/ruff/tests/direxist_guard.rs diff --git a/crates/ruff/src/lib.rs b/crates/ruff/src/lib.rs index 9c60eb5625..309bfc6fd0 100644 --- a/crates/ruff/src/lib.rs +++ b/crates/ruff/src/lib.rs @@ -112,7 +112,7 @@ fn is_stdin(files: &[PathBuf], stdin_filename: Option<&Path>) -> bool { file == Path::new("-") } -/// Returns the default set of files if none are provided, otherwise returns `None`. +/// Returns the default set of files if none are provided, otherwise returns provided files. fn resolve_default_files(files: Vec, is_stdin: bool) -> Vec { if files.is_empty() { if is_stdin { diff --git a/crates/ruff/src/resolve.rs b/crates/ruff/src/resolve.rs index 4fbea4dad6..b68d2c61fd 100644 --- a/crates/ruff/src/resolve.rs +++ b/crates/ruff/src/resolve.rs @@ -1,6 +1,6 @@ use std::path::Path; -use anyhow::Result; +use anyhow::{bail, Result}; use log::debug; use path_absolutize::path_dedot; @@ -21,10 +21,14 @@ pub fn resolve( config_arguments: &ConfigArguments, stdin_filename: Option<&Path>, ) -> Result { + let Ok(cwd) = std::env::current_dir() else { + bail!("Working directory does not exist") + }; + // First priority: if we're running in isolated mode, use the default settings. if config_arguments.isolated { let config = config_arguments.transform(Configuration::default()); - let settings = config.into_settings(&path_dedot::CWD)?; + let settings = config.into_settings(&cwd)?; debug!("Isolated mode, not reading any pyproject.toml"); return Ok(PyprojectConfig::new( PyprojectDiscoveryStrategy::Fixed, @@ -58,11 +62,7 @@ pub fn resolve( // that directory. (With `Strategy::Hierarchical`, we'll end up finding // the "closest" `pyproject.toml` file for every Python file later on, // so these act as the "default" settings.) - if let Some(pyproject) = pyproject::find_settings_toml( - stdin_filename - .as_ref() - .unwrap_or(&path_dedot::CWD.as_path()), - )? { + if let Some(pyproject) = pyproject::find_settings_toml(stdin_filename.unwrap_or(&cwd))? { debug!( "Using configuration file (via parent) at: {}", pyproject.display() @@ -130,17 +130,13 @@ pub fn resolve( // `pyproject::find_settings_toml`.) // However, there may be a `pyproject.toml` with a `requires-python` // specified, and that is what we look for in this step. - let fallback = find_fallback_target_version( - stdin_filename - .as_ref() - .unwrap_or(&path_dedot::CWD.as_path()), - ); + let fallback = find_fallback_target_version(stdin_filename.unwrap_or(&cwd)); if let Some(version) = fallback { debug!("Derived `target-version` from found `requires-python`: {version:?}"); } config.target_version = fallback.map(ast::PythonVersion::from); } - let settings = config.into_settings(&path_dedot::CWD)?; + let settings = config.into_settings(&cwd)?; Ok(PyprojectConfig::new( PyprojectDiscoveryStrategy::Hierarchical, settings, diff --git a/crates/ruff/tests/direxist_guard.rs b/crates/ruff/tests/direxist_guard.rs new file mode 100644 index 0000000000..3812d9ba03 --- /dev/null +++ b/crates/ruff/tests/direxist_guard.rs @@ -0,0 +1,30 @@ +//! Test to verify Ruff's behavior when run from deleted directory. +//! It has to be isolated in a separate module. +//! Tests in the same module become flaky under `cargo test`s parallel execution +//! due to in-test working directory manipulation. + +#![cfg(target_family = "unix")] + +use std::env::set_current_dir; +use std::process::Command; + +use insta_cmd::{assert_cmd_snapshot, get_cargo_bin}; +const BIN_NAME: &str = "ruff"; + +#[test] +fn check_in_deleted_directory_errors() { + let temp_dir = tempfile::tempdir().unwrap(); + let temp_path = temp_dir.path().to_path_buf(); + set_current_dir(&temp_path).unwrap(); + drop(temp_dir); + + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)).arg("check"), @r###" + success: false + exit_code: 2 + ----- stdout ----- + + ----- stderr ----- + ruff failed + Cause: Working directory does not exist + "###); +}