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

FBT001: exclude boolean operators #14203

Merged
merged 6 commits into from
Nov 10, 2024

Conversation

randolf-scholz
Copy link
Contributor

Fixes #14202

Summary

Exclude rule FBT001 for boolean operators.

Test Plan

Updated existing FBT.py test.

@charliermarsh
Copy link
Member

Should we be ignoring all dunders?

@charliermarsh
Copy link
Member

We have an is_known_dunder_method method that we could use for that.

Copy link
Contributor

github-actions bot commented Nov 10, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@randolf-scholz
Copy link
Contributor Author

@charliermarsh I updated using is_known_dunder_method (this required making crate::rules::flake8_boolean_trap::helpers public).

However, I am not sure if this is the best approach. For one, there is no official list of dunder methods from python's side, and what is considered a known dunder method can change from one version to the next.

Maybe a simpler approach would be to just exclude dunder methods in general? This would be easier to document/teach as well.

@randolf-scholz
Copy link
Contributor Author

The is_known_dunder_method also contains __init__ and __new__, which are probably the two most important functions for FBT001 to check.

@randolf-scholz randolf-scholz marked this pull request as ready for review November 10, 2024 12:02
@dylwil3
Copy link
Collaborator

dylwil3 commented Nov 10, 2024

I think "all dunder methods except init and new" should be easy to maintain and explain.

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.

Thanks -- this looks reasonable to me.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Nov 10, 2024
@charliermarsh charliermarsh enabled auto-merge (squash) November 10, 2024 22:36
@charliermarsh charliermarsh merged commit 5d91ba0 into astral-sh:main Nov 10, 2024
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

FBT001 exclude operators like __and__, __or__ and __xor__, etc.
3 participants