[syntax-errors] except* before Python 3.11#16446
Merged
Conversation
CodSpeed Performance ReportMerging #16446 will not alter performanceComparing Summary
|
Summary -- One of the simpler ones, just detect the use of `except*` before 3.11. Test Plan -- New inline tests. I'm thinking it may be okay to leave out linter CLI tests for these since we have a few tests for the plumbing already, but I'm happy to add those again if that's preferable.
ntBre
force-pushed
the
brent/syn-except-star
branch
from
2f7acbd to
c2600ef
Compare
`flake8_bugbear/B904.py` was failing and just needed 3.11 but this could be a recurring issue as we add more syntax errors, so just bump to latest
Contributor
|
MichaReiser
approved these changes
Mar 2, 2025
Member
MichaReiser
left a comment
MichaReiser
left a comment
There was a problem hiding this comment.
I agree, we don't need more CLI test here. The inline tests are sufficient and easier to maintain.
dhruvmanila
reviewed
Mar 3, 2025
Comment on lines
+67
to
+72
| | | ||
| 1 | # parse_options: {"target-version": "3.10"} | ||
| 2 | / try: ... | ||
| 3 | | except* ValueError: ... | ||
| | |_______________________^ Syntax Error: Cannot use `except*` on Python 3.10 (syntax was added in Python 3.11) | ||
| | |
Member
There was a problem hiding this comment.
I think I'd prefer to just highlight the except* part instead of the entire try statement. Pyright only highlights the * token..
Contributor
Author
There was a problem hiding this comment.
Ah okay. I did try that at one point, but I wasn't sure how to handle multiple except* lines after the first. It looks like pyright emits a separate error for each one, which makes sense.
ntBre
added a commit
that referenced
this pull request
Mar 3, 2025
Summary -- This is a follow-up to #16446 to fix the diagnostic range. Storing the range in the `ExceptClauseKind::Star` variant feels slightly awkward, but we don't store the star itself anywhere on the `ExceptHandler`. And we can't just take `ExceptHandler.start() + "except".text_len()` because this code appears to be valid: ```python try: ... except * Error: ... ``` Test Plan -- Existing tests.
This was referenced Mar 3, 2025
ntBre
added a commit
that referenced
this pull request
Mar 4, 2025
Summary -- This is a follow-up to #16446 to fix the diagnostic range to point to the `*` like `pyright` does (#16446 (comment)). Storing the range in the `ExceptClauseKind::Star` variant feels slightly awkward, but we don't store the star itself anywhere on the `ExceptHandler`. And we can't just take `ExceptHandler.start() + "except".text_len()` because this code appears to be valid: ```python try: ... except * Error: ... ``` Test Plan -- Existing tests.
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
One of the simpler ones, just detect the use of
except*before 3.11.Test Plan
New inline tests.
I'm thinking it may be okay to leave out linter CLI tests for these since we have a few tests for the plumbing already, but I'm happy to add those again if that's preferable.