Fix isolation groups for unused imports (#6774)

## Summary

The isolation group for unused imports was relying on
`checker.semantic().current_statement()`, which isn't valid for that
rule, since it runs over the _scope_, not the statement. Instead, we
need to lookup the isolation group based on the `NodeId` of the
statement.

Our tests didn't catch this, because we mostly have cases that look like
this:

```python
if TYPE_CHECKING:
    import shelve
    import importlib
```

In this case, the two fixes to remove the two unused imports are
considered overlapping (since we delete the _full_ line, and the two
_full_ lines touch, and we consider exactly-adjacent fixes to be
overlapping), and so they don't run in a single pass due to the
non-overlapping-fixes requirement. That is: the isolation groups aren't
required for this case. They are, however, required for cases like:

```python
if TYPE_CHECKING:
    import shelve

    import importlib
```

...where the fixes don't overlap.

Closes https://github.com/astral-sh/ruff/issues/6758.

## Test Plan

`cargo test`

crates/ruff/resources/test/fixtures/pyflakes/F401_0.py

@@ -99,3 +99,16 @@ import foo.bar as bop
 import foo.bar.baz
 
 print(bop.baz.read_csv("test.csv"))
+
+# Test: isolated deletions.
+if TYPE_CHECKING:
+    import a1
+
+    import a2
+
+
+match *0, 1, *2:
+    case 0,:
+        import b1
+
+        import b2

crates/ruff/resources/test/fixtures/pyflakes/F841_3.py

@@ -154,3 +154,14 @@ def f() -> None:
         print("hello")
     except A as e :
         print("oh no!")
+
+
+def f():
+    x = 1
+    y = 2
+
+
+def f():
+    x = 1
+
+    y = 2

crates/ruff/src/checkers/ast/mod.rs

@@ -52,7 +52,8 @@ use ruff_python_parser::typing::{parse_type_annotation, AnnotationKind};
 use ruff_python_semantic::analyze::{typing, visibility};
 use ruff_python_semantic::{
     BindingFlags, BindingId, BindingKind, Exceptions, Export, FromImport, Globals, Import, Module,
-    ModuleKind, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, StarImport, SubmoduleImport,
+    ModuleKind, NodeId, ScopeId, ScopeKind, SemanticModel, SemanticModelFlags, StarImport,
+    SubmoduleImport,
 };
 use ruff_python_stdlib::builtins::{BUILTINS, MAGIC_GLOBALS};
 use ruff_source_file::Locator;
@@ -193,24 +194,6 @@ impl<'a> Checker<'a> {
         }
     }
 
-    /// Returns the [`IsolationLevel`] to isolate fixes for the current statement.
-    ///
-    /// The primary use-case for fix isolation is to ensure that we don't delete all statements
-    /// in a given indented block, which would cause a syntax error. We therefore need to ensure
-    /// that we delete at most one statement per indented block per fixer pass. Fix isolation should
-    /// thus be applied whenever we delete a statement, but can otherwise be omitted.
-    pub(crate) fn statement_isolation(&self) -> IsolationLevel {
-        IsolationLevel::Group(self.semantic.current_statement_id().into())
-    }
-
-    /// Returns the [`IsolationLevel`] to isolate fixes in the current statement's parent.
-    pub(crate) fn parent_isolation(&self) -> IsolationLevel {
-        self.semantic
-            .current_statement_parent_id()
-            .map(|node_id| IsolationLevel::Group(node_id.into()))
-            .unwrap_or_default()
-    }
-
     /// The [`Locator`] for the current file, which enables extraction of source code from byte
     /// offsets.
     pub(crate) const fn locator(&self) -> &'a Locator<'a> {
@@ -259,6 +242,18 @@ impl<'a> Checker<'a> {
     pub(crate) const fn any_enabled(&self, rules: &[Rule]) -> bool {
         self.settings.rules.any_enabled(rules)
     }
+
+    /// Returns the [`IsolationLevel`] to isolate fixes for a given node.
+    ///
+    /// The primary use-case for fix isolation is to ensure that we don't delete all statements
+    /// in a given indented block, which would cause a syntax error. We therefore need to ensure
+    /// that we delete at most one statement per indented block per fixer pass. Fix isolation should
+    /// thus be applied whenever we delete a statement, but can otherwise be omitted.
+    pub(crate) fn isolation(node_id: Option<NodeId>) -> IsolationLevel {
+        node_id
+            .map(|node_id| IsolationLevel::Group(node_id.into()))
+            .unwrap_or_default()
+    }
 }
 
 impl<'a, 'b> Visitor<'b> for Checker<'a>

crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs

@@ -85,7 +85,9 @@ pub(crate) fn duplicate_class_field_definition(checker: &mut Checker, body: &[St
                     checker.locator(),
                     checker.indexer(),
                 );
-                diagnostic.set_fix(Fix::suggested(edit).isolate(checker.statement_isolation()));
+                diagnostic.set_fix(Fix::suggested(edit).isolate(Checker::isolation(Some(
+                    checker.semantic().current_statement_id(),
+                ))));
             }
             checker.diagnostics.push(diagnostic);
         }

crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs

@@ -69,7 +69,9 @@ pub(crate) fn ellipsis_in_non_empty_class_body(checker: &mut Checker, body: &[St
                     checker.locator(),
                     checker.indexer(),
                 );
-                diagnostic.set_fix(Fix::automatic(edit).isolate(checker.statement_isolation()));
+                diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(Some(
+                    checker.semantic().current_statement_id(),
+                ))));
             }
             checker.diagnostics.push(diagnostic);
         }

crates/ruff/src/rules/flake8_pyi/rules/pass_in_class_body.rs

@@ -36,7 +36,9 @@ pub(crate) fn pass_in_class_body(checker: &mut Checker, class_def: &ast::StmtCla
         if checker.patch(diagnostic.kind.rule()) {
             let edit =
                 autofix::edits::delete_stmt(stmt, Some(stmt), checker.locator(), checker.indexer());
-            diagnostic.set_fix(Fix::automatic(edit).isolate(checker.statement_isolation()));
+            diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(Some(
+                checker.semantic().current_statement_id(),
+            ))));
         }
         checker.diagnostics.push(diagnostic);
     }

crates/ruff/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs

@@ -99,7 +99,9 @@ pub(crate) fn str_or_repr_defined_in_stub(checker: &mut Checker, stmt: &Stmt) {
         let stmt = checker.semantic().current_statement();
         let parent = checker.semantic().current_statement_parent();
         let edit = delete_stmt(stmt, parent, checker.locator(), checker.indexer());
-        diagnostic.set_fix(Fix::automatic(edit).isolate(checker.parent_isolation()));
+        diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(
+            checker.semantic().current_statement_parent_id(),
+        )));
     }
     checker.diagnostics.push(diagnostic);
 }

crates/ruff/src/rules/flake8_type_checking/rules/empty_type_checking_block.rs

@@ -61,7 +61,9 @@ pub(crate) fn empty_type_checking_block(checker: &mut Checker, stmt: &ast::StmtI
         let stmt = checker.semantic().current_statement();
         let parent = checker.semantic().current_statement_parent();
         let edit = autofix::edits::delete_stmt(stmt, parent, checker.locator(), checker.indexer());
-        diagnostic.set_fix(Fix::automatic(edit).isolate(checker.parent_isolation()));
+        diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(
+            checker.semantic().current_statement_parent_id(),
+        )));
     }
     checker.diagnostics.push(diagnostic);
 }

crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs

@@ -236,7 +236,8 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
     )?;
 
     Ok(
-        Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits())
-            .isolate(checker.parent_isolation()),
+        Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits()).isolate(
+            Checker::isolation(checker.semantic().parent_statement_id(node_id)),
+        ),
     )
 }

crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs

@@ -486,7 +486,8 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
     )?;
 
     Ok(
-        Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits())
-            .isolate(checker.parent_isolation()),
+        Fix::suggested_edits(remove_import_edit, add_import_edit.into_edits()).isolate(
+            Checker::isolation(checker.semantic().parent_statement_id(node_id)),
+        ),
     )
 }

