-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
## Summary This PR adds the implementation for the current [flake8-bugbear](https://github.com/PyCQA/flake8-bugbear)'s B038 rule. The B038 rule checks for mutation of loop iterators in the body of a for loop and alerts when found. Rational: Editing the loop iterator can lead to undesired behavior and is probably a bug in most cases. Closes #9511. Note there will be a second iteration of B038 implemented in `flake8-bugbear` soon, and this PR currently only implements the weakest form of the rule. I'd be happy to also implement the further improvements to B038 here in ruff 🙂 See PyCQA/flake8-bugbear#454 for more information on the planned improvements. ## Test Plan Re-using the same test file that I've used for `flake8-bugbear`, which is included in this PR (look for the `B038.py` file). Note: this is my first time using `rust` (beside `rustlings`) - I'd be very happy about thorough feedback on what I could've done better :slightly_smiling_face: - Bring it on :grinning:
- Loading branch information
Showing
9 changed files
with
805 additions
and
0 deletions.
There are no files selected for viewing
160 changes: 160 additions & 0 deletions
160
crates/ruff_linter/resources/test/fixtures/flake8_bugbear/B909.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,160 @@ | ||
""" | ||
Should emit: | ||
B909 - on lines 11, 25, 26, 40, 46 | ||
""" | ||
|
||
# lists | ||
|
||
some_list = [1, 2, 3] | ||
some_other_list = [1, 2, 3] | ||
for elem in some_list: | ||
# errors | ||
some_list.remove(0) | ||
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(0) | ||
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(0) | ||
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 | ||
|
||
# should error | ||
for _ in foo: | ||
if bar: | ||
pass | ||
else: | ||
foo.remove(1) | ||
|
||
# should error | ||
for elem in some_list: | ||
if some_list.pop() == 2: | ||
pass | ||
|
||
# should not error | ||
for elem in some_list: | ||
if some_list.pop() == 2: | ||
break | ||
|
||
# should error | ||
for elem in some_list: | ||
if some_list.pop() == 2: | ||
pass | ||
else: | ||
break | ||
|
||
# should not error | ||
for elem in some_list: | ||
del some_list[elem] | ||
some_list[elem] = 1 | ||
some_list.remove(elem) | ||
some_list.discard(elem) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.