ruff icon indicating copy to clipboard operation
ruff copied to clipboard

[`flake8-raise`] Add Plugin and `R102` Rule

Open saadmk11 opened this issue 3 years ago • 6 comments

closes https://github.com/charliermarsh/ruff/issues/2347

saadmk11 avatar Jan 30 '23 14:01 saadmk11

I'm tempted to put this in tryceratops rather than introduce a new category. But we don't really have any precedent for doing that. I need to think on it.

charliermarsh avatar Jan 30 '23 15:01 charliermarsh

@charliermarsh I thought the same... It's a tricky one.

I guess in the fullness of time it might make sense to not group rules in ruff by the flake8 plugin they came from, but have a "lookup" for each plugin that states what is implemented and what rule it maps to in ruff, e.g.

flake8-raise:R100 → ruff:reraise-no-cause
flake8-raise:R101 → ruff:verbose-raise
flake8-raise:R102 → ruff:unnecessary-paren-on-raise-exception
tryceratops:TC200 → ruff:reraise-no-cause
tryceratops:TC201 → ruff:verbose-raise

And any rules that are intentionally not implemented, and the reason for their omission, can also be explained.

In a sense, the name of the plugin only feels relevant while transitioning to ruff. The main benefit is to have a comprehensive compatibility mapping[^1] to help people migrate and understand what has become of the rules they've been using. I guess that ties in somewhat with the discussions in #1773. (It would also be good to review those friendly names with respect to each other to ensure they make sense where they've been implemented in little islands before, e.g. reraise-no-cause might be better as reraise-without-from, verbose-raise might be better as reraise-redundant-argument, and unnecessary-paren-on-raise-exception could be raise-with-empty-parentheses?

[^1]: Might see if I can get around to this by dredging the issue tracker...

ngnpope avatar Jan 30 '23 16:01 ngnpope

But we don't really have any precedent for doing that.

I think this furthers my point about:

In a sense, the name of the plugin only feels relevant while transitioning to ruff.

ngnpope avatar Jan 30 '23 16:01 ngnpope

@ngnpope - Yeah this all makes sense. I've written about it a bit before, mostly here, but we'll eventually move towards a Ruff-first API that looks less like a Flake8 compatibility layer (while still retaining that layer). In that world, all of those rules can be categorized under the same exceptions category (or whatever's appropriate) rather than being split up by Flake8 plugins.

Soon, we're also going to enable a many-to-many mapping between these Flake8-like codes and the actual Ruff rules. So we'll be able to point both TRY200 and R100 to the same underlying rule, which would help too.

charliermarsh avatar Jan 30 '23 17:01 charliermarsh

I think the right thing to do here, for consistency, is to add this as flake8-raise, and then we can alias the R100 and R101 rules once aliasing is up and running.

However -- we should use a different prefix, as R is too generic. RSE or RAI would be reasonable to me?

charliermarsh avatar Jan 30 '23 17:01 charliermarsh

Agreed on switching the prefix. Either would work, but I guess RSE is potentially less likely to conflict with something else?

ngnpope avatar Jan 30 '23 17:01 ngnpope

However -- we should use a different prefix, as R is too generic. RSE or RAI would be reasonable to me?

RSE sounds good to me. Updated the PR with it. :)

saadmk11 avatar Jan 31 '23 11:01 saadmk11