From 8c2c7cf959ae06cc62cb661e57f82b1b6f2c0cb5 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 29 Sep 2023 13:04:51 -0400 Subject: [PATCH] Track fix isolation in unnecessary-pass --- .../test/fixtures/flake8_pie/PIE790.py | 14 +++ .../rules/duplicate_class_field_definition.rs | 4 +- .../flake8_pie/rules/unnecessary_pass.rs | 4 +- ...__flake8_pie__tests__PIE790_PIE790.py.snap | 112 ++++++++++++++++++ .../rules/ellipsis_in_non_empty_class_body.rs | 4 +- .../flake8_pyi/rules/pass_in_class_body.rs | 4 +- .../src/rules/pylint/rules/useless_return.rs | 4 +- crates/ruff_python_semantic/src/model.rs | 8 +- 8 files changed, 140 insertions(+), 14 deletions(-) diff --git a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py index ae89164ff100a..976b15dbd52ef 100644 --- a/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py +++ b/crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py @@ -134,3 +134,17 @@ def foo(): """A docstring.""" print("foo") pass + + +for i in range(10): + pass + pass + +for i in range(10): + pass + + pass + +for i in range(10): + pass # comment + pass diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs index eba237f90c6df..d724e6baf605e 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/duplicate_class_field_definition.rs @@ -82,9 +82,9 @@ pub(crate) fn duplicate_class_field_definition(checker: &mut Checker, body: &[St if checker.patch(diagnostic.kind.rule()) { let edit = fix::edits::delete_stmt(stmt, Some(stmt), checker.locator(), checker.indexer()); - diagnostic.set_fix(Fix::suggested(edit).isolate(Checker::isolation(Some( + diagnostic.set_fix(Fix::suggested(edit).isolate(Checker::isolation( checker.semantic().current_statement_id(), - )))); + ))); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs index 433aeaa0b78e9..eaea78bd5c941 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs +++ b/crates/ruff_linter/src/rules/flake8_pie/rules/unnecessary_pass.rs @@ -70,7 +70,9 @@ pub(crate) fn no_unnecessary_pass(checker: &mut Checker, body: &[Stmt]) { } else { fix::edits::delete_stmt(stmt, None, checker.locator(), checker.indexer()) }; - diagnostic.set_fix(Fix::automatic(edit)); + diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation( + checker.semantic().current_statement_id(), + ))); } checker.diagnostics.push(diagnostic); }); diff --git a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap index c7ad48b9af144..1107602350c2c 100644 --- a/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap +++ b/crates/ruff_linter/src/rules/flake8_pie/snapshots/ruff_linter__rules__flake8_pie__tests__PIE790_PIE790.py.snap @@ -349,5 +349,117 @@ PIE790.py:136:5: PIE790 [*] Unnecessary `pass` statement 134 134 | """A docstring.""" 135 135 | print("foo") 136 |- pass +137 136 | +138 137 | +139 138 | for i in range(10): + +PIE790.py:140:5: PIE790 [*] Unnecessary `pass` statement + | +139 | for i in range(10): +140 | pass + | ^^^^ PIE790 +141 | pass + | + = help: Remove unnecessary `pass` + +ℹ Fix +138 138 | +139 139 | for i in range(10): +140 140 | pass +141 |- pass +142 141 | +143 142 | for i in range(10): +144 143 | pass + +PIE790.py:141:5: PIE790 [*] Unnecessary `pass` statement + | +139 | for i in range(10): +140 | pass +141 | pass + | ^^^^ PIE790 +142 | +143 | for i in range(10): + | + = help: Remove unnecessary `pass` + +ℹ Fix +138 138 | +139 139 | for i in range(10): +140 140 | pass +141 |- pass +142 141 | +143 142 | for i in range(10): +144 143 | pass + +PIE790.py:144:5: PIE790 [*] Unnecessary `pass` statement + | +143 | for i in range(10): +144 | pass + | ^^^^ PIE790 +145 | +146 | pass + | + = help: Remove unnecessary `pass` + +ℹ Fix +141 141 | pass +142 142 | +143 143 | for i in range(10): +144 |- pass +145 144 | +146 145 | pass +147 146 | + +PIE790.py:146:5: PIE790 [*] Unnecessary `pass` statement + | +144 | pass +145 | +146 | pass + | ^^^^ PIE790 +147 | +148 | for i in range(10): + | + = help: Remove unnecessary `pass` + +ℹ Fix +143 143 | for i in range(10): +144 144 | pass +145 145 | +146 |- pass +147 146 | +148 147 | for i in range(10): +149 148 | pass # comment + +PIE790.py:149:5: PIE790 [*] Unnecessary `pass` statement + | +148 | for i in range(10): +149 | pass # comment + | ^^^^ PIE790 +150 | pass + | + = help: Remove unnecessary `pass` + +ℹ Fix +146 146 | pass +147 147 | +148 148 | for i in range(10): +149 |- pass # comment + 149 |+ # comment +150 150 | pass + +PIE790.py:150:5: PIE790 [*] Unnecessary `pass` statement + | +148 | for i in range(10): +149 | pass # comment +150 | pass + | ^^^^ PIE790 + | + = help: Remove unnecessary `pass` + +ℹ Fix +147 147 | +148 148 | for i in range(10): +149 149 | pass # comment +150 |- pass diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs index d503c99b846ca..9a6e2d38667d8 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/ellipsis_in_non_empty_class_body.rs @@ -66,9 +66,9 @@ pub(crate) fn ellipsis_in_non_empty_class_body(checker: &mut Checker, body: &[St if checker.patch(diagnostic.kind.rule()) { let edit = fix::edits::delete_stmt(stmt, Some(stmt), checker.locator(), checker.indexer()); - diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(Some( + diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation( checker.semantic().current_statement_id(), - )))); + ))); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs index b6275313567d0..9b39eec211372 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/pass_in_class_body.rs @@ -37,9 +37,9 @@ pub(crate) fn pass_in_class_body(checker: &mut Checker, class_def: &ast::StmtCla if checker.patch(diagnostic.kind.rule()) { let edit = fix::edits::delete_stmt(stmt, Some(stmt), checker.locator(), checker.indexer()); - diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(Some( + diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation( checker.semantic().current_statement_id(), - )))); + ))); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_linter/src/rules/pylint/rules/useless_return.rs b/crates/ruff_linter/src/rules/pylint/rules/useless_return.rs index 3effece5698cb..104ca92bc9146 100644 --- a/crates/ruff_linter/src/rules/pylint/rules/useless_return.rs +++ b/crates/ruff_linter/src/rules/pylint/rules/useless_return.rs @@ -106,9 +106,9 @@ pub(crate) fn useless_return( if checker.patch(diagnostic.kind.rule()) { let edit = fix::edits::delete_stmt(last_stmt, Some(stmt), checker.locator(), checker.indexer()); - diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation(Some( + diagnostic.set_fix(Fix::automatic(edit).isolate(Checker::isolation( checker.semantic().current_statement_id(), - )))); + ))); } checker.diagnostics.push(diagnostic); } diff --git a/crates/ruff_python_semantic/src/model.rs b/crates/ruff_python_semantic/src/model.rs index f8e3f8438661c..8f742ae452ada 100644 --- a/crates/ruff_python_semantic/src/model.rs +++ b/crates/ruff_python_semantic/src/model.rs @@ -887,11 +887,9 @@ impl<'a> SemanticModel<'a> { .filter(|id| self.nodes[*id].is_statement()) } - /// Return the [`NodeId`] of the current [`Stmt`]. - pub fn current_statement_id(&self) -> NodeId { - self.current_statement_ids() - .next() - .expect("No current statement") + /// Return the [`NodeId`] of the current [`Stmt`], if any. + pub fn current_statement_id(&self) -> Option { + self.current_statement_ids().next() } /// Return the [`NodeId`] of the current [`Stmt`] parent, if any.