Emit diagnostics for missing nodes #657
Conversation
Recovery should be handled as unexpected nodes, which are usually produced by `expect`.
Previously, we returned a missing syntax in these cases, which made it harder for recovery to determine whether anything was consumed.
|
@swift-ci Please test |
| message += " after '\(previousToken.text)'" | ||
| } | ||
| if let parent = missingNode.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics { | ||
| if parentTypeName != "source file" { |
There was a problem hiding this comment.
Can this not check the type rather than string?
| @@ -97,7 +97,8 @@ final class DeclarationTests: XCTestCase { | |||
| AssertParse( | |||
| "_ = foo/* */?.description#^DIAG^#", | |||
| diagnostics: [ | |||
| DiagnosticSpec(message: "Expected ':' after '? ...' in ternary expression") | |||
| DiagnosticSpec(message: "Expected ':' after '? ...' in ternary expression"), | |||
| DiagnosticSpec(message: "Expected expression after ':'"), | |||
There was a problem hiding this comment.
I think I mentioned this in some other PR, but multiple diagnostics at the same location seem odd to me. Could we filter all but the first at the same location out?
| @@ -439,7 +443,12 @@ final class DeclarationTests: XCTestCase { | |||
|
|
|||
| func testExtraneousRightBraceRecovery() { | |||
| AssertParse( | |||
| "class ABC { let def = ghi(jkl: mno) } #^DIAG^#}", | |||
There was a problem hiding this comment.
Did this change anything, or were you just making the test case easier to see?
| AssertParse( | ||
| "a ? b :" | ||
| ) |
There was a problem hiding this comment.
Why doesn't this have the "expected expression after ':'" diagnostic?
| DiagnosticSpec(message: "Expected expression after ':' in dictionary"), | ||
| DiagnosticSpec(message: "Expected ']' to end dictionary"), |
There was a problem hiding this comment.
Same as above. FWIW the current diagnostic is a missing dictionary value (expected value in dictionary literal), but I think it's fine not to special case it.
There's obviously quite a few of these cases so I won't mention any others 😅
Depends on #656
Previously, we weren’t emitting any errors for the missing layout nodes (
RawMissingExprSyntaxetc.)