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

B038 false positive in 24.1.15 + 24.1.16 #451

Closed
mikaelarguedas opened this issue Jan 16, 2024 · 5 comments · Fixed by #453
Closed

B038 false positive in 24.1.15 + 24.1.16 #451

mikaelarguedas opened this issue Jan 16, 2024 · 5 comments · Fixed by #453

Comments

@mikaelarguedas
Copy link

mikaelarguedas commented Jan 16, 2024

First of all thanks for this new B038 new rule, it's a great addition!

Using flake8-bugbear 24.1.15

The following code is flagged.

some_list = [1, 2, 3]
for elem in some_list:
    print(elem)
    if elem == 2:
        found_idx = some_list.index(elem)  # should not error
        break

Although a method of some_list is called I believe this doesn't have any effect on the mutable

@mikaelarguedas
Copy link
Author

cc @mimre25 for feedback on this as it may also impact the ruff implementation

@wxtim
Copy link

wxtim commented Jan 16, 2024

Looks similar to

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

for mydict in mydicts:
    if mydicts.get('a', ''):
        print(mydict['foo'])

B038 is complaining about any instance of an object method called inside the loop that object is the iterator for, without checking whether the loop is modifying it.

@cooperlees
Copy link
Collaborator

Agree this looks a bad / annoying false positive. Will take a fix for this asap and release.

Thanks for reporting.

@mimre25
Copy link
Contributor

mimre25 commented Jan 16, 2024

Oh snap, I actually ran this on a quite big code-base as a test and I didn't have any false positives reported.

I'll take a look and see if I can fix this.

Thanks for reporting this and tagging me 🙂

mimre25 added a commit to mimre25/flake8-bugbear that referenced this issue Jan 16, 2024
Previous implementation produced false positives.
This fixes those and adds some more tests.

See PyCQA#451
@cooperlees cooperlees changed the title B038 false positive in 24.1.15? B038 false positive in 24.1.15 + 24.1.16 Jan 16, 2024
@Skrethel
Copy link

I've also encountered this false positive for copy() function

d = {}
for _ in d:
	copied = d.copy()

cooperlees pushed a commit that referenced this issue Jan 17, 2024
* fix(b038): Restrict rule to mutating functions only

Previous implementation produced false positives.
This fixes those and adds some more tests.

See #451

* fix(b038): Add dict.popitem() to list of mutating functions
busgaidw2 added a commit to busgaidw2/flake8-bugbear that referenced this issue Aug 6, 2024
* fix(b038): Restrict rule to mutating functions only

Previous implementation produced false positives.
This fixes those and adds some more tests.

See PyCQA/flake8-bugbear#451

* fix(b038): Add dict.popitem() to list of mutating functions
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants