-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Add rule WPS111 from wemake-python-styleguide #5632
Conversation
3e366fb
to
bdf992d
Compare
bdf992d
to
0212858
Compare
PR Check ResultsEcosystem✅ ecosystem check detected no linter changes. ✅ ecosystem check detected no format changes. |
0212858
to
f8e0833
Compare
use once_cell::sync::Lazy; | ||
use regex::Regex; | ||
|
||
const ALIAS_NAMES_WHITELIST: [&str; 7] = ["np", "pd", "df", "plt", "sns", "tf", "cv"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const ALIAS_NAMES_WHITELIST: [&str; 7] = ["np", "pd", "df", "plt", "sns", "tf", "cv"]; | |
const ALIAS_NAMES_ALLOWLIST: [&str; 7] = ["np", "pd", "df", "plt", "sns", "tf", "cv"]; |
Or what I like to do (if you don't need to enumerate the values) is to write a match
statement instead:
if matches!(x, "np" | "pd" | "df" | ...)
The compiler will generate efficient code for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure what to do here, because it is a potential conflict with another linter. flake8-import-conventions defines convential import aliases here. Actually I would like to use these as the single source of truth for allowed aliases. This list is also configurable via settings (see here) in which case I would also like to add these to the allowlist.
How should this be done?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
crates/ruff/src/rules/wemake_python_styleguide/helpers/naming.rs
Outdated
Show resolved
Hide resolved
crates/ruff/src/rules/wemake_python_styleguide/helpers/naming.rs
Outdated
Show resolved
Hide resolved
crates/ruff/src/rules/wemake_python_styleguide/helpers/naming.rs
Outdated
Show resolved
Hide resolved
crates/ruff/src/rules/wemake_python_styleguide/rules/too_short_name.rs
Outdated
Show resolved
Hide resolved
38e6452
to
ba72adb
Compare
crates/ruff/src/rules/wemake_python_styleguide/helpers/naming.rs
Outdated
Show resolved
Hide resolved
crates/ruff/src/rules/wemake_python_styleguide/rules/too_short_name.rs
Outdated
Show resolved
Hide resolved
5e8962f
to
bf62b06
Compare
14782ad
to
0387893
Compare
f930ab7
to
f408885
Compare
152ffd3
to
de3e841
Compare
a092918
to
1b291c6
Compare
cba38d9
to
a6f80d5
Compare
a6f80d5
to
cc90294
Compare
cc90294
to
711c72e
Compare
711c72e
to
2d03f74
Compare
Thank you, @bo5o, for working on this rule and keeping the PR up to date. We think this is a valuable rule for many but don't feel comfortable merging it today because it is very opinionated, and everyone using I'm closing this PR for now because I don't want you to spend more time updating the PR, but I hope we can resurrect this PR once #1774 is completed. I'm sorry for the late reply. We should have made this decision sooner. |
Summary
Implements WPS111 from wemake-python-styleguide, as part of #3845.
I started by merging latest upstream changes into an existing PR to revive this implementation. However, after looking at it more closely, I realized that the existing implementation only covers the most basic case of WPS111.
After studying the original code a bit more I updated my implementation to match the original more closely and emit the same behavior. The result is a quite substantial difference compared to the existing PR for this rule, so I decided to make it a separate one, supposed to supersede the previous.
Test Plan
I added a fixture for snapshot testing and copied unit tests from the original logic.