Skip to content

Expand semantic syntax errors for invalid walruses#25415

Merged
charliermarsh merged 10 commits into
mainfrom
charlie/walrus-syntax-errors
May 27, 2026
Merged

Expand semantic syntax errors for invalid walruses#25415
charliermarsh merged 10 commits into
mainfrom
charlie/walrus-syntax-errors

Conversation

@charliermarsh

@charliermarsh charliermarsh commented May 27, 2026

Copy link
Copy Markdown
Member

Summary

Python rejects assignment expressions in several comprehension contexts, but we previously only reported the case in which a walrus rebound a comprehension variable.

As a result, we accepted invalid walruses in the iterable and class-body portions clause of a comprehension, and missed some rebinding cases in filter clauses and nested comprehensions. It turns out the rules are really complicated!

For example, we now report the two newly modeled syntax errors:

# error: [invalid-syntax] "assignment expression cannot be used in a comprehension iterable expression"
[x for x in (values := [1])]

class C:
    # error: [invalid-syntax] "assignment expression within a comprehension cannot be used in a class body"
    [(value := item) for item in [1]]

We also catch previously missed instances of the existing rebinding error:

# error: [invalid-syntax] "assignment expression cannot rebind comprehension variable"
[x for x in [1] if (x := 0)]

In general, I decided not to worry about emitting multiple diagnostics for a single error. These are rare, they're fatal, and it added a lot of complexity (so in some cases we may emit two errors for a walrus that is invalid for two reasons).

@astral-sh-bot

astral-sh-bot Bot commented May 27, 2026

Copy link
Copy Markdown

Typing conformance results

No changes detected ✅

Current numbers
The percentage of diagnostics emitted that were expected errors held steady at 91.94%. The percentage of expected errors that received a diagnostic held steady at 87.09%. The number of fully passing files held steady at 92/134.

@astral-sh-bot

astral-sh-bot Bot commented May 27, 2026

Copy link
Copy Markdown

Memory usage report

Memory usage unchanged ✅

@astral-sh-bot

astral-sh-bot Bot commented May 27, 2026

Copy link
Copy Markdown

ecosystem-analyzer results

No diagnostic changes detected ✅

Full report with detailed diff (timing results)

@astral-sh-bot

astral-sh-bot Bot commented May 27, 2026

Copy link
Copy Markdown

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.

@codspeed-hq

codspeed-hq Bot commented May 27, 2026

Copy link
Copy Markdown

Merging this PR will not alter performance

✅ 125 untouched benchmarks


Comparing charlie/walrus-syntax-errors (09158b7) with main (4a7ca06)

Open in CodSpeed

@charliermarsh

Copy link
Copy Markdown
Member Author

Tagging @ntBre on this one. (Not super urgent.)

@charliermarsh charliermarsh added parser Related to the parser linter Related to the linter labels May 27, 2026
@charliermarsh charliermarsh force-pushed the charlie/walrus-syntax-errors branch from 59f3a4e to d5e10cc Compare May 27, 2026 16:22

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

Looks good to me!

Comment on lines -1876 to -1878
/// Searches for the first named expression (`x := y`) rebinding one of the `iteration_variables` in
/// a comprehension or generator expression.
struct ReboundComprehensionVisitor<'a> {

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.

Should we preserve this doc comment?

Comment on lines -1107 to -1109
// TODO(brent) with multiple diagnostic ranges, we could mark both the named expr (current)
// and the name expr being rebound
for range in rebound_variables {

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.

Should we preserve this TODO somewhere too? Unless it doesn't seem useful to do it anymore.

Comment on lines +1969 to +1971
struct ComprehensionTargetNameVisitor {
names: FxHashSet<ast::name::Name>,
}

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.

I think you could probably reuse the StoredNameFinder from the AST crate here, but the API is a little different.

/// A [`Visitor`] to collect all stored [`Expr::Name`] nodes in an AST.
#[derive(Debug, Default)]
pub struct StoredNameFinder<'a> {
/// A map from identifier to defining expression.
pub names: FxHashMap<&'a str, &'a ast::ExprName>,
}
impl<'a> Visitor<'a> for StoredNameFinder<'a> {
fn visit_expr(&mut self, expr: &'a Expr) {
if let Expr::Name(name) = expr {
if name.ctx.is_store() {
self.names.insert(&name.id, name);
}
}
crate::visitor::walk_expr(self, expr);
}
}

@charliermarsh charliermarsh force-pushed the charlie/walrus-syntax-errors branch from 46d25ae to 209dd25 Compare May 27, 2026 18:43
@charliermarsh charliermarsh force-pushed the charlie/walrus-syntax-errors branch from 209dd25 to 09158b7 Compare May 27, 2026 20:19
@charliermarsh charliermarsh merged commit 574e107 into main May 27, 2026
60 checks passed
@charliermarsh charliermarsh deleted the charlie/walrus-syntax-errors branch May 27, 2026 20:39
anishgirianish pushed a commit to anishgirianish/ruff that referenced this pull request May 28, 2026
## Summary

Python rejects assignment expressions in several comprehension contexts,
but we previously only reported the case in which a walrus rebound a
comprehension variable.

As a result, we accepted invalid walruses in the iterable and class-body
portions clause of a comprehension, and missed some rebinding cases in
filter clauses and nested comprehensions. It turns out the rules are
really complicated!

For example, we now report the two newly modeled syntax errors:

```py
# error: [invalid-syntax] "assignment expression cannot be used in a comprehension iterable expression"
[x for x in (values := [1])]

class C:
    # error: [invalid-syntax] "assignment expression within a comprehension cannot be used in a class body"
    [(value := item) for item in [1]]
```

We also catch previously missed instances of the existing rebinding
error:

```py
# error: [invalid-syntax] "assignment expression cannot rebind comprehension variable"
[x for x in [1] if (x := 0)]
```

In general, I decided not to worry about emitting multiple diagnostics
for a single error. These are rare, they're fatal, and it added a lot of
complexity (so in some cases we may emit two errors for a walrus that is
invalid for two reasons).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

linter Related to the linter parser Related to the parser

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants