B038 false positives

[xqt]

having the following code;

 mylist = list(range(5))
 for item in mylist:
    if item == 2:
        mylist.remove(item)  # <- badly detects B038 here
        break

bugbear badly detects B038.

Sample: https://doc.wikimedia.org/pywikibot/master/_modules/proofreadpage.html#ProofreadPage
in index property (search for def index). As B038 is descibed as Found a mutation of a mutable loop iterable inside the loop body. Changes to the iterable of a loop such as calls to list.remove() or via del can cause unintended bugs. This rule should be an optionated warning instead of a general rule.

[JelleZijlstra]

cc @mimre25.

I think we can fix this by making the rule not trigger if the mutation is unconditionally followed by a break (or return or raise).

[mimre25]

Oh this is good one.

I think excluding mutations that are followed by a break (unconditionally) sounds like a good fix.

Should I loop this into #454 ?

[xqt]

I also found an issue when iterating over a CookieJar instance where __iter__ already uses a copy with deepvalues method. And a third false positive when iterating over a DequeGenerator within our repository:

• https://gerrit.wikimedia.org/r/c/pywikibot/core/+/991742/
• https://phabricator.wikimedia.org/T355372

Therefore I propose to make this rule optional.

[mimre25]

Oh my... with all those false positives stemming from the standard lib, I also think it's best to make the rule optional.

What do you think @JelleZijlstra @cooperlees?

[cooperlees]

I'll accept the PR if we all feel we should move this to a B9XX optional due to our learning here. Thanks all!