-
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
Human-friendly rule names #1773
Comments
I agree with this and I want to do it! Will reply in a bit with some more thoughts. |
Okay, so like I said -- I agree! And I think there's a lot of appetite for this (I know I've heard it from @andersk and @smackesey to name a few). We should definitely do it. A few comments:
Separately:
This I appreciate too. I've thought about it in the past (in particular, whether the rules should describe the positive form or the negative behavior), but we've never been consistent on it. (Some of the names were also drawn from the originating plugins, which explains some of the discrepancy.) |
Oh, it's worth mentioning: the one thing we lose by moving away from the |
I use the prefix categories heavily, as in Also I can remember a lot of codes but could not remember a lot of long names. And also wouldn't want to name them in a |
I'd actually like to use the human friendly names as |
+1 for switching to human friendly names everywhere eventually. I think we should pivot away from the current flake8-plugin grouping of rules entirely to rule categories (as per #1774). That includes the way that the rules are grouped in the repository. And then only maintain a mapping of flake8-plugins/codes to ruff rules to show what has been implemented to aid migration. Ideally I'd like to see this documentation piece also refer to the original code prefixes (especially where ruff has changed them) and also state reasons for not implementing rules that the original plugin did, or why the implementation differs, e.g. This would also overcome some of the confusion highlighted in #2517 as well as the fact that many plugins have overlap with each other so they're implemented in some areas in the code, but not others. A couple of other thoughts:
|
Oh please yes! I've just started trying out migrating to Ruff, and here's an example of a bunch of extra comments for that: https://github.com/Avasam/speedrun.com_global_scoreboard_webapp/blob/6678056ac2d42ee66b97fc074515855646f2bfc4/pyproject.toml |
This would make def complex_func(): # noqa: C901
... vs. def compoex_func(): # noqa: complex-structure
... |
What needs to be done to get this implemented? What's left and can a beginner in this repo help out? |
I pretty much agree with what was already summarized above. One lesson from pylint: rules will need multiple names. Only one should be considered the primary (presumably used in the error message and suggested for addition to noqa comments or use in a modern suppression config elsewhere). That way when you don't really like an existing name or come up with a more meaningful alternative name, or need to align with another tools name, you can. But all existing configs and directives in code still remain valid. |
Yes, the refactor to be able to have multiple aliases after the fact was very painful.
In pylint we call this, "old_names" and we have a primary name the only one that is active (it has an impact in the documentation in particular), but in fact |
Do you think there is enough consensus to start extending the rule selectors to accept the current human-readable names in addition to the legacy codes? Happy to give it a try. Selecting by human-readable names is a stepping stone to presets/recategorisation. We could start off with extending support I see there has been an attempt to extend selection for |
We have been running the changes proposed in #11757 internally at Pexip for the last few months, and have been very happy with the experience. While the linter output changes are useful, I agree they are not required for this PR. I'll work on removing that part to make the PR more manageable. |
I also have a very good use case for |
I suggest to first discuss it on the issue before submitting a PR. But I'm interested to hear about your use case. |
Pylint, ESLint and Clippy all let you enable or disable lints via human-friendly names e.g. Pylint lets you configure:
The idea is to also support such human-friendly names for ruff in
pyproject.toml
, while still somehow supporting the numeric codes for backwards compatibility. The reasoning being that symbolic names are much more human-friendly than cryptic numeric codes. Case in point projects that care about readable configs end up adding comments to every numeric code in their config: zulip, fastapi, sphinxCurrently Ruff requires every rule to have a numeric code, which I don't think makes much sense given that Ruff also has Ruff-specific rules ... and I don't think we should have to introduce new cryptic numeric codes when we introduce a new Ruff-specific rule. This obviously requires quite some changes to the codebase/UI but I think doing this is worth it.
In particular I would like us to follow the Rust naming convention for lints:
I think this is a very nice convention because it results in descriptive names. And currently many of our lint names are not descriptive, e.g. TrueFalseComparison should probably rather be called EqualityComperatorForTrueOrFalse, UnnecessaryMap should probably rather be called MapWithLambdaInsteadOfComprehension.
(Previous discussion: #967)
The text was updated successfully, but these errors were encountered: