Skip to content

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Feb 28, 2025

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 ntBre added the preview Related to preview mode features label Feb 28, 2025
Copy link

codspeed-hq bot commented Feb 28, 2025

CodSpeed Performance Report

Merging #16446 will not alter performance

Comparing brent/syn-except-star (cc2f852) with main (0d615b8)

Summary

✅ 32 untouched benchmarks

Base automatically changed from brent/syntax-walrus-38 to main February 28, 2025 22:13
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 ntBre force-pushed the brent/syn-except-star branch from 2f7acbd to c2600ef Compare February 28, 2025 22:17
`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
Copy link
Contributor

github-actions bot commented Feb 28, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, we don't need more CLI test here. The inline tests are sufficient and easier to maintain.

@ntBre ntBre enabled auto-merge (squash) March 2, 2025 18:17
@ntBre ntBre merged commit e924ecb into main Mar 2, 2025
20 checks passed
@ntBre ntBre deleted the brent/syn-except-star branch March 2, 2025 18:20
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)
|
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I'd prefer to just highlight the except* part instead of the entire try statement. Pyright only highlights the * token..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@dhruvmanila dhruvmanila added the parser Related to the parser label Mar 3, 2025
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.
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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
parser Related to the parser preview Related to preview mode features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants