diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F401_0.py b/crates/ruff/resources/test/fixtures/pyflakes/F401_0.py index dcec5beed4..c3a2db56f7 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F401_0.py +++ b/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 diff --git a/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py b/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py index a2e6c623f7..ce7b19a1b3 100644 --- a/crates/ruff/resources/test/fixtures/pyflakes/F841_3.py +++ b/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 diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index ef7b794783..d19d212b19 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/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) -> IsolationLevel { + node_id + .map(|node_id| IsolationLevel::Group(node_id.into())) + .unwrap_or_default() + } } impl<'a, 'b> Visitor<'b> for Checker<'a> diff --git a/crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs b/crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs index 922f40ec19..df9a36c219 100644 --- a/crates/ruff/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs +++ b/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); } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs b/crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs index 51c21e105e..6b9d15327c 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs +++ b/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); } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/pass_in_class_body.rs b/crates/ruff/src/rules/flake8_pyi/rules/pass_in_class_body.rs index fd18b84174..f5d071132e 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/pass_in_class_body.rs +++ b/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); } diff --git a/crates/ruff/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs b/crates/ruff/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs index 203c201f7b..6676c9009e 100644 --- a/crates/ruff/src/rules/flake8_pyi/rules/str_or_repr_defined_in_stub.rs +++ b/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); } diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/empty_type_checking_block.rs b/crates/ruff/src/rules/flake8_type_checking/rules/empty_type_checking_block.rs index 6d249e2d15..eee9638e21 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/empty_type_checking_block.rs +++ b/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); } diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs b/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs index 9fb04c5cee..a3a01523f3 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/runtime_import_in_type_checking_block.rs +++ b/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)), + ), ) } diff --git a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs b/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs index 150a07e3d2..bc88a6e10b 100644 --- a/crates/ruff/src/rules/flake8_type_checking/rules/typing_only_runtime_import.rs +++ b/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)), + ), ) } diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_import.rs b/crates/ruff/src/rules/pyflakes/rules/unused_import.rs index 0ec33b2ad8..3c6c184289 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_import.rs +++ b/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), + ))) } diff --git a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs b/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs index 024aa0eb0f..a748f2c433 100644 --- a/crates/ruff/src/rules/pyflakes/rules/unused_variable.rs +++ b/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 { 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 { diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_0.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_0.py.snap index 4996e8b899..3ae9fc7308 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F401_F401_0.py.snap +++ b/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 diff --git a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap b/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap index 3484053324..85c614015c 100644 --- a/crates/ruff/src/rules/pyflakes/snapshots/ruff__rules__pyflakes__tests__F841_F841_3.py.snap +++ b/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 diff --git a/crates/ruff/src/rules/pylint/rules/useless_return.rs b/crates/ruff/src/rules/pylint/rules/useless_return.rs index f3a6457fd1..eaa8be21c6 100644 --- a/crates/ruff/src/rules/pylint/rules/useless_return.rs +++ b/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); } diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs index ba9ab76cd5..919a26bff6 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_builtin_import.rs +++ b/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); diff --git a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs b/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs index b62de683be..402de0a9a3 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/unnecessary_future_import.rs +++ b/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); diff --git a/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs b/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs index f23fd8339a..df5795f5d9 100644 --- a/crates/ruff/src/rules/pyupgrade/rules/useless_metaclass_type.rs +++ b/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); }