-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[flake8-simplify] Make example error out-of-the-box (SIM116)
#19111
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
Conversation
flake8-simplify] Make example error out-of-the-box (SIM116)flake8-simplify] Make example error out-of-the-box (SIM116)
|
| /// elif x == 3: | ||
| /// return "Good morning" | ||
| /// else: | ||
| /// return "Goodnight" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I got a couple more hits adding the "Use instead" sections to the search, SIM110 and PLW1501
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
## 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.
ntBre
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
…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) ...
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