Track fix isolation in unnecessary-pass (#7715)

## Summary

This wasn't necessary in the past, since we _only_ applied this rule to
bodies that contained two statements, one of which was a `pass`. Now
that it applies to any `pass` in a block with multiple statements, we
can run into situations in which we remove both passes, and so need to
apply the fixes in isolation.

See:
https://github.com/astral-sh/ruff/issues/7455#issuecomment-1741107573.
This commit is contained in:
Charlie Marsh 2023-09-29 13:23:04 -04:00 committed by GitHub
parent 974262ad2c
commit 253fbb665f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
8 changed files with 140 additions and 14 deletions

View file

@ -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

View file

@ -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);
}

View file

@ -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);
});

View file

@ -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

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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);
}

View file

@ -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<NodeId> {
self.current_statement_ids().next()
}
/// Return the [`NodeId`] of the current [`Stmt`] parent, if any.