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

PERF101 False positive #8746

Closed
NeilGirdhar opened this issue Nov 17, 2023 · 4 comments · Fixed by #8809
Closed

PERF101 False positive #8746

NeilGirdhar opened this issue Nov 17, 2023 · 4 comments · Fixed by #8809
Assignees
Labels
bug Something isn't working rule Implementing or modifying a lint rule

Comments

@NeilGirdhar
Copy link

NeilGirdhar commented Nov 17, 2023

From the CPython source:

        klasses = [
            Mock, MagicMock, NonCallableMock, NonCallableMagicMock
        ]
        for Klass in list(klasses):  # PERF101 [*] Do not cast an iterable to `list` before iterating over it
            klasses.append(lambda K=Klass: K(spec=Anything))
            klasses.append(lambda K=Klass: K(spec_set=Anything))

The auto-fix rule removes list, which breaks this code.

@zanieb
Copy link
Member

zanieb commented Nov 17, 2023

What type is klasses? How does this break the code?

@NeilGirdhar
Copy link
Author

NeilGirdhar commented Nov 17, 2023

It's a list. Removing the copy means that the append statements below would modify the list that's being iterated over. I edited in the context.

@zanieb
Copy link
Member

zanieb commented Nov 17, 2023

I see that makes sense. We can probably special case append calls to the inner item which would then not trigger the rule. Separately, I think this code should use copy(klasses) which is much clearer.

@NeilGirdhar
Copy link
Author

NeilGirdhar commented Nov 17, 2023

I agree with you that copy would have been clearer. (Not my code and I can't change it.)

@zanieb zanieb added bug Something isn't working rule Implementing or modifying a lint rule labels Nov 17, 2023
@dhruvmanila dhruvmanila self-assigned this Nov 21, 2023
dhruvmanila added a commit that referenced this issue Nov 21, 2023
## Summary

Avoid `PERF101` if there's an append in loop body

## Test Plan

Add new test cases for this pattern.

fixes: #8746
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants