diff --git a/crates/ruff/tests/format.rs b/crates/ruff/tests/format.rs index 35443daa2394b..f1369048f7326 100644 --- a/crates/ruff/tests/format.rs +++ b/crates/ruff/tests/format.rs @@ -500,6 +500,35 @@ OTHER = "OTHER" Ok(()) } +/// Regression test for +#[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()?; diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index b54e69996d197..492665555a071 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -271,6 +271,50 @@ OTHER = "OTHER" Ok(()) } +/// Regression test for +#[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()?; diff --git a/crates/ruff_workspace/src/resolver.rs b/crates/ruff_workspace/src/resolver.rs index 84a31ebbeea4f..d6e642ed486e5 100644 --- a/crates/ruff_workspace/src/resolver.rs +++ b/crates/ruff_workspace/src/resolver.rs @@ -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), @@ -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), @@ -715,18 +747,6 @@ impl ResolvedFile { } } -impl PartialOrd for ResolvedFile { - fn partial_cmp(&self, other: &Self) -> Option { - 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,