Skip to content

Add check for is comparison with mutable initialisers to rule F632#8607

Merged
charliermarsh merged 3 commits intoastral-sh:mainfrom
jesse1412:is-comp-mutable-initializer
Nov 11, 2023
Merged

Add check for is comparison with mutable initialisers to rule F632#8607
charliermarsh merged 3 commits intoastral-sh:mainfrom
jesse1412:is-comp-mutable-initializer

Conversation

@jesse1412
Copy link
Contributor

@jesse1412 jesse1412 commented Nov 10, 2023

Summary

Adds an extra check to F632 to check for any is comparisons to a mutable initialisers.
Implements #8589 .

Example:

named_var = {}
if named_var is {}:  # F632 (fix)
    pass

The if condition will always evaluate to False because it checks on identity and it's impossible to take the same identity as a hard coded list/set/dict initializer.

Multiple test cases were added to ensure the rule works + doesn't flag false positives + the fix works correctly.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 10, 2023 •
edited
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@vinhocent
Copy link
Contributor

vinhocent commented Nov 10, 2023

Just wondering - what was the original reasoning for not using a match statement in is_constant() @charliermarsh ?

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Nov 10, 2023
@zanieb
Copy link
Member

zanieb commented Nov 10, 2023 •
edited
Loading

We should probably gate this change to only apply when preview mode is enabled per the versioning policy as I think it fits into

The scope of a stable rule is significantly increased

Note on how to do this in another pull request #8608 (comment)

@zanieb
Copy link
Member

zanieb commented Nov 10, 2023

Thanks for contributing :) the rest of the implementation looks good to me. I don't have the answer to your question though.

@jesse1412
Copy link
Contributor Author

jesse1412 commented Nov 10, 2023

I thiiink I've added the preview flag. And thanks for the encouraging words!

charliermarsh
charliermarsh approved these changes Nov 11, 2023
View reviewed changes
Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thank you!

@charliermarsh charliermarsh enabled auto-merge (squash) November 11, 2023 00:20
@charliermarsh charliermarsh added the preview Related to preview mode features label Nov 11, 2023
@charliermarsh charliermarsh merged commit 39728a1 into astral-sh:main Nov 11, 2023
@charliermarsh charliermarsh mentioned this pull request Nov 11, 2023
Closed
@jesse1412 jesse1412 deleted the is-comp-mutable-initializer branch November 13, 2023 13:10
@miccal miccal mentioned this pull request Nov 17, 2023
Merged
@charliermarsh charliermarsh mentioned this pull request Jun 26, 2024
Merged
charliermarsh added a commit that referenced this pull request Jun 26, 2024
@charliermarsh
[pyflakes] Stabilize detection of is comparisons to lists, etc. (`F…
7462545 
…632`) (#12049)

## Summary

See: #8607. Rare but
uncontroversial.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
@charliermarsh @MichaReiser
[pyflakes] Stabilize detection of is comparisons to lists, etc. (`F…
d0fe904 
…632`) (#12049)

## Summary

See: #8607. Rare but
uncontroversial.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
@charliermarsh @MichaReiser
[pyflakes] Stabilize detection of is comparisons to lists, etc. (`F…
c98d8a0 
…632`) (#12049)

## Summary

See: #8607. Rare but
uncontroversial.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@charliermarsh charliermarsh charliermarsh approved these changes

Assignees

No one assigned

Labels

preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jesse1412 @vinhocent @zanieb @charliermarsh