[syntax-errors] Unparenthesized assignment expressions in sets and indexes #16404

ntBre · 2025-02-26T19:20:27Z

Summary

This PR detects unparenthesized assignment expressions used in set literals and comprehensions and in sequence indexes. The link to the release notes in #6591 just has this entry:

Assignment expressions can now be used unparenthesized within set literals and set comprehensions, as well as in sequence indexes (but not slices).

with no other information, so hopefully the test cases I came up with cover all of the changes. I also tested these out in the Python REPL and they actually worked in Python 3.9 too. I'm guessing this may be another case that was "formally made part of the language spec in Python 3.10, but usable -- and commonly used -- in Python >=3.9" as @AlexWaygood added to the body of #6591 for context managers. So we may want to change the version cutoff, but I've gone along with the release notes for now.

Test Plan

New inline parser tests and linter CLI tests.

github-actions · 2025-02-26T19:30:13Z

`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.

This PR is the first in a series derived from #16308, each of which add support for detecting one version-related syntax error from #6591. This one should be the largest because it also includes a couple of additional changes: 1. the `syntax_errors!` macro, which makes adding more variants a bit easier 2. the `Parser::add_unsupported_syntax_error` method Otherwise I think the general structure will be the same for each syntax error: * Detecting the error in the parser * Inline parser tests for the new error * New ruff CLI tests for the new error Because of the second point here, this PR is currently stacked on #16357. As noted above, there are new inline parser tests, as well as new ruff CLI tests. Once #16379 is resolved, there should also be new mdtests for red-knot, but this PR does not currently include those.

crates/ruff/tests/lint.rs

MichaReiser

I'd prefer to wait for @AlexWaygood to confirm the exact details of this change in CPython (and when it was chagned). We otherwise risk flagging code that works perfectly fine on Python 3.9

crates/ruff/tests/lint.rs

MichaReiser · 2025-03-04T08:38:38Z

crates/ruff_python_parser/src/error.rs

@@ -457,6 +458,7 @@ impl Display for UnsupportedSyntaxError {
            UnsupportedSyntaxErrorKind::Match => "`match` statement",
            UnsupportedSyntaxErrorKind::Walrus => "named assignment expression (`:=`)",
            UnsupportedSyntaxErrorKind::ExceptStar => "`except*`",
+            UnsupportedSyntaxErrorKind::UnparWalrus => "unparenthesized assignment expression",


We use NamedExpr for Walrus expressions internally.

Suggested change

UnsupportedSyntaxErrorKind::UnparWalrus => "unparenthesized assignment expression",

UnsupportedSyntaxErrorKind::UnparenthesizedNamedExprInSubscript => "unparenthesized assignment expression",

Or should this allow all assignment expressions, e.g. even a[x=10]?

Unless you meant something else and this is a typo, that's an assignment statement which isn't allowed here.

I think we should just rename it to UnparenthesizedNamedExpr as it's relevant in other context as well other than subscript (set expression, set comprehension).

Yeah sorry about the names, that seems like a running theme in these reviews. I was trying to make them shorter, but it does seem better just to use explanatory names. I will also add documentation on each of the variants, I should have been doing that as I went too.

MichaReiser · 2025-03-04T08:44:58Z

crates/ruff_python_parser/src/parser/expression.rs

+
+        // test_err walrus_slice
+        // # even after 3.10, an unparenthesized walrus is not allowed in a slice
+        // lst[x:=1:-1]


Can we also add a test for when the named expression isn't at the lower position

MichaReiser · 2025-03-04T08:49:20Z

crates/ruff_python_parser/src/parser/expression.rs

            if self.at_ts(NEWLINE_EOF_SET.union([TokenKind::Rsqb, TokenKind::Comma].into())) {
+                // check this here because if we don't return, we will emit a ParseError instead


I don't understand this comment. Or rather, I think I find it more confusing than helpful. Maybe we could change the comment. What I understand is that the parsed expression is a subscript if we reach 878?

I think it might be related to the fact that parse_slice is used for both sequence indexes (data[x := 1]) and slices (data[x := 1: 2]) but the unsupported syntax kind is only to be raised for the former. We have the same check for the latter case down below on 893 which raises a syntax error.

Yes, @dhruvmanila is right, that's what I meant. I initially checked it right after let lower = ... and then had to move it into this if to avoid a duplicate with the ParseError, so I thought it was worth noting, but I see how it could be confusing. I think I'll just delete this comment because it's pretty clear in hindsight what's going on.

Or we could change it to saying that we have a sequence index here and add a comment to the other branch saying that it's a slice.

MichaReiser · 2025-03-04T08:52:20Z

...ff_python_parser/tests/snapshots/invalid_syntax@unparenthesized_walrus_set_comp_py39.py.snap

+  |
+1 | # parse_options: {"target-version": "3.9"}
+2 | {last := x for x in range(3)}
+  |  ^^^^^^^^^ Syntax Error: Cannot use unparenthesized assignment expression on Python 3.9 (syntax was added in Python 3.10)


I think the message needs to be more explicit and include sequence index or similar because it otherwise suggests that if x:= 10: wouldn't be allowed

crates/ruff_python_parser/src/parser/expression.rs

crates/ruff/tests/lint.rs

dhruvmanila · 2025-03-04T09:48:34Z

crates/ruff_python_parser/src/error.rs

@@ -457,6 +458,7 @@ impl Display for UnsupportedSyntaxError {
            UnsupportedSyntaxErrorKind::Match => "`match` statement",
            UnsupportedSyntaxErrorKind::Walrus => "named assignment expression (`:=`)",
            UnsupportedSyntaxErrorKind::ExceptStar => "`except*`",
+            UnsupportedSyntaxErrorKind::UnparWalrus => "unparenthesized assignment expression",


Unless you meant something else and this is a typo, that's an assignment statement which isn't allowed here.

dhruvmanila · 2025-03-04T09:49:50Z

crates/ruff_python_parser/src/error.rs

@@ -457,6 +458,7 @@ impl Display for UnsupportedSyntaxError {
            UnsupportedSyntaxErrorKind::Match => "`match` statement",
            UnsupportedSyntaxErrorKind::Walrus => "named assignment expression (`:=`)",
            UnsupportedSyntaxErrorKind::ExceptStar => "`except*`",
+            UnsupportedSyntaxErrorKind::UnparWalrus => "unparenthesized assignment expression",


I think we should just rename it to UnparenthesizedNamedExpr as it's relevant in other context as well other than subscript (set expression, set comprehension).

dhruvmanila · 2025-03-04T09:53:25Z

crates/ruff_python_parser/src/parser/expression.rs

+        // test_ok parenthesized_walrus_index_py39
+        // # parse_options: {"target-version": "3.9"}
+        // lst[(x:=1)]
+
+        // test_ok unparenthesized_walrus_index_py310
+        // # parse_options: {"target-version": "3.10"}
+        // lst[x:=1]
+
+        // test_err unparenthesized_walrus_index_py39
+        // # parse_options: {"target-version": "3.9"}
+        // lst[x:=1]
+
+        // test_err walrus_slice
+        // # even after 3.10, an unparenthesized walrus is not allowed in a slice
+        // lst[x:=1:-1]


I don't think there's anything to change here but just wanted to point out that we test some of these in the filesystem based tests:

https://github.com/astral-sh/ruff/blob/cb362f4b0a9116867e60e3f08d185ea73c40e452/crates/ruff_python_parser/resources/valid/expressions/subscript.py

https://github.com/astral-sh/ruff/tree/cb362f4b0a9116867e60e3f08d185ea73c40e452/crates/ruff_python_parser/resources/invalid/expressions/subscript

These are fine here as they're using different parser options.

crates/ruff_python_parser/src/parser/expression.rs

dhruvmanila · 2025-03-04T10:13:39Z

crates/ruff_python_parser/src/parser/expression.rs

+        // indexes", which I take to mean subscripts. See
+        // <https://docs.python.org/3/whatsnew/3.10.html#other-language-changes>.
+
+        // test_ok parenthesized_walrus_index_py39


Can we use "named_expr" instead of "walrus"? I think it'll be useful to maintain consistency in the terminology used internally. What do you think?

I think that totally makes sense. I left the existing Walrus variant alone for now but cleaned up all of the walruses in this PR. I'll do another cleanup pass for the already-merged variants separately.

dhruvmanila · 2025-03-04T10:17:07Z

crates/ruff_python_parser/src/parser/expression.rs

            if self.at_ts(NEWLINE_EOF_SET.union([TokenKind::Rsqb, TokenKind::Comma].into())) {
+                // check this here because if we don't return, we will emit a ParseError instead


I think it might be related to the fact that parse_slice is used for both sequence indexes (data[x := 1]) and slices (data[x := 1: 2]) but the unsupported syntax kind is only to be raised for the former. We have the same check for the latter case down below on 893 which raises a syntax error.

dhruvmanila · 2025-03-04T10:21:43Z

I think it's python/cpython@87c87b5 that added it and it seems that the change was added in 3.9.1 so it doesn't exists in 3.9.0 (looking at the tags that commit is included in) which is why I guess it's not in the 3.9 release notes?

So, it was fixed in python/cpython#23332 and backported to 3.9 in python/cpython#23333.

dhruvmanila

Thank you. We just need to make sure about the version cutoff but otherwise this looks good minor some nit comments.

AlexWaygood · 2025-03-04T10:30:11Z

I'd prefer to wait for @AlexWaygood to confirm the exact details of this change in CPython (and when it was chagned). We otherwise risk flagging code that works perfectly fine on Python 3.9

I'm on PTO this week and don't have my laptop on me. But I hear that we have other CPython core devs on the team who might be able to help with this kind of question ;)

In general, I think we should definitely follow CPython's implementation rather than CPython's documentation when it comes to the Python version that things like this became legal syntax. That seems like it should avoid false positives. In cases where the syntax was added unofficially for one or two versions prior to it being added to the language spec, it's very possible that people started using the new syntax accidentally even if it wasn't "officially" supported by the language

MichaReiser · 2025-03-04T10:31:38Z

I'm on PTO this week and don't have my laptop on me. But I hear that we have other core devs on the team who might be able to help with this kind of question ;)

I think this PR can wait till you're back ;) Enjoy your PTO

This also required changing parse_set_expression to take a ParsedExpr instead of an Expr to call is_unparenthesized_named_expr

ntBre · 2025-03-04T16:36:05Z

Thanks for the reviews! I think I have handled all of the comments, but I'm definitely happy to wait for Alex to come back to discuss the version. 3.9 sounds like a safer way to go than 3.10, but there's no rush.

ntBre · 2025-03-14T17:37:17Z

I handled the merge conflicts and changed the version cutoff to 3.9, so I think this should be good to go.

* main: [red-knot] Use `try_call_dunder` for augmented assignment (#16717) [red-knot] Document current state of attribute assignment diagnostics (#16746) [red-knot] Case sensitive module resolver (#16521) [red-knot] Very minor simplification of the render tests (#16759) [syntax-errors] Unparenthesized assignment expressions in sets and indexes (#16404) ruff_db: add a new diagnostic renderer ruff_db: add `context` configuration red_knot: plumb through `DiagnosticFormat` to the CLI ruff_db: add concise diagnostic mode [syntax-errors] Star annotations before Python 3.11 (#16545) [syntax-errors] Star expression in index before Python 3.11 (#16544) Ruff 0.11.0 (#16723) [red-knot] Preliminary tests for typing.Final (#15917) [red-knot] fix: improve type inference for binary ops on tuples (#16725) [red-knot] Assignments to attributes (#16705) [`pygrep-hooks`]: Detect file-level suppressions comments without rul… (#16720) Fallback to requires-python in certain cases when target-version is not found (#16721)

ntBre force-pushed the brent/syntax-decorators-39 branch from 5801f20 to c833f0c Compare February 28, 2025 20:41

ntBre force-pushed the brent/syntax-assign-sequence-310 branch 2 times, most recently from 4fc3761 to eb63c6e Compare February 28, 2025 21:45

ntBre force-pushed the brent/syntax-decorators-39 branch from c833f0c to e57b11e Compare February 28, 2025 22:19

ntBre force-pushed the brent/syntax-assign-sequence-310 branch from eb63c6e to 47da321 Compare February 28, 2025 22:38

ntBre changed the base branch from brent/syntax-decorators-39 to main February 28, 2025 22:38

ntBre added the preview label Feb 28, 2025

ntBre changed the title ~~[syntax-errors] Detect unparenthesized assignment expressions in sets and indexes~~ [syntax-errors] Unparenthesized assignment expressions in sets and indexes Feb 28, 2025

ntBre added the parser label Mar 3, 2025

ntBre added 6 commits March 3, 2025 13:31

update kebab case

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

3c7bac8

update snaps

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

GPG key ID: B5690EEEBB952194

Verified
Learn about vigilant mode

c99efb6

pass all the inline tests

cebcf90

improve diagnostic ranges

bb37c60

add linter test and fix duplicate syntax error

Loading
Loading status checks…

cb362f4

ntBre force-pushed the brent/syntax-assign-sequence-310 branch from 47da321 to cb362f4 Compare March 3, 2025 18:36

ntBre commented Mar 3, 2025

View reviewed changes

crates/ruff/tests/lint.rs Outdated Show resolved Hide resolved

ntBre marked this pull request as ready for review March 3, 2025 18:45

ntBre requested review from MichaReiser and dhruvmanila as code owners March 3, 2025 18:45

MichaReiser approved these changes Mar 4, 2025

View reviewed changes

dhruvmanila reviewed Mar 4, 2025

View reviewed changes

dhruvmanila approved these changes Mar 4, 2025

View reviewed changes

ntBre added 3 commits March 4, 2025 08:56

move unnecessary linter test to inline parser test

be5f3ff

rename and document error variant

b1622ac

add tests for other slice positions

6b07bec

ntBre added 8 commits March 4, 2025 11:25

delete confusing comment

7501737

add UnparenthesizedNamedExprKind and customize messages

a625061

else if to clarify mutual exclusion

51bd135

move first_element check into parse_set_expression

aad54b5

This also required changing parse_set_expression to take a ParsedExpr instead of an Expr to call is_unparenthesized_named_expr

update slice/subscript comment and move tests closer to their use

5b4acae

clean up extra comments

1878b4b

walrus -> named expression

7a6dedd

move tests into else if

Loading
Loading status checks…

eec7060

This was referenced Mar 4, 2025

[syntax-errors] Type parameter lists before Python 3.12 #16479

Merged

Emit diagnostics for new syntax as per the target Python version #6591

Closed

ntBre added 3 commits March 14, 2025 13:24

Merge branch 'main' into brent/syntax-assign-sequence-310

5a824c0

fix missing derives

2ecc5b3

change version cutoff from 3.10 to 3.9

Loading
Loading status checks…

726fa2b

fix unreferenced snaps

Loading
Loading status checks…

5281872

ntBre merged commit 3a32e56 into main Mar 14, 2025
21 checks passed

ntBre deleted the brent/syntax-assign-sequence-310 branch March 14, 2025 19:06

BrewTestBot mentioned this pull request Mar 20, 2025

ruff 0.11.1 Homebrew/homebrew-core#212356

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[syntax-errors] Unparenthesized assignment expressions in sets and indexes #16404

[syntax-errors] Unparenthesized assignment expressions in sets and indexes #16404

ntBre commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

MichaReiser left a comment •

edited

Loading

MichaReiser Mar 4, 2025

MichaReiser Mar 4, 2025

dhruvmanila Mar 4, 2025

dhruvmanila Mar 4, 2025

ntBre Mar 4, 2025

MichaReiser Mar 4, 2025

ntBre Mar 4, 2025

MichaReiser Mar 4, 2025

dhruvmanila Mar 4, 2025

ntBre Mar 4, 2025

MichaReiser Mar 4, 2025

MichaReiser Mar 4, 2025

dhruvmanila Mar 4, 2025

dhruvmanila Mar 4, 2025

dhruvmanila Mar 4, 2025

dhruvmanila Mar 4, 2025

ntBre Mar 4, 2025

dhruvmanila Mar 4, 2025

dhruvmanila commented Mar 4, 2025 •

edited

Loading

dhruvmanila left a comment

AlexWaygood commented Mar 4, 2025 •

edited

Loading

MichaReiser commented Mar 4, 2025

ntBre commented Mar 4, 2025

ntBre commented Mar 14, 2025

	UnsupportedSyntaxErrorKind::UnparWalrus => "unparenthesized assignment expression",
	UnsupportedSyntaxErrorKind::UnparenthesizedNamedExprInSubscript => "unparenthesized assignment expression",

		if self.at_ts(NEWLINE_EOF_SET.union([TokenKind::Rsqb, TokenKind::Comma].into())) {
		// check this here because if we don't return, we will emit a ParseError instead

[syntax-errors] Unparenthesized assignment expressions in sets and indexes #16404

[syntax-errors] Unparenthesized assignment expressions in sets and indexes #16404

Conversation

ntBre commented Feb 26, 2025 • edited Loading

Summary

Test Plan

github-actions bot commented Feb 26, 2025 • edited Loading

ruff-ecosystem results

Linter (stable)

Linter (preview)

Formatter (stable)

Formatter (preview)

MichaReiser left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dhruvmanila commented Mar 4, 2025 • edited Loading

dhruvmanila left a comment

Choose a reason for hiding this comment

AlexWaygood commented Mar 4, 2025 • edited Loading

MichaReiser commented Mar 4, 2025

ntBre commented Mar 4, 2025

ntBre commented Mar 14, 2025

ntBre commented Feb 26, 2025 •

edited

Loading

github-actions bot commented Feb 26, 2025 •

edited

Loading

`ruff-ecosystem` results

MichaReiser left a comment •

edited

Loading

dhruvmanila commented Mar 4, 2025 •

edited

Loading

AlexWaygood commented Mar 4, 2025 •

edited

Loading