Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[syntax-errors] Star expression in index before Python 3.11 #16544

Merged
merged 2 commits into from
Mar 14, 2025

Conversation

ntBre
Copy link
Contributor

@ntBre ntBre commented Mar 6, 2025

Summary

This PR detects tuple unpacking expressions in index/subscript expressions before Python 3.11.

Test Plan

New inline tests

Unverified

The email in this signature doesn’t match the committer email.
Summary
--

This PR detects tuple unpacking expressions in index/subscript expressions
before Python 3.11.

Test Plan
--

New inline tests
@ntBre ntBre added parser Related to the parser preview Related to preview mode features labels Mar 6, 2025
Copy link
Contributor

github-actions bot commented Mar 6, 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.

ntBre added a commit that referenced this pull request Mar 6, 2025

Verified

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

This is closely related to (and stacked on)
#16544 and detects star annotations in
function definitions.

I initially called the variant `StarExpressionInAnnotation` to mirror
`StarExpressionInIndex`, but I realized it's not really a "star expression" in
this position and renamed it. `StarAnnotation` seems in line with the PEP.

Test Plan
--

Two new inline tests. It looked like there was pretty good existing coverage of
this syntax, so I just added simple examples to test the version cutoff.
@@ -636,6 +664,9 @@ impl Display for UnsupportedSyntaxError {
UnsupportedSyntaxErrorKind::TypeParamDefault => {
"Cannot set default type for a type parameter"
}
UnsupportedSyntaxErrorKind::StarExpressionInIndex => {
"Cannot use star expression in index"
Copy link
Member

Choose a reason for hiding this comment

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

Nit: Maybe in index operations to align with the terminology used in the PEP

Comment on lines +360 to +361
2 | lst[*index] # simple index
| ^^^^^^ Syntax Error: Cannot use star expression in index on Python 3.10 (syntax was added in Python 3.11)
Copy link
Member

@MichaReiser MichaReiser Mar 7, 2025

Choose a reason for hiding this comment

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

I'm not sure this is correct. I haven't checked if it is a parse or semantic error but Python 3.12 gives me:

>>> [][*list]
<python-input-1>:1: SyntaxWarning: list indices must be integers or slices, not tuple; perhaps you missed a comma?
  [][*list]
Traceback (most recent call last):
  File "<python-input-1>", line 1, in <module>
    [][*list]
    ~~^^^^^^^
TypeError: list indices must be integers or slices, not tuple

Which is also what I read from the grammar change: It only allows star expressions for slices except the first

slices:
    | slice !','
    | ','.(slice | starred_expression)+ [',']

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think the check needs to be moved up in the if branch which is a pure tuple case.

This is a TypeError and is not relevant to the parser:

$ bat _.py                         
         File: _.py
     1 ~ [][*data]

$ uv run --python=3.12 --no-project python -m ast _.py
Module(
   body=[
      Expr(
         value=Subscript(
            value=List(elts=[], ctx=Load()),
            slice=Tuple(
               elts=[
                  Starred(
                     value=Name(id='data', ctx=Load()),
                     ctx=Load())],
               ctx=Load()),
            ctx=Load()))],
   type_ignores=[])

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I may have made this test case a bit misleading by naming the container lst, but as @dhruvmanila points out with the valid AST, this is syntactically valid, and it's not a type error for a container like a numpy array or another container that can be validly indexed by a tuple:

>>> import numpy as np
>>> a = np.array([[1, 2, 3], [4, 5, 6]])
>>> t = [1, 2]
>>> a[*t]
np.int64(6)

It's not a parser or compiler error, it's only a runtime TypeError if the container doesn't support tuple indexing.

I believe the grammar in the reference also supports this, albeit with different terminology from the PEP:

slicing      ::= primary "[" slice_list "]"
slice_list   ::= slice_item ("," slice_item)* [","]
slice_item   ::= expression | proper_slice
proper_slice ::= [lower_bound] ":" [upper_bound] [ ":" [stride] ]
lower_bound  ::= expression
upper_bound  ::= expression
stride       ::= expression

slice_item is either an expression or a proper_slice with multiple parts.

Copy link
Member

Choose a reason for hiding this comment

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

Sorry, I think I mis-understood the change here. I think what Brent has here is correct. The example and I and Micha have pointed out is a runtime error and should throw a syntax error for Python version older than 3.11.

Comment on lines +360 to +361
2 | lst[*index] # simple index
| ^^^^^^ Syntax Error: Cannot use star expression in index 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.

Yeah, I think the check needs to be moved up in the if branch which is a pure tuple case.

This is a TypeError and is not relevant to the parser:

$ bat _.py                         
         File: _.py
     1 ~ [][*data]

$ uv run --python=3.12 --no-project python -m ast _.py
Module(
   body=[
      Expr(
         value=Subscript(
            value=List(elts=[], ctx=Load()),
            slice=Tuple(
               elts=[
                  Starred(
                     value=Name(id='data', ctx=Load()),
                     ctx=Load())],
               ctx=Load()),
            ctx=Load()))],
   type_ignores=[])

Copy link
Member

@dhruvmanila dhruvmanila left a comment

Choose a reason for hiding this comment

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

Looks good!

Unverified

The email in this signature doesn’t match the committer email.
@ntBre ntBre enabled auto-merge (squash) March 14, 2025 14:48
@ntBre ntBre merged commit 4f28519 into main Mar 14, 2025
20 checks passed
@ntBre ntBre deleted the brent/syn-star-index branch March 14, 2025 14:51
ntBre added a commit that referenced this pull request Mar 14, 2025

Verified

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

This is closely related to (and stacked on)
#16544 and detects star
annotations in function definitions.

I initially called the variant `StarExpressionInAnnotation` to mirror
`StarExpressionInIndex`, but I realized it's not really a "star
expression" in this position and renamed it. `StarAnnotation` seems in
line with the PEP.

Test Plan
--

Two new inline tests. It looked like there was pretty good existing
coverage of this syntax, so I just added simple examples to test the
version cutoff.
dcreager added a commit that referenced this pull request Mar 14, 2025
* 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)
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.

None yet

3 participants