Enable Ruff SIM

[Avasam]

Ref #13295
https://docs.astral.sh/ruff/rules/#flake8-simplify-sim

Most rules are autofixable

---

Noting the rules we didn't select, and why, for future references:

ignore = [
    # Long single-line ternaries tend to be less readable with our 130 line-length
    "SIM102", # Use a single if statement instead of nested if statements
    "SIM108", # Use ternary operator {contents} instead of if-else-block
    # Explicit short-circuiting loops can be more readable and any/all on a single line
    "SIM110", # Use `{replacement}` instead of `for` loop (reimplemented-builtin)
    # contextlib.suppress is roughly 3x slower than try/except
    "SIM105", # Use contextlib.suppress() instead of try-except-pass
    # Can be less readable on long lines if the dict isn't extracted, possible performance implications
    "SIM116", # Use a dictionary instead of consecutive `if` statements
    # Unsure if more readable in all cases
    "SIM117", # Use a single `with` statement with multiple contexts instead of nested `with` statements
]

About suppressible-exception (SIM105):

Note that contextlib.suppress is slower than using try-except-pass directly. For performance-critical code, consider retaining the try-except-pass pattern.

[Avasam]

astral-sh/ruff#15153

[JelleZijlstra]

I feel these mostly make the code less readable.

[Avasam]

I feel these mostly make the code less readable.

I kinda like SIM110 using all. But for the rest yeah, ternary conditions aren't super readable with our line length of 130 and black/ruff's lack of vertical formatting for ternaries and comprehensions. astral-sh/ruff#11753 (comment)

[AlexWaygood]

I would revert the changes related to if-else-block-instead-of-if-exp (SIM108) and ignore that one. I just don't really agree that it's always an improvement to use a ternary expression instead of explicit if/else branches.

I don't mind most of the other changes, but the first stubsabot.py change (related to collapsible-if (SIM102)) makes it much less readable in my opinion. There's just too much logic crammed into one condition when it's written like that. I don't think I'd want that rule enabled in CI if it means we have to spend time during code review debating whether suggestions like that make code more or less readable; it just isn't a great use of our time :-)

In general this category has quite a few opinionated rules, and I'm not sure I'd want the whole category enabled. I think I'd probably want to think explicitly about which rules make useful, uncontroversial suggestions and which don't.

[Avasam]

Wholly agree with you Alex.

I'm a fan of ternaries in other languages, but Python formatting and the need for parenthesis or \ never did them justice.

For the sake of discussion, I'm offering an alternative to the SIM110 lint, since the issue, imo, is formatting. (whitespaces are still formatted, it just prevents black from unwrapping). But wouldn't mind just noqa'ing this instance.

[JelleZijlstra]

Why is this needed? This looks like how Black would format the code anyway.

[Avasam]

With our 130 line length black puts the whole thing on a single line. Which greatly affected readability. (Black and Ruff don't tend to have great vertical formatting for ternaries and comprehensions when the line length is long).

For the sake of discussion about the "reimplemented-builtin (SIM110)" rule, I worked around this formatting issue

[JelleZijlstra]

If you need to manually format the result, that seems like an argument against applying this "simplification" rule.

[Avasam]

Apparently the 3.9 and 3.10 branches were duplicate (manual fix)

[AlexWaygood]

I still think I'd rather explicitly list the rules we think are useful here, rather than enabling the whole category and disabling specific rules. This category is just too hit-and-miss for me: some of the rules are pretty clear wins, but quite a few make some very opinionated suggestions that have high false-positive rates. Even if some of those rules don't have any false positives on typeshed today, there's a chance we might add code in the future that they'd emit false positives on, or there's a chance that Ruff might stabilise new SIM rules that have false positives on typeshed code.

If lint rules end up irritating us (or confusing contributors) more than they actually help us write better code, then it defeats the purpose of a linter, in my opinion :-)

[Avasam]

Updated to be more specific about rules selection
I listed the rules I didn't select in the PR's description

[Avasam]

Could be

Suggested change

	"SIM201", # Use `{left} != {right}` instead of not `{left} == {right}`
	"SIM202", # Use `{left} == {right}` instead of not `{left} != {right}`
	"SIM208", # Use `{expr}` instead of `not (not {expr})`
	"SIM210", # Remove unnecessary `True if ... else False`
	"SIM211", # Use `not ...` instead of `False if ... else True`
	"SIM212", # Use `{expr_else} if {expr_else} else {expr_body}` instead of `{expr_body} if not {expr_else} else {expr_else}`
	"SIM220", # Use `False` instead of `{name} and not {name}`
	"SIM221", # Use `True` instead of `{name} or not {name}`
	"SIM222", # Use `{expr}` instead of `{replaced}`
	"SIM223", # Use `{expr}` instead of `{replaced}`
	"SIM2", # flake8-simplify conditional ordering rules

but as asked I explicitly selected them individually

[github-actions]

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