crates/ruff/src/rules/pyflakes/rules/unused_import.rs

@@ -217,6 +217,7 @@ pub(crate) fn unused_import(checker: &Checker, scope: &Scope, diagnostics: &mut
 }
 
 /// An unused import with its surrounding context.
+#[derive(Debug)]
 struct ImportBinding<'a> {
     /// The qualified name of the import (e.g., `typing.List` for `from typing import List`).
     import: AnyImport<'a>,
@@ -251,5 +252,7 @@ fn fix_imports(checker: &Checker, node_id: NodeId, imports: &[ImportBinding]) ->
         checker.stylist(),
         checker.indexer(),
     )?;
-    Ok(Fix::automatic(edit).isolate(checker.parent_isolation()))
+    Ok(Fix::automatic(edit).isolate(Checker::isolation(
+        checker.semantic().parent_statement_id(node_id),
+    )))
 }

crates/ruff/src/rules/pyflakes/rules/unused_variable.rs

@@ -1,6 +1,6 @@
 use itertools::Itertools;
 
-use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, IsolationLevel, Violation};
+use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
 use ruff_macros::{derive_message_formats, violation};
 use ruff_python_ast::helpers::contains_effect;
 use ruff_python_ast::{self as ast, PySourceType, Ranged, Stmt};
@@ -206,11 +206,7 @@ fn remove_unused_variable(binding: &Binding, checker: &Checker) -> Option<Fix> {
     let node_id = binding.source?;
     let statement = checker.semantic().statement(node_id);
     let parent = checker.semantic().parent_statement(node_id);
-    let isolation = checker
-        .semantic()
-        .parent_statement_id(node_id)
-        .map(|node_id| IsolationLevel::Group(node_id.into()))
-        .unwrap_or_default();
+    let isolation = Checker::isolation(checker.semantic().parent_statement_id(node_id));
 
     // First case: simple assignment (`x = 1`)
     if let Stmt::Assign(ast::StmtAssign { targets, value, .. }) = statement {

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_0.py.snap

@@ -190,5 +190,78 @@ F401_0.py:99:8: F401 [*] `foo.bar.baz` imported but unused
 99      |-import foo.bar.baz
 100 99  | 
 101 100 | print(bop.baz.read_csv("test.csv"))
+102 101 | 
+
+F401_0.py:105:12: F401 [*] `a1` imported but unused
+    |
+103 | # Test: isolated deletions.
+104 | if TYPE_CHECKING:
+105 |     import a1
+    |            ^^ F401
+106 | 
+107 |     import a2
+    |
+    = help: Remove unused import: `a1`
+
+ℹ Fix
+102 102 | 
+103 103 | # Test: isolated deletions.
+104 104 | if TYPE_CHECKING:
+105     |-    import a1
+106 105 | 
+107 106 |     import a2
+108 107 | 
+
+F401_0.py:107:12: F401 [*] `a2` imported but unused
+    |
+105 |     import a1
+106 | 
+107 |     import a2
+    |            ^^ F401
+    |
+    = help: Remove unused import: `a2`
+
+ℹ Fix
+104 104 | if TYPE_CHECKING:
+105 105 |     import a1
+106 106 | 
+107     |-    import a2
+108 107 | 
+109 108 | 
+110 109 | match *0, 1, *2:
+
+F401_0.py:112:16: F401 [*] `b1` imported but unused
+    |
+110 | match *0, 1, *2:
+111 |     case 0,:
+112 |         import b1
+    |                ^^ F401
+113 | 
+114 |         import b2
+    |
+    = help: Remove unused import: `b1`
+
+ℹ Fix
+109 109 | 
+110 110 | match *0, 1, *2:
+111 111 |     case 0,:
+112     |-        import b1
+113 112 | 
+114 113 |         import b2
+
+F401_0.py:114:16: F401 [*] `b2` imported but unused
+    |
+112 |         import b1
+113 | 
+114 |         import b2
+    |                ^^ F401
+    |
+    = help: Remove unused import: `b2`
+
+ℹ Fix
+111 111 |     case 0,:
+112 112 |         import b1
+113 113 | 
+114     |-        import b2
 
 

crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap

@@ -601,5 +601,76 @@ F841_3.py:155:17: F841 [*] Local variable `e` is assigned to but never used
 155     |-    except A as e :
     155 |+    except A:
 156 156 |         print("oh no!")
+157 157 | 
+158 158 | 
+
+F841_3.py:160:5: F841 [*] Local variable `x` is assigned to but never used
+    |
+159 | def f():
+160 |     x = 1
+    |     ^ F841
+161 |     y = 2
+    |
+    = help: Remove assignment to unused variable `x`
+
+ℹ Suggested fix
+157 157 | 
+158 158 | 
+159 159 | def f():
+160     |-    x = 1
+161 160 |     y = 2
+162 161 | 
+163 162 | 
+
+F841_3.py:161:5: F841 [*] Local variable `y` is assigned to but never used
+    |
+159 | def f():
+160 |     x = 1
+161 |     y = 2
+    |     ^ F841
+    |
+    = help: Remove assignment to unused variable `y`
+
+ℹ Suggested fix
+158 158 | 
+159 159 | def f():
+160 160 |     x = 1
+161     |-    y = 2
+162 161 | 
+163 162 | 
+164 163 | def f():
+
+F841_3.py:165:5: F841 [*] Local variable `x` is assigned to but never used
+    |
+164 | def f():
+165 |     x = 1
+    |     ^ F841
+166 | 
+167 |     y = 2
+    |
+    = help: Remove assignment to unused variable `x`
+
+ℹ Suggested fix
+162 162 | 
+163 163 | 
+164 164 | def f():
+165     |-    x = 1
+166 165 | 
+167 166 |     y = 2
+
+F841_3.py:167:5: F841 [*] Local variable `y` is assigned to but never used
+    |
+165 |     x = 1
+166 | 
+167 |     y = 2
+    |     ^ F841
+    |
+    = help: Remove assignment to unused variable `y`
+
+ℹ Suggested fix
+164 164 | def f():
+165 165 |     x = 1
+166 166 | 
+167     |-    y = 2
 
 

crates/ruff/src/rules/pylint/rules/useless_return.rs

@@ -109,7 +109,9 @@ pub(crate) fn useless_return(
             checker.locator(),
             checker.indexer(),
         );
-        diagnostic.set_fix(Fix::automatic(edit).isolate(checker.statement_isolation()));
+        diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(Some(
+            checker.semantic().current_statement_id(),
+        ))));
     }
     checker.diagnostics.push(diagnostic);
 }

crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs

@@ -135,7 +135,9 @@ pub(crate) fn unnecessary_builtin_import(
                 checker.stylist(),
                 checker.indexer(),
             )?;
-            Ok(Fix::suggested(edit).isolate(checker.parent_isolation()))
+            Ok(Fix::suggested(edit).isolate(Checker::isolation(
+                checker.semantic().current_statement_parent_id(),
+            )))
         });
     }
     checker.diagnostics.push(diagnostic);

crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs

@@ -124,7 +124,9 @@ pub(crate) fn unnecessary_future_import(checker: &mut Checker, stmt: &Stmt, name
                 checker.stylist(),
                 checker.indexer(),
             )?;
-            Ok(Fix::suggested(edit).isolate(checker.parent_isolation()))
+            Ok(Fix::suggested(edit).isolate(Checker::isolation(
+                checker.semantic().current_statement_parent_id(),
+            )))
         });
     }
     checker.diagnostics.push(diagnostic);

crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs

@@ -66,7 +66,9 @@ pub(crate) fn useless_metaclass_type(
         let stmt = checker.semantic().current_statement();
         let parent = checker.semantic().current_statement_parent();
         let edit = autofix::edits::delete_stmt(stmt, parent, checker.locator(), checker.indexer());
-        diagnostic.set_fix(Fix::automatic(edit).isolate(checker.parent_isolation()));
+        diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(
+            checker.semantic().current_statement_parent_id(),
+        )));
     }
     checker.diagnostics.push(diagnostic);
 }