Skip to content

[syntax-errors] Improve error message and range for pre-PEP-614 decorator syntax errors - #16581

Merged
AlexWaygood merged 3 commits into
mainfrom
alex/syntax-error-msgs
Mar 17, 2025
Merged

AlexWaygood merged 3 commits into
mainfrom
alex/syntax-error-msgs

Conversation

@AlexWaygood

Copy link
Copy Markdown
Member

Summary

A small followup to #16386. We now tell the user exactly what it was about their decorator that constituted invalid syntax on Python <3.9, and the range now highlights the specific sub-expression that is invalid rather than highlighting the whole decorator

Test Plan

Inline snapshots are updated, and new ones are added.

@AlexWaygood AlexWaygood added the parser Related to the parser label Mar 9, 2025
@AlexWaygood
AlexWaygood requested a review from ntBre March 9, 2025 22:32
@AlexWaygood AlexWaygood added the preview Related to preview-mode features label Mar 9, 2025
@AlexWaygood
AlexWaygood force-pushed the alex/syntax-error-msgs branch from 08badbb to 63e2de2 Compare March 9, 2025 22:34
@github-actions

github-actions Bot commented Mar 9, 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.

@ntBre ntBre left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is great, thank you!

Comment thread crates/ruff_python_parser/src/error.rs Outdated
Comment thread crates/ruff_python_parser/src/parser/helpers.rs Outdated
@MichaReiser

MichaReiser commented Mar 10, 2025 •

Copy link
Copy Markdown
Member

@ntBre and I discussed this and we deliberately decided against it because:

  • It introduces more complexity and an extra cost for every added syntax
  • It seems low priority considering that < Py39 is end of life

I'm not opposed to this change. I just want to make clear that we deliberately decided against it because we wanted to focus our time on higher priority syntax errors and reduce complexity

Comment on lines +93 to +94
3 | @await bar
| ^^^^^^^^^ Syntax Error: Cannot use `await` expression outside function-call arguments in a decorator on Python 3.8 (syntax was added in Python 3.9)

@dhruvmanila dhruvmanila Mar 10, 2025 •

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 actually find the "outside function-call arguments in a decorator" part a bit confusing because it made me think that this is only allowed in function-call arguments. I'd possibly remove it and keep it simply "Cannot use await expression in a decorator on Python 3.8 ..." but then the message cannot be used in a general way because, in this example, the await expression is only not allowed at the top-level (@(await x)) while @decorator(await x) would be ok.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oh, I see. So you interpreted "outside function-call arguments" as meaning "If I add parentheses, it will be valid syntax"?

Hmm, I'm not sure how to clarify that :/ adding the parentheses doesn't make it a function call -- but I can see how a beginner might find the language confusing. Would you find "outside call expression arguments" any clearer?

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.

Sorry for the late reply, what I thought it could be interpreted is related to the function that's being decorated and not the decorator itself. But, I think what you've is fine as well. We can iterate if anyone finds it confusing.

@AlexWaygood
AlexWaygood force-pushed the alex/syntax-error-msgs branch from b034baf to 4158b95 Compare March 10, 2025 13:39
@AlexWaygood
AlexWaygood merged commit 38bfda9 into main Mar 17, 2025
@AlexWaygood
AlexWaygood deleted the alex/syntax-error-msgs branch March 17, 2025 11:17
dcreager added a commit that referenced this pull request Mar 17, 2025
* main: (25 commits)
  [syntax-errors] Parenthesized context managers before Python 3.9 (#16523)
  [ci]: Disable wheel testing on `ppc64le` (#16793)
  [red-knot] Stabilize `negation_reverses_subtype_order` property test (#16801)
  [red-knot] Emit error if int/float/complex/bytes/boolean literals appear in type expressions outside `typing.Literal[]` (#16765)
  [ci] Use `git diff` instead of `changed-files` GH action (#16796)
  [syntax-errors] Improve error message and range for pre-PEP-614 decorator syntax errors (#16581)
  [`flake8-bandit`] Allow raw strings in `suspicious-mark-safe-usage` (`S308`) #16702 (#16770)
  [`refurb`] Avoid panicking `unwrap` in `verbose-decimal-constructor` (`FURB157`) (#16777)
  [red-knot] Add `--color` CLI option (#16758)
  [internal]: Upgrade salsa (#16794)
  Pin dependencies (#16791)
  [internal]: Update indirect dependencies (#16792)
  [ci]: Fixup codspeed upgrade (#16790)
  Update Rust crate compact_str to 0.9.0 (#16785)
  Update Rust crate clap to v4.5.32 (#16778)
  Update Rust crate codspeed-criterion-compat to v2.9.1 (#16784)
  Update Rust crate quote to v1.0.40 (#16782)
  Update Rust crate ordermap to v0.5.6 (#16781)
  Update cloudflare/wrangler-action action to v3.14.1 (#16783)
  Update Rust crate env_logger to v0.11.7 (#16779)
  ...
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.

4 participants