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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flake8-bugbear] Implement loop-iterator-mutation (B909) #9578

Merged
merged 18 commits into from Apr 11, 2024
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
129 changes: 129 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
@@ -0,0 +1,129 @@
"""
Should emit:
B999 - on lines 11, 25, 26, 40, 46
charliermarsh marked this conversation as resolved.
Show resolved Hide resolved
"""

# lists

some_list = [1, 2, 3]
some_other_list = [1, 2, 3]
for elem in some_list:
# errors
some_list.remove(elem)
del some_list[2]
some_list.append(elem)
some_list.sort()
some_list.reverse()
some_list.clear()
some_list.extend([1, 2])
some_list.insert(1, 1)
some_list.pop(1)
some_list.pop()

# conditional break should error
if elem == 2:
some_list.remove(elem)
if elem == 3:
break

# non-errors
some_other_list.remove(elem)
del some_list
del some_other_list
found_idx = some_list.index(elem)
some_list = 3

# unconditional break should not error
if elem == 2:
some_list.remove(elem)
break


# dicts
mydicts = {'a': {'foo': 1, 'bar': 2}}

for elem in mydicts:
# errors
mydicts.popitem()
mydicts.setdefault('foo', 1)
mydicts.update({'foo': 'bar'})

# no errors
elem.popitem()
elem.setdefault('foo', 1)
elem.update({'foo': 'bar'})

# sets

myset = {1, 2, 3}

for _ in myset:
# errors
myset.update({4, 5})
myset.intersection_update({4, 5})
myset.difference_update({4, 5})
myset.symmetric_difference_update({4, 5})
myset.add(4)
myset.discard(3)

# no errors
del myset


# members
class A:
some_list: list

def __init__(self, ls):
self.some_list = list(ls)


a = A((1, 2, 3))
# ensure member accesses are handled as errors
for elem in a.some_list:
a.some_list.remove(elem)
del a.some_list[2]


# Augassign should error

foo = [1, 2, 3]
bar = [4, 5, 6]
for _ in foo:
foo *= 2
foo += bar
foo[1] = 9
foo[1:2] = bar
foo[1:2:3] = bar

foo = {1,2,3}
bar = {4,5,6}
for _ in foo: # should error
foo |= bar
foo &= bar
foo -= bar
foo ^= bar


# more tests for unconditional breaks - should not error
for _ in foo:
foo.remove(1)
for _ in bar:
bar.remove(1)
break
break

# should not error
for _ in foo:
foo.remove(1)
for _ in bar:
...
break

# should error (?)
for _ in foo:
foo.remove(1)
if bar:
bar.remove(1)
break
break
Expand Up @@ -30,6 +30,9 @@ pub(crate) fn deferred_for_loops(checker: &mut Checker) {
if checker.enabled(Rule::EnumerateForLoop) {
flake8_simplify::rules::enumerate_for_loop(checker, stmt_for);
}
if checker.enabled(Rule::LoopIteratorMutation) {
flake8_bugbear::rules::loop_iterator_mutation(checker, stmt_for);
}
}
}
}
1 change: 1 addition & 0 deletions crates/ruff_linter/src/checkers/ast/analyze/statement.rs
Expand Up @@ -1271,6 +1271,7 @@ pub(crate) fn statement(stmt: &Stmt, checker: &mut Checker) {
Rule::UnnecessaryEnumerate,
Rule::UnusedLoopControlVariable,
Rule::YieldInForLoop,
Rule::LoopIteratorMutation,
]) {
checker.analyze.for_loops.push(checker.semantic.snapshot());
}
Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/codes.rs
Expand Up @@ -376,6 +376,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> {
(Flake8Bugbear, "033") => (RuleGroup::Stable, rules::flake8_bugbear::rules::DuplicateValue),
(Flake8Bugbear, "034") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ReSubPositionalArgs),
(Flake8Bugbear, "035") => (RuleGroup::Stable, rules::flake8_bugbear::rules::StaticKeyDictComprehension),
(Flake8Bugbear, "909") => (RuleGroup::Preview, rules::flake8_bugbear::rules::LoopIteratorMutation),
(Flake8Bugbear, "904") => (RuleGroup::Stable, rules::flake8_bugbear::rules::RaiseWithoutFromInsideExcept),
(Flake8Bugbear, "905") => (RuleGroup::Stable, rules::flake8_bugbear::rules::ZipWithoutExplicitStrict),

Expand Down
1 change: 1 addition & 0 deletions crates/ruff_linter/src/rules/flake8_bugbear/mod.rs
Expand Up @@ -61,6 +61,7 @@ mod tests {
#[test_case(Rule::UselessContextlibSuppress, Path::new("B022.py"))]
#[test_case(Rule::UselessExpression, Path::new("B018.ipynb"))]
#[test_case(Rule::UselessExpression, Path::new("B018.py"))]
#[test_case(Rule::LoopIteratorMutation, Path::new("B909.py"))]
fn rules(rule_code: Rule, path: &Path) -> Result<()> {
let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy());
let diagnostics = test_path(
Expand Down