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

Add support for rule aliasing #2186

Open
charliermarsh opened this issue Jan 26, 2023 · 15 comments
Open

Add support for rule aliasing #2186

charliermarsh opened this issue Jan 26, 2023 · 15 comments
Labels
core Related to core functionality

Comments

@charliermarsh
Copy link
Member

charliermarsh commented Jan 26, 2023

This is known thing we're working on and have mentioned in a bunch of other issues and PRs.

Apart from tracking the implementation, I want to use this issue to catalog any known aliases:

  • PGH002 (pygrep-hooks) is an alias of G010 (flake8-logging-format)
  • B904 is an alias of TRY200 (see: Implement flake8-raise #2347)
  • R100 (flake8-raise) is an alias of TRY200
  • R101 (flake8-raise) is an alias of TRY201

These aren't one-to-one-aliases, but:

  • UP031 and UP032 reimplement flynt
  • RUF100 reimplements yesqa
@charliermarsh charliermarsh added the core Related to core functionality label Jan 26, 2023
@charliermarsh
Copy link
Member Author

The Pylint rules are mapped to Ruff aliases in #970.

@not-my-profile
Copy link
Contributor

not-my-profile commented Jan 26, 2023

I'd call it "many-to-many mapping between codes and rules" since I think we also want to move away from rules having a primary code and rules needing a code at all (see #1773) and since we also want to support the mapping of one code to multiple rules.

Note that I have already nearly implemented this (other than being recently side-tracked by my CLI refactor #2155). I still want to perform the switch to subcommands (#2190) before tackling this issue, since introducing a new subcommand to list linters would help with tackling this issue :)

@sbrugman
Copy link
Contributor

sbrugman commented Jan 26, 2023

flake8-use-pathlib (PTH) is an alias for most of refurbs pathlib violations

Just adding a thought to the discussion: users may care about ruff's plugin coverage before making the switch. Developers might also wonder which are still open. If the many-to-many mapping registers aliases (and perhaps also won't implement) this could be computed/documented automatically.

PGH001 no-eval = PLW0123 eval-used
N804 = PLC0202

@charliermarsh
Copy link
Member Author

I hadn't really considered a many-to-many mapping -- I'd been assuming one-to-many. Interesting.

@charliermarsh
Copy link
Member Author

SIM104 is an alias for UP028.

@Skylion007
Copy link
Contributor

PIE792 would be an alias of UP004 if we implemented it.

@Skylion007
Copy link
Contributor

This problem is only growing as we implement more and more and rules and forces us to make decision like: #7506 We really should revisit this and try to make it a higher pri feature to prevent the maintainer burden / tech debt from continuing to grow.

@Skylion007
Copy link
Contributor

There are a ton of aliases in pylint rules: #970

@charliermarsh
Copy link
Member Author

charliermarsh commented Sep 30, 2023

Yeah the rule system has been a highly important but hard-to-prioritize thing for a while now. Candidly, I don't know that rule aliasing is actually the right solution -- I feel like it addresses a symptom but creates other problems. For example, if a rule is aliased, does it show up twice if you enable it and the alias? If not, which code should it use? When you select a rule via a code that's an alias for that rule, what code shows up when we report diagnostics? Is it the code you selected, or the canonical code, which you didn't? How do # noqa identifiers work for aliased rules? Etc.

I want to view the problem holistically. Ideally, I'd like to recategorize all the rules into semantically meaningful categories, and add tooling to aid in migrations from Flake8 to Ruff (and from older to newer Ruff versions).

@Skylion007
Copy link
Contributor

I feel like it addresses a symptom but creates other problems. For example, if a rule is aliased, does it show up twice if you enable it and the alias?

I think the more explicit use case here is category rules. If a rule is selected by categories, both aliases/rule codes will appear (they can both indicate the same problem). Flake8 has this problem when multiple plugins are enabled after all so we can copy their display logic. The main difference is that we actually deduplicate the rules on the backend?

noqa: presumably noqa would need to block both rule codes? We could add a command that automatically "cannonicalizes" the aliases. Once again, we can look to flake8, copy what they have an migrate to our own better version gradually.

I agree all these problems are pretty good to fix in the long term, but many of have workarounds or solution courtesy flake8's admittingly flawed rule code system. While we are borrowing their rule code system, why not also borrow their way of handling duplicate/overlapping rules too?

My biggest concern currently is that having all these alias written down in the codebase somewhere instead of scattered throughout issues will make it easier to migrate to whatever final system we end up choosing.

@charliermarsh
Copy link
Member Author

Yeah, that's true, one model for aliases (similar to what you're describing) would be that the user-facing behavior is entirely unchanged from (e.g.) having two separate rules, and instead it's just an abstraction within the codebase to allow exposing a violation under multiple codes, which lets us categorize these internally and ensure that (e.g.) all Pylint rules are added when you enable Pylint.

This would have the nice effect that enabling Pylint gives you all the Pylint rules.

The downside is that we'd still be exposing the same rule multiple times in the docs and elsewhere (so, e.g., would #7506 be solved?), and if you run with ALL or overlapping categories, you'd see the same violation multiple times. Both of these could be sources of confusion (though I'm not commenting here on whether it's the right tradeoff or not).

@Skylion007
Copy link
Contributor

True, but this tradeoff has to be balanced with the current behavior where enabling the PyLint code does not actually enable all the pylint checks we support.

@zanieb
Copy link
Member

zanieb commented Oct 1, 2023

It feels like this capability might just be a stop-gap until we recategorize the rules, but it's hard to know when we'll do that. I think I may prefer to focus our efforts on recategorization.

@mikaelarguedas
Copy link
Contributor

mikaelarguedas commented Jan 6, 2024

A couple other rule aliasing (not duplicated within Ruff but useful to know for displaying to people migrating or for a migration tool)

(flake8-super) SPR100 : UP008
(flake8-bugbear) B035 : RUF011
(flake8-loopy) LPY100 : FURB148
(flake8-tuple) T801 : COM818
(flake8-simplify) SIM203 : E713

@mscheifer
Copy link

I think B036 is an alias of BLE001.

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