Report a specific diagnostic for walrus in a comprehension iterable expression - #11606
Report a specific diagnostic for walrus in a comprehension iterable expression#11606Henry Su (hsusul) wants to merge 1 commit into
Conversation
A walrus operator used anywhere within a comprehension's iterable expression is a syntax error (PEP 572), and it cannot be fixed by adding parentheses. Pyright detected this but reused the generic "is not allowed in this context without surrounding parentheses" message, which is misleading here. Emit a dedicated message for this case while preserving the existing message for a bare walrus used as a comprehension "if" condition, where parentheses do resolve the error. Addresses microsoft#11514.
|
🔒 Automated review in progress — Stella Huang (@StellaHuang95) is auto-reviewing this PR. |
Stella Huang (StellaHuang95)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Stella Huang (StellaHuang95)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
|
🔒 Automated review in progress — Rich Chiodo (@rchiodo) is auto-reviewing this PR. |
Rich Chiodo (rchiodo)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
Heejae Chang (heejaechang)
left a comment
There was a problem hiding this comment.
Approved via Review Center.
| # This should generate an error because an assignment expression is not | ||
| # allowed within a comprehension's iterable expression, even when it is | ||
| # surrounded by parentheses. | ||
| [a for a in (b := x)] |
There was a problem hiding this comment.
Info · Optional note
Consider adding the issue's nested-comprehension reproduction to this exact-message test. Existing tests cover nested detection, but not the new message for that precise shape.
[verified]
|
Verification: The relevant tests could not be fully run in the isolated environment; this review is not fully verified. |
Summary
An assignment expression (
:=) used anywhere within the iterable expression of a comprehension'sforclause is a syntax error under PEP 572, and — unlike a walrus used bare as a comprehensionifcondition — it cannot be made legal by adding parentheses. Pyright already reports this as an error, but it reused the generic message:That message is misleading here: the offending walrus in the reproduction is already parenthesized, and no amount of parenthesization will fix it. This addresses #11514.
Minimal reproduction
CPython:
Current behavior
Pyright reports
Operator ":=" is not allowed in this context without surrounding parentheses, even though the walrus is parenthesized.Corrected behavior
Pyright reports
Operator ":=" is not allowed within a comprehension iterable expression.Root cause
In
parser.ts,_parseAssignmentExpressionemittedwalrusNotAllowedfor two distinct conditions that were OR'd together:These two flags actually represent different restrictions:
!this._assignmentExpressionsAllowedis set only by_disallowAssignmentExpression, whose only caller wraps parsing of a comprehensionfor ... in <iterable>sequence expression. This is the "walrus in a comprehension iterable" restriction, which parentheses do not resolve.disallowAssignmentExpressioncorresponds to a bare walrus that requires surrounding parentheses (e.g. an unparenthesized walrus used as a comprehensionifcondition, or a bare walrus where atestexpression is expected). Here parentheses do resolve the error, so the existing message is correct.Implementation
walrusNotAllowedInComprehension, while the parentheses case keepswalrusNotAllowed. The iterable case is checked first so it wins when both apply (parentheses would not help there either).walrusNotAllowedInComprehensionkey tolocalize.tsandpackage.nls.en-us.json.Error detection (which cases are flagged, and the diagnostic range) is unchanged — only the message text differs for the comprehension-iterable case.
Tests
assignmentExprMessage1.pyand testAssignmentExprMessage1assert the exact message for both branches: the comprehension-iterable case gets the new message, and a bare walrus in a comprehensionifkeeps the generic message.AssignmentExpr4test (which already exercises iterable-walrus cases) continues to pass with its error count unchanged, confirming detection is unaffected.Validation
Run in
packages/pyright-internal:npx tsc --noEmit— passes.npx prettier@2.8.8 -con all changed TS/JSON files — "All matched files use Prettier code style!".git diff --check— clean.npx jest parser tokenizer typeEvaluator1— all pass (includes the newAssignmentExprMessage1and the unchangedAssignmentExpr4).npx jest— all suites pass except LSP/type-server integration suites, which fail to run in this environment because they require the full monorepo bootstrap and the webpack test-server bundle (Cannot find module 'fs-extra'/Server bundle does not exist); these are unrelated to this change.Compatibility
No API changes. The only behavioral difference is a more accurate diagnostic message for walrus operators inside a comprehension iterable expression; the set of reported errors and their source ranges are unchanged.