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

Merge rules checking the same violation #2714

Open
2 of 7 tasks
not-my-profile opened this issue Feb 10, 2023 · 6 comments
Open
2 of 7 tasks

Merge rules checking the same violation #2714

not-my-profile opened this issue Feb 10, 2023 · 6 comments
Labels
tracking A "meta" issue that tracks completion of a bigger task via a list of smaller scoped issues.

Comments

@not-my-profile
Copy link
Contributor

not-my-profile commented Feb 10, 2023

As per the rule naming convention established in #2712 the rule name should be the bad thing that's being checked for and not contain the solution, so we should merge the following rules accordingly:

In the long run it would be nice if ruff supported different autofixes for one violation, see #2713.

@charliermarsh
Copy link
Member

Agreed. Not sure which to "remove" (i.e., treat as the canonical rule, as opposed to the redirect), but I guess that gets back to the discussion in #2517.

@not-my-profile
Copy link
Contributor Author

I don't think it matters much which code we map and which one we redirect for now, since #2517 should land soon(tm).

@not-my-profile not-my-profile changed the title Merge use-contextlib-suppress and try-except-pass Merge rules checking the same violation Feb 11, 2023
@charliermarsh
Copy link
Member

charliermarsh commented Feb 12, 2023

To unify blind- and bare-except, we need to change bare-except to flag except Exception and except BaseException. I assume that's fine.

@not-my-profile
Copy link
Contributor Author

Ah right ... good catch! I think except: and except BaseException: are equivalent since every exception must derive BaseException. except Exception is different though since some exceptions e.g. GeneratorExit, KeyboardInterrupt and SystemExit don't derive from it. So we may want to split blind-except into except-any and except-exception.

@charliermarsh
Copy link
Member

We could also make it a setting on the rule, rather than creating separate rules. But I don’t think we’ve articulated a strict philosophy either way there.

@andersk
Copy link
Contributor

andersk commented Feb 14, 2023

use-contextlib-suppress (SIM105) is intentionally much weaker than try-except-pass (S110), and it’s not unreasonable to enable the former without the latter. See #1948.

@dhruvmanila dhruvmanila added the tracking A "meta" issue that tracks completion of a bigger task via a list of smaller scoped issues. label May 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tracking A "meta" issue that tracks completion of a bigger task via a list of smaller scoped issues.
Projects
None yet
Development

No branches or pull requests

4 participants