Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make B007 fix relevance stricter #4607

Merged
merged 1 commit into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions crates/ruff/resources/test/fixtures/flake8_bugbear/B007.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,18 @@ def f():


def f():
# Fixable.
# Unfixable.
for foo, bar, baz in (["1", "2", "3"],):
if foo or baz:
break
else:
bar = 1

print(bar)


def f():
# Unfixable (false negative) due to usage of `bar` outside of loop.
for foo, bar, baz in (["1", "2", "3"],):
if foo or baz:
break
Expand All @@ -85,4 +96,4 @@ def f():
# Unfixable due to trailing underscore (`_line_` wouldn't be considered an ignorable
# variable name).
for line_ in range(self.header_lines):
fp.readline()
fp.readline()
Original file line number Diff line number Diff line change
Expand Up @@ -152,24 +152,19 @@ pub(crate) fn unused_loop_control_variable(checker: &mut Checker, target: &Expr,
);
if let Some(rename) = rename {
if certainty.into() && checker.patch(diagnostic.kind.rule()) {
// Find the `BindingKind::LoopVar` corresponding to the name.
// Avoid fixing if the variable, or any future bindings to the variable, are
// used _after_ the loop.
let scope = checker.semantic_model().scope();
let binding = scope.bindings_for_name(name).find_map(|binding_id| {
let binding = &checker.semantic_model().bindings[*binding_id];
binding.source.and_then(|source| {
(Some(source) == checker.semantic_model().stmt_id).then_some(binding)
})
});
if let Some(binding) = binding {
if binding.kind.is_loop_var() {
if !binding.used() {
#[allow(deprecated)]
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
rename,
expr.range(),
)));
}
}
if scope
.bindings_for_name(name)
.map(|binding_id| &checker.semantic_model().bindings[*binding_id])
.all(|binding| !binding.used())
{
#[allow(deprecated)]
diagnostic.set_fix(Fix::unspecified(Edit::range_replacement(
rename,
expr.range(),
)));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -152,34 +152,35 @@ B007.py:68:14: B007 [*] Loop control variable `bar` not used within loop body
70 70 | break
71 71 |

B007.py:77:14: B007 [*] Loop control variable `bar` not used within loop body
B007.py:77:14: B007 Loop control variable `bar` not used within loop body
|
77 | def f():
78 | # Fixable.
78 | # Unfixable.
79 | for foo, bar, baz in (["1", "2", "3"],):
| ^^^ B007
80 | if foo or baz:
81 | break
|
= help: Rename unused `bar` to `_bar`

ℹ Suggested fix
74 74 |
75 75 | def f():
76 76 | # Fixable.
77 |- for foo, bar, baz in (["1", "2", "3"],):
77 |+ for foo, _bar, baz in (["1", "2", "3"],):
78 78 | if foo or baz:
79 79 | break
80 80 |

B007.py:87:5: B007 Loop control variable `line_` not used within loop body
|
87 | # Unfixable due to trailing underscore (`_line_` wouldn't be considered an ignorable
88 | # variable name).
89 | for line_ in range(self.header_lines):
| ^^^^^ B007
90 | fp.readline()
B007.py:88:14: B007 Loop control variable `bar` not used within loop body
|
88 | def f():
89 | # Unfixable (false negative) due to usage of `bar` outside of loop.
90 | for foo, bar, baz in (["1", "2", "3"],):
| ^^^ B007
91 | if foo or baz:
92 | break
|
= help: Rename unused `bar` to `_bar`

B007.py:98:5: B007 Loop control variable `line_` not used within loop body
|
98 | # Unfixable due to trailing underscore (`_line_` wouldn't be considered an ignorable
99 | # variable name).
100 | for line_ in range(self.header_lines):
| ^^^^^ B007
101 | fp.readline()
|