Skip to content

[syntax-errors] except* before Python 3.11 - #16446

Merged
ntBre merged 5 commits into
mainfrom
brent/syn-except-star
Mar 2, 2025
Merged

ntBre merged 5 commits into
mainfrom
brent/syn-except-star

Conversation

@ntBre

@ntBre ntBre commented Feb 28, 2025

Copy link
Copy Markdown
Contributor

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
@codspeed

codspeed Bot commented Feb 28, 2025 •

Copy link
Copy Markdown

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
@github-actions

github-actions Bot commented Feb 28, 2025 •

Copy link
Copy Markdown
Contributor

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.

@MichaReiser MichaReiser left a comment

Copy link
Copy Markdown
Member

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.

Comment thread crates/ruff_python_parser/src/parser/statement.rs Outdated
@ntBre
ntBre enabled auto-merge (squash) March 2, 2025 18:17
@ntBre
ntBre merged commit e924ecb into main Mar 2, 2025
@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
Copy Markdown
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
Copy Markdown
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.
KotlinIsland pushed a commit to KotlinIsland/basedpython that referenced this pull request May 1, 2026
Summary
--
This is a follow-up to #16446 to fix the diagnostic range to point to
the `*` like `pyright` does
(astral-sh/ruff#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