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

Rule categorization #1774

Open
not-my-profile opened this issue Jan 10, 2023 · 7 comments
Open

Rule categorization #1774

not-my-profile opened this issue Jan 10, 2023 · 7 comments
Labels
core Related to core functionality

Comments

@not-my-profile
Copy link
Contributor

not-my-profile commented Jan 10, 2023

I think there are two perspectives ruff users may have:

  • using ruff as a replacement for an existing linter (such as pylint or flake8)
  • using ruff from the get-go

The README currently groups rules by their origin. Which I think is suboptimal for both cases. For the first case our README only lists the lints implemented by ruff but doesn't tell you which (or how many) lints ruff is missing. For the second case you don't actually care about where the lints are coming from, you just want to see them grouped by topic.

So I think we should steal a nice feature from Clippy which are lint categories. Clippy has the following categories:

  • correctness: code that is outright wrong or useless
  • suspicious: code that is most likely wrong or useless
  • style: code that should be written in a more idiomatic way
  • complexity: code that does something simple but in a complex way
  • perf: code that can be written to run faster
  • pedantic: lints which are rather strict or have occasional false positives
  • nursery: new lints that are still under development
  • cargo: lints for the cargo manifest

We could simply use the same categorization (except with cargo instead being pyproject as per #1472). I think we would however need some additional categories since there are several lint categories that Rust does not need, for example:

  • syntax: code that is syntactically invalid (syntax errors cannot go by unnoticed in Rust)
  • deprecated: functions that have been deprecated (Rust has a #[deprecated] macro for that) (see also #1507)

What do you think about this?

@charliermarsh
Copy link
Member

I agree that we should kick off a re-categorization effort. Like #1773, will reply in a bit with some more thoughts :)

@charliermarsh
Copy link
Member

I generally agree. I need to give some thought to the right categorization since this will be hard to undo.

My gut reaction is that... we may need more categories? Two that come to mind for me are docstrings and typing (or type-annotations, or whatever).

For the first case our README only lists the lints implemented by ruff but doesn't tell you which (or how many) lints ruff is missing.

It's not really relevant for this debate, but: in the majority of cases, we support the entire plugin. There are a few exceptions.

@charliermarsh charliermarsh added the core Related to core functionality label Jan 11, 2023
@not-my-profile not-my-profile changed the title Lint categorization Rule categorization Jan 19, 2023
@sbrugman
Copy link
Contributor

sbrugman commented Jan 23, 2023

For inspiration/reference: overview of categories used in refurb

Plugin ticket: #1348

@tgross35
Copy link
Contributor

+1 for simplifying categories, it's kind of a mess with 60 categories now (wow that's a lot).

I think it is totally reasonable to recategorize everything and assign coherent numbers for the error output (could use the R prefix and not just RUF since I don't see that used anywhere). With #1256, it would be useful to assign different levels to each lint group and allow the user to override the options, e.g.:

[lints]
suspicious = "allow"
docstrings = "error"

I do think it is still important to keep the flake8 numbers/groups around in the docs and usable for configuration - just since people are going to be looking for flake8 compatibility for the next decade. Doesn't need to be the main promoted style, but I have to imagine it would slow new adoption if enable = ["E", "W"] doesn't continue to work. Maybe there could be a "fix" tool that upgrades the flake8 style codes to whatever the new style is?

@charliermarsh
Copy link
Member

Yeah, that's roughly my thinking too: recategorize the rules, but try to preserve support for the Flake8 compatibility layer at runtime + ship tooling to automatically migration an existing configuration file.

@MichaReiser
Copy link
Member

Related requests are to disable all formatter rules, or deprecated rules

#10225
#9778

@wwuck
Copy link

wwuck commented Sep 24, 2024

For the first case our README only lists the lints implemented by ruff but doesn't tell you which (or how many) lints ruff is missing.

It's not really relevant for this debate, but: in the majority of cases, we support the entire plugin. There are a few exceptions.

Is this the case where ruff implements an entire flake8 plugin but then the plugin later adds new linting checks not covered by ruff? It seems to be hard to keep track of this sort of thing.

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

No branches or pull requests

7 participants