Skip to content

Conversation

@jesse1412
Copy link
Contributor

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

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

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

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

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

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
@jesse1412 jesse1412 deleted the is-comp-mutable-initializer branch November 13, 2023 13:10
charliermarsh added a commit that referenced this pull request Jun 26, 2024
…632`) (#12049)

## Summary

See: #8607. Rare but
uncontroversial.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
…632`) (#12049)

## Summary

See: #8607. Rare but
uncontroversial.
MichaReiser pushed a commit that referenced this pull request Jun 27, 2024
…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

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants