[flake8-simplify] Make example error out-of-the-box (SIM116)#19111
Merged
ntBre merged 2 commits intoastral-sh:mainfrom Jul 7, 2025
Merged
[flake8-simplify] Make example error out-of-the-box (SIM116)#19111ntBre merged 2 commits intoastral-sh:mainfrom
flake8-simplify] Make example error out-of-the-box (SIM116)#19111ntBre merged 2 commits intoastral-sh:mainfrom
Conversation
MeGaGiGaGon
changed the title
flake8-simplify] Make example error out-of-the-box (SIM116)flake8-simplify] Make example error out-of-the-box (SIM116)
Contributor
|
ntBre
reviewed
Jul 3, 2025
| /// elif x == 3: | ||
| /// return "Good morning" | ||
| /// else: | ||
| /// return "Goodnight" |
Contributor
There was a problem hiding this comment.
Should we wrap these in functions? Otherwise the returns should be syntax errors. I was alarmed that we weren't flagging the syntax error, but that semantic error is still associated with the F706 lint rule.
Contributor
Author
There was a problem hiding this comment.
That would probably be a good idea. I ran my tool again looking for that, and I got only this rule and UP028 as having examples that raised one of F701-F706 (Aside from those rule's examples themselves).
Should the tests also be updated? Since they have the same problem as the example. I also did a ruff run on the tests itself, and found a few other rules that run into these in their tests:
crates\ruff_linter\resources\test\fixtures\flake8_bandit\S112.py: F702 `continue` not properly in loop
crates\ruff_linter\resources\test\fixtures\flake8_bugbear\B031.py: F706 `return` statement outside of a function/method
crates\ruff_linter\resources\test\fixtures\flake8_pie\PIE804.py: F704 `yield` statement outside of a function
crates\ruff_linter\resources\test\fixtures\flake8_simplify\SIM115.py: F704 `await` statement outside of a function
crates\ruff_linter\resources\test\fixtures\flake8_simplify\SIM116.py: F706 `return` statement outside of a function/method
crates\ruff_linter\resources\test\fixtures\pycodestyle\E27.py: F704 `yield` statement outside of a function
Contributor
Author
There was a problem hiding this comment.
I got a couple more hits adding the "Use instead" sections to the search, SIM110 and PLW1501
Contributor
There was a problem hiding this comment.
Sure, that seems like a good idea! I think the plan is still to make those rules into true SyntaxErrors eventually, so these snapshots would change then anyway.
dylwil3
pushed a commit
that referenced
this pull request
Jul 4, 2025
## Summary Per @ntBre in #19111, it would be a good idea to make the tests no longer have these syntax errors, so this PR updates the tests and snapshots. `B031` gave me a lot of trouble since the ending test of declaring a function named `groupby` makes it so that inside other functions, it's unclear which `groupby` is referred to since it depends on when the function is called. To fix it I made each function have it's own `from itertools import groupby` so there's no more ambiguity.
UnboundVariable
pushed a commit
to UnboundVariable/ruff
that referenced
this pull request
Jul 7, 2025
…c_tokens * 'main' of https://github.com/astral-sh/ruff: (27 commits) [ty] First cut at semantic token provider (astral-sh#19108) [`flake8-simplify`] Make example error out-of-the-box (`SIM116`) (astral-sh#19111) [`flake8-use-pathlib`] Make example error out-of-the-box (`PTH210`) (astral-sh#19189) [`flake8-use-pathlib`] Add autofixes for `PTH203`, `PTH204`, `PTH205` (astral-sh#18922) [`flake8-type-checking`] Fix syntax error introduced by fix (`TC008`) (astral-sh#19150) [`flake8-pyi`] Make example error out-of-the-box (`PYI007`, `PYI008`) (astral-sh#19103) Update Rust crate indicatif to 0.18.0 (astral-sh#19165) [ty] Add separate CI job for memory usage stats (astral-sh#19134) [ty] Add documentation for server traits (astral-sh#19137) Rename to `SessionSnapshot`, move unwind assertion closer (astral-sh#19177) [`flake8-type-checking`] Make example error out-of-the-box (`TC001`) (astral-sh#19151) [ty] Bare `ClassVar` annotations (astral-sh#15768) [ty] Re-enable multithreaded pydantic benchmark (astral-sh#19176) [ty] Implement equivalence for protocols with method members (astral-sh#18659) [ty] Use RHS inferred type for bare `Final` symbols (astral-sh#19142) [ty] Support declaration-only attributes (astral-sh#19048) [ty] Sync vendored typeshed stubs (astral-sh#19174) Update dependency pyodide to ^0.28.0 (astral-sh#19164) Update NPM Development dependencies (astral-sh#19170) Update taiki-e/install-action action to v2.56.7 (astral-sh#19169) ...
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
2 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
Part of #18972
This PR makes if-else-block-instead-of-dict-lookup (SIM116)'s example error out-of-the-box
Old example
New example
The "Use instead" section was also updated to reflect the new case. I also changed it to use an intermediary variable since I find the
return <long dict>.getvery ugly and hard to read.Test Plan
N/A, no functionality/tests affected