Skip to content

Commit d0c8eaa

Browse files
Error instead of panic! when running Ruff from a deleted directory (#16903) (#17054)
<!-- Thank you for contributing to Ruff! To help us out with reviewing, please consider the following: - Does this pull request include a summary of the change? (See below.) - Does this pull request include a descriptive title? - Does this pull request include references to any relevant issues? --> 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`. <!-- What's the purpose of the change? What does it do, and why? --> ## 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 <micha@reiser.io>
1 parent aa93005 commit d0c8eaa

File tree

3 files changed

+40
-14
lines changed

3 files changed

+40
-14
lines changed

crates/ruff/src/lib.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ fn is_stdin(files: &[PathBuf], stdin_filename: Option<&Path>) -> bool {
112112
file == Path::new("-")
113113
}
114114

115-
/// Returns the default set of files if none are provided, otherwise returns `None`.
115+
/// Returns the default set of files if none are provided, otherwise returns provided files.
116116
fn resolve_default_files(files: Vec<PathBuf>, is_stdin: bool) -> Vec<PathBuf> {
117117
if files.is_empty() {
118118
if is_stdin {

crates/ruff/src/resolve.rs

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use std::path::Path;
22

3-
use anyhow::Result;
3+
use anyhow::{bail, Result};
44
use log::debug;
55
use path_absolutize::path_dedot;
66

@@ -21,10 +21,14 @@ pub fn resolve(
2121
config_arguments: &ConfigArguments,
2222
stdin_filename: Option<&Path>,
2323
) -> Result<PyprojectConfig> {
24+
let Ok(cwd) = std::env::current_dir() else {
25+
bail!("Working directory does not exist")
26+
};
27+
2428
// First priority: if we're running in isolated mode, use the default settings.
2529
if config_arguments.isolated {
2630
let config = config_arguments.transform(Configuration::default());
27-
let settings = config.into_settings(&path_dedot::CWD)?;
31+
let settings = config.into_settings(&cwd)?;
2832
debug!("Isolated mode, not reading any pyproject.toml");
2933
return Ok(PyprojectConfig::new(
3034
PyprojectDiscoveryStrategy::Fixed,
@@ -58,11 +62,7 @@ pub fn resolve(
5862
// that directory. (With `Strategy::Hierarchical`, we'll end up finding
5963
// the "closest" `pyproject.toml` file for every Python file later on,
6064
// so these act as the "default" settings.)
61-
if let Some(pyproject) = pyproject::find_settings_toml(
62-
stdin_filename
63-
.as_ref()
64-
.unwrap_or(&path_dedot::CWD.as_path()),
65-
)? {
65+
if let Some(pyproject) = pyproject::find_settings_toml(stdin_filename.unwrap_or(&cwd))? {
6666
debug!(
6767
"Using configuration file (via parent) at: {}",
6868
pyproject.display()
@@ -130,17 +130,13 @@ pub fn resolve(
130130
// `pyproject::find_settings_toml`.)
131131
// However, there may be a `pyproject.toml` with a `requires-python`
132132
// specified, and that is what we look for in this step.
133-
let fallback = find_fallback_target_version(
134-
stdin_filename
135-
.as_ref()
136-
.unwrap_or(&path_dedot::CWD.as_path()),
137-
);
133+
let fallback = find_fallback_target_version(stdin_filename.unwrap_or(&cwd));
138134
if let Some(version) = fallback {
139135
debug!("Derived `target-version` from found `requires-python`: {version:?}");
140136
}
141137
config.target_version = fallback.map(ast::PythonVersion::from);
142138
}
143-
let settings = config.into_settings(&path_dedot::CWD)?;
139+
let settings = config.into_settings(&cwd)?;
144140
Ok(PyprojectConfig::new(
145141
PyprojectDiscoveryStrategy::Hierarchical,
146142
settings,
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
//! Test to verify Ruff's behavior when run from deleted directory.
2+
//! It has to be isolated in a separate module.
3+
//! Tests in the same module become flaky under `cargo test`s parallel execution
4+
//! due to in-test working directory manipulation.
5+
6+
#![cfg(target_family = "unix")]
7+
8+
use std::env::set_current_dir;
9+
use std::process::Command;
10+
11+
use insta_cmd::{assert_cmd_snapshot, get_cargo_bin};
12+
const BIN_NAME: &str = "ruff";
13+
14+
#[test]
15+
fn check_in_deleted_directory_errors() {
16+
let temp_dir = tempfile::tempdir().unwrap();
17+
let temp_path = temp_dir.path().to_path_buf();
18+
set_current_dir(&temp_path).unwrap();
19+
drop(temp_dir);
20+
21+
assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)).arg("check"), @r###"
22+
success: false
23+
exit_code: 2
24+
----- stdout -----
25+
26+
----- stderr -----
27+
ruff failed
28+
Cause: Working directory does not exist
29+
"###);
30+
}

0 commit comments

Comments
 (0)