[red-knot] Move standalone expr inference to for non-name target#14788
[red-knot] Move standalone expr inference to for non-name target#14788dhruvmanila merged 1 commit intomainfrom
for non-name target#14788Conversation
| if let ast::Expr::Name(name) = &**target { | ||
| self.infer_definition(name); | ||
| } else { | ||
| self.infer_standalone_expression(iter); |
There was a problem hiding this comment.
Why is it okay to only infer the iterator when the target isn't a name? Is it because it is otherwise handled in infer_for_statement_definition or is it something else?
There was a problem hiding this comment.
Yes, that's correct. It's handled while inferring the definition types. This pattern is also followed in other definitions like assignment, with items, etc.
There was a problem hiding this comment.
The main reason for this requirement is the self.extend call which duplicated the diagnostics.
|
| # error: [invalid-syntax] | ||
| # error: [invalid-syntax] | ||
| # error: [invalid-syntax] |
There was a problem hiding this comment.
Why are there still three invalid-syntax errors? Can we add messages clarifying what the three errors are? I would expect two (one for while used as identifier, the other for pass used as identifier.)
There was a problem hiding this comment.
In brief, our parser cannot recover from certain syntax errors completely. For example, if there's a keyword (like for) in the middle of another statement (like function definition), then it's more likely that the rest of the tokens are going to be part of the for statement and not the function definition. But, it's not necessary that the remaining tokens are valid in the context of a for statement. This is what's happening here. There's a function definition before this part of the code that the parser doesn't recover well from:
def True(for):
pass
for while in pass:
passTry commenting out the function definition and you'll get the expected number of errors. I could add this as a general comment to this file.
Playground: https://play.ruff.rs/a36e1a68-abd9-404d-bb79-146855c6d521
Ref: #14788 (comment) This PR: * Separates code snippets as individual tests for the invalid syntax cases * Adds a general comment explaining why the parser could emit more syntax errors than expected
Summary
Ref: #14754 (comment)
Test Plan
Remove the TODO comment and update the mdtest.