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

Possible overlap between SIM300 and PLC2201 #1954

Closed
rabelux opened this issue Jan 18, 2023 · 4 comments · Fixed by #1980
Closed

Possible overlap between SIM300 and PLC2201 #1954

rabelux opened this issue Jan 18, 2023 · 4 comments · Fixed by #1980
Assignees

Comments

@rabelux
Copy link

rabelux commented Jan 18, 2023

From my understanding the rules C2201 (PLC2201 in ruff) and SIM300 are pretty much the same.
If that's the case I'd suggest to drop one in favour of the other.

PLC2201 got introduced in #1023, SIM300 in #1539.

@charliermarsh
Copy link
Member

charliermarsh commented Jan 18, 2023

Yeah I agree with this. My gut reaction is to remove PLC2201. In the future, we should be able to map rules to multiple codes for compatibility purposes (see, e.g., #1941 which lays the foundation), although there are some questions to answer around how that works with --select ALL, # noqa , etc.

@rabelux
Copy link
Author

rabelux commented Jan 18, 2023

I'd also prefer to keep SIM300.

Regarding #1941:
I hope the README stays as concise as it is currently? I really love the fact that all error codes are listed in a single spot and I can just ctrl+f to look for codes or phrases (like "concatenation" for example) and can almost expect to not find duplicates.

Maybe introduce a column like "aliases" in the suppurted rules section and also treat these codes as aliases? Meaning noqa statements and config files may mention other codes but ruff tracks them down to its natively known and implemented error codes. Evaluating --select ALL would only look in implemented error codes, not in aliases.

@charliermarsh charliermarsh self-assigned this Jan 18, 2023
@charliermarsh
Copy link
Member

These rules are actually slightly different because the Pylint rule flags any comparison (e.g., i >= 5).

@charliermarsh
Copy link
Member

I'll probably extend SIM300 to flag those cases too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants