Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions crates/ruff/tests/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,35 @@ OTHER = "OTHER"
Ok(())
}

/// Regression test for <https://github.com/astral-sh/ruff/issues/20035>
#[test]
fn deduplicate_directory_and_explicit_file() -> Result<()> {
let tempdir = TempDir::new()?;
let root = tempdir.path();

let main = root.join("main.py");
fs::write(&main, "x = 1\n")?;

assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.current_dir(root)
.args(["format", "--no-cache", "--check"])
.arg(".")
.arg("main.py"),
@r"
success: false
exit_code: 1
----- stdout -----
Would reformat: main.py
1 file would be reformatted

----- stderr -----
"
);

Ok(())
}

#[test]
fn syntax_error() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down
44 changes: 44 additions & 0 deletions crates/ruff/tests/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,50 @@ OTHER = "OTHER"
Ok(())
}

/// Regression test for <https://github.com/astral-sh/ruff/issues/20035>
#[test]
fn deduplicate_directory_and_explicit_file() -> Result<()> {
let tempdir = TempDir::new()?;
let root = tempdir.path();
let ruff_toml = tempdir.path().join("ruff.toml");
fs::write(
&ruff_toml,
r#"
[lint]
exclude = ["main.py"]
"#,
)?;

let main = root.join("main.py");
fs::write(&main, "import os\n")?;

insta::with_settings!({
filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")]
}, {
assert_cmd_snapshot!(
Command::new(get_cargo_bin(BIN_NAME))
.current_dir(root)
.args(STDIN_BASE_OPTIONS)
.args(["--config", &ruff_toml.file_name().unwrap().to_string_lossy()])
.arg(".")
// Explicitly pass main.py, should be linted regardless of it being excluded by lint.exclude
.arg("main.py"),
@r"
success: false
exit_code: 1
----- stdout -----
main.py:1:8: F401 [*] `os` imported but unused
Found 1 error.
[*] 1 fixable with the `--fix` option.

----- stderr -----
"
);
});

Ok(())
}

#[test]
fn exclude_stdin() -> Result<()> {
let tempdir = TempDir::new()?;
Expand Down
48 changes: 34 additions & 14 deletions crates/ruff_workspace/src/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,10 +523,42 @@ impl<'config> WalkPythonFilesState<'config> {
let (files, error) = self.merged.into_inner().unwrap();
error?;

Ok((files, self.resolver.into_inner().unwrap()))
let deduplicated_files = deduplicate_files(files);

Ok((deduplicated_files, self.resolver.into_inner().unwrap()))
}
}

/// Deduplicate files by path, prioritizing `Root` files over `Nested` files.
///
/// When the same path appears both as a directly specified input (`Root`)
/// and via directory traversal (`Nested`), keep the `Root` entry and drop
/// the `Nested` entry.
///
/// Dropping the root entry means that the explicitly passed path may be
/// unintentionally ignored, since it is treated as nested and can be excluded
/// despite being requested.
///
/// Concretely, with `lint.exclude = ["foo.py"]` and `ruff check . foo.py`,
/// we must keep `Root(foo.py)` and drop `Nested(foo.py)` so `foo.py` is
/// linted as the user requested.
fn deduplicate_files(mut files: ResolvedFiles) -> ResolvedFiles {
// Sort by path; for identical paths, prefer Root over Nested; place errors after files
files.sort_by(|a, b| match (a, b) {
(Ok(a_file), Ok(b_file)) => a_file.cmp(b_file),
(Ok(_), Err(_)) => Ordering::Less,
(Err(_), Ok(_)) => Ordering::Greater,
(Err(_), Err(_)) => Ordering::Equal,
});

files.dedup_by(|a, b| match (a, b) {
(Ok(a_file), Ok(b_file)) => a_file.path() == b_file.path(),
_ => false,
});

files
}

struct PythonFilesVisitorBuilder<'s, 'config> {
state: &'s WalkPythonFilesState<'config>,
transformer: &'s (dyn ConfigurationTransformer + Sync),
Expand Down Expand Up @@ -682,7 +714,7 @@ impl Drop for PythonFilesVisitor<'_, '_> {
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
#[derive(Clone, Debug, PartialEq, Eq, Ord, PartialOrd)]
pub enum ResolvedFile {
/// File explicitly passed to the CLI
Root(PathBuf),
Expand Down Expand Up @@ -715,18 +747,6 @@ impl ResolvedFile {
}
}

impl PartialOrd for ResolvedFile {
fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl Ord for ResolvedFile {
fn cmp(&self, other: &Self) -> Ordering {
self.path().cmp(other.path())
}
}

/// Return `true` if the Python file at [`Path`] is _not_ excluded.
pub fn python_file_at_path(
path: &Path,
Expand Down
Loading