Drop a few rules from the preview default set#23879
Merged
Conversation
Summary -- This PR incorporates some feedback from internal discussions on [Discord] and from #23203 into the preview default rule set. In particular, we drop: - [`PERF401`](https://docs.astral.sh/ruff/rules/PERF401) - [`PERF403`](https://docs.astral.sh/ruff/rules/PERF403) - [`PLR1714`](https://docs.astral.sh/ruff/rules/PLR1714) - [`RET504`](https://docs.astral.sh/ruff/rules/RET504) - [`SIM102`](https://docs.astral.sh/ruff/rules/SIM102) - [`SIM114`](https://docs.astral.sh/ruff/rules/SIM114) - [`TRY300`](https://docs.astral.sh/ruff/rules/TRY300) A couple of these disagree a bit with Clippy's own categorization. For example, our SIM102 is [collapsible_if], SIM114 is [if_same_then_else], and RET504 is [let_and_return], all of which are Style lints for Clippy. But if users find them annoying in Python, it makes sense to me to bump them down to Pedantic. PERF401 and PERF403 also have open issues (#21890, #21891), so I think it makes sense to consider them lower severity, at least until those are resolved. TRY300 and PLR1714 are just a bit more pedantic than expected and some people prefer the styles they flag. [Discord]: https://discord.com/channels/1039017663004942429/1082324250112823306/1478345981916348526 [let_and_return]: https://rust-lang.github.io/rust-clippy/master/?search=assign#let_and_return [collapsible_if]: https://rust-lang.github.io/rust-clippy/master/?search=collap#collapsible_if [if_same_then_else]: https://rust-lang.github.io/rust-clippy/master/?search=then_#if_same_then_else Test Plan -- Future default users
these are based on internal adoption feedback from: https://discord.com/channels/1039017663004942429/1082324250112823306/1478345981916348526
this came up both internally and in the discussion, and it feels similar in spirit to SIM114
|
| code | total | + violation | - violation | + fix | - fix |
|---|---|---|---|---|---|
| RET504 | 457 | 0 | 457 | 0 | 0 |
| TRY300 | 118 | 0 | 118 | 0 | 0 |
| PERF401 | 107 | 0 | 107 | 0 | 0 |
| PLR1714 | 36 | 0 | 36 | 0 | 0 |
| PERF403 | 5 | 0 | 5 | 0 | 0 |
Member
|
I'll personally be a tiny bit sad if SIM102 and SIM114 don't make it into the defaults. They seem like easy wins for both readability and simplicity. |
amyreese
approved these changes
Mar 10, 2026
Contributor
Author
Yeah I'm a bit torn on these too. I think they're nice in simple cases like the ones in the docs, but I just ignored a Clippy lint for SIM114 today when I wanted the same code in each branch, and I also found some of the real examples on Discord clearly less readable. For example: -if something in [value1, value2] and another > condition:
- do_something()
-elif something_else == "a string" and condition:
+if something in [value1, value2] and another > condition or something_else == "a string" and condition:
do_something()Now I have to think about precedence, and it gets reformatted like this: if (
something in [value1, value2]
and another > condition
or something_else == "a string"
and condition
):
do_something()I don't feel super strongly, but they have come up multiple times from different sources. |
Member
|
Wow, that suggested change should really come with wrapping parens to clarify OoO and where it previously came from, because that precedence is rough. |
MichaReiser
approved these changes
Mar 11, 2026
Member
MichaReiser
left a comment
There was a problem hiding this comment.
I feel similar to @amyreese. Should we leave the two SIM rules in the default set for now and improve their wording for better readability to see if that changes how the rules are perceived? We can still move them out later.
Contributor
Author
|
Sure, happy to leave the SIM rules in. I'll open a follow-up issue for improving the fixes. |
ntBre
added a commit
to ntBre/default-rules
that referenced
this pull request
Mar 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR incorporates some feedback from internal discussions on Discord and
from #23203 into the preview default rule set. In particular, we drop:
PERF401PERF403PLR1714RET504TRY300A couple of these disagree a bit with Clippy's own categorization. For example, RET504 is
let_and_return, which is a Style lint for Clippy. But if users find
it annoying in Python, it makes sense to me to bump it down to Pedantic.
PERF401 and PERF403 also have open issues (#21890, #21891), so I think it makes
sense to consider them lower severity, at least until those are resolved.
TRY300 and PLR1714 are just a bit more pedantic than expected and some people
prefer the styles they flag.
Test Plan
Future default users