From ec3243a6e5fa647260f8f3c0fe6e97ad77bc4b69 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Sat, 27 Apr 2024 11:44:53 -0400 Subject: [PATCH] Prioritize `redefined-while-unused` over `unused-import` (#11173) ## Summary This PR adds an override to the fixer to ensure that we apply any `redefined-while-unused` fixes prior to `unused-import`. Closes https://github.com/astral-sh/ruff/issues/10905. --- crates/ruff/tests/lint.rs | 46 +++++++++++++++++++++++++++++++ crates/ruff_linter/src/fix/mod.rs | 38 +++++++++++++++---------- 2 files changed, 70 insertions(+), 14 deletions(-) diff --git a/crates/ruff/tests/lint.rs b/crates/ruff/tests/lint.rs index 47767a6a4d..44a4ce82b6 100644 --- a/crates/ruff/tests/lint.rs +++ b/crates/ruff/tests/lint.rs @@ -1284,3 +1284,49 @@ fn negated_per_file_ignores_overlap() -> Result<()> { "###); Ok(()) } + +#[test] +fn unused_interaction() -> Result<()> { + let tempdir = TempDir::new()?; + let ruff_toml = tempdir.path().join("ruff.toml"); + fs::write( + &ruff_toml, + r#" +[lint] +select = ["F"] +"#, + )?; + + insta::with_settings!({ + filters => vec![(tempdir_filter(&tempdir).as_str(), "[TMP]/")] + }, { + assert_cmd_snapshot!(Command::new(get_cargo_bin(BIN_NAME)) + .args(STDIN_BASE_OPTIONS) + .arg("--config") + .arg(&ruff_toml) + .args(["--stdin-filename", "test.py"]) + .arg("--fix") + .arg("-") + .pass_stdin(r#" +import os # F401 + +def function(): + import os # F811 + print(os.name) +"#), @r###" + success: true + exit_code: 0 + ----- stdout ----- + + import os # F401 + + def function(): + print(os.name) + + ----- stderr ----- + Found 1 error (1 fixed, 0 remaining). + "###); + }); + + Ok(()) +} diff --git a/crates/ruff_linter/src/fix/mod.rs b/crates/ruff_linter/src/fix/mod.rs index c9d8e6bb2e..17a6018fa2 100644 --- a/crates/ruff_linter/src/fix/mod.rs +++ b/crates/ruff_linter/src/fix/mod.rs @@ -129,21 +129,31 @@ fn apply_fixes<'a>( /// Compare two fixes. fn cmp_fix(rule1: Rule, rule2: Rule, fix1: &Fix, fix2: &Fix) -> std::cmp::Ordering { - fix1.min_start() - .cmp(&fix2.min_start()) - .then_with(|| match (&rule1, &rule2) { - // Apply `EndsInPeriod` fixes before `NewLineAfterLastParagraph` fixes. - (Rule::EndsInPeriod, Rule::NewLineAfterLastParagraph) => std::cmp::Ordering::Less, - (Rule::NewLineAfterLastParagraph, Rule::EndsInPeriod) => std::cmp::Ordering::Greater, - // Apply `IfElseBlockInsteadOfDictGet` fixes before `IfElseBlockInsteadOfIfExp` fixes. - (Rule::IfElseBlockInsteadOfDictGet, Rule::IfElseBlockInsteadOfIfExp) => { - std::cmp::Ordering::Less - } - (Rule::IfElseBlockInsteadOfIfExp, Rule::IfElseBlockInsteadOfDictGet) => { - std::cmp::Ordering::Greater - } + // Always apply `RedefinedWhileUnused` before `UnusedImport`, as the latter can end up fixing + // the former. + { + match (rule1, rule2) { + (Rule::RedefinedWhileUnused, Rule::UnusedImport) => return std::cmp::Ordering::Less, + (Rule::UnusedImport, Rule::RedefinedWhileUnused) => return std::cmp::Ordering::Greater, _ => std::cmp::Ordering::Equal, - }) + } + } + // Apply fixes in order of their start position. + .then_with(|| fix1.min_start().cmp(&fix2.min_start())) + // Break ties in the event of overlapping rules, for some specific combinations. + .then_with(|| match (&rule1, &rule2) { + // Apply `EndsInPeriod` fixes before `NewLineAfterLastParagraph` fixes. + (Rule::EndsInPeriod, Rule::NewLineAfterLastParagraph) => std::cmp::Ordering::Less, + (Rule::NewLineAfterLastParagraph, Rule::EndsInPeriod) => std::cmp::Ordering::Greater, + // Apply `IfElseBlockInsteadOfDictGet` fixes before `IfElseBlockInsteadOfIfExp` fixes. + (Rule::IfElseBlockInsteadOfDictGet, Rule::IfElseBlockInsteadOfIfExp) => { + std::cmp::Ordering::Less + } + (Rule::IfElseBlockInsteadOfIfExp, Rule::IfElseBlockInsteadOfDictGet) => { + std::cmp::Ordering::Greater + } + _ => std::cmp::Ordering::Equal, + }) } #[cfg(test)]