Skip to content

Commit 0ddee83

Browse files
committed
Emit diagnostics for missing nodes
1 parent c5b3859 commit 0ddee83

File tree

7 files changed

+177
-50
lines changed

7 files changed

+177
-50
lines changed

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,40 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
118118
return .skipChildren
119119
}
120120

121+
private func handleMissingSyntax<T: SyntaxProtocol>(_ node: T) -> SyntaxVisitorContinueKind {
122+
if shouldSkip(node) {
123+
return .skipChildren
124+
}
125+
addDiagnostic(node, position: node.endPosition, MissingNodeError(missingNode: Syntax(node)))
126+
return .visitChildren
127+
}
128+
121129
// MARK: - Specialized diagnostic generation
122130

131+
public override func visit(_ node: MissingDeclSyntax) -> SyntaxVisitorContinueKind {
132+
return handleMissingSyntax(node)
133+
}
134+
135+
public override func visit(_ node: MissingExprSyntax) -> SyntaxVisitorContinueKind {
136+
return handleMissingSyntax(node)
137+
}
138+
139+
public override func visit(_ node: MissingPatternSyntax) -> SyntaxVisitorContinueKind {
140+
return handleMissingSyntax(node)
141+
}
142+
143+
public override func visit(_ node: MissingStmtSyntax) -> SyntaxVisitorContinueKind {
144+
return handleMissingSyntax(node)
145+
}
146+
147+
public override func visit(_ node: MissingSyntax) -> SyntaxVisitorContinueKind {
148+
return handleMissingSyntax(node)
149+
}
150+
151+
public override func visit(_ node: MissingTypeSyntax) -> SyntaxVisitorContinueKind {
152+
return handleMissingSyntax(node)
153+
}
154+
123155
public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
124156
if shouldSkip(node) {
125157
return .skipChildren
@@ -145,7 +177,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
145177
Syntax(node.unexpectedBetweenWhereClauseAndBody),
146178
Syntax(unexpectedCondition)
147179
] as [Syntax?]).compactMap({ $0 }),
148-
handledNodes: [node.inKeyword.id, unexpectedCondition.id]
180+
handledNodes: [node.inKeyword.id, node.sequenceExpr.id, unexpectedCondition.id]
149181
)
150182
}
151183
return .visitChildren

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

Lines changed: 40 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,19 @@ extension SyntaxProtocol {
2222
/// correspondence in the source code that's meeingful to the user by default
2323
/// use the name of the parent node that encloses it. Pass `false` to `inherit`
2424
/// to prevent this name inheritance.
25-
func nodeTypeNameForDiagnostics(inherit: Bool = true) -> String? {
25+
/// If `allowSourceFile` is `false`, `nil` will be returned if the inherited
26+
/// node type name is "source file".
27+
func nodeTypeNameForDiagnostics(inherit: Bool = true, allowSourceFile: Bool = true) -> String? {
2628
if let name = Syntax(self).as(SyntaxEnum.self).nameForDiagnostics {
27-
return name
29+
if Syntax(self).is(SourceFileSyntax.self) && !allowSourceFile {
30+
return nil
31+
} else {
32+
return name
33+
}
2834
}
2935
if inherit {
3036
if let parent = self.parent {
31-
return parent.nodeTypeNameForDiagnostics(inherit: inherit)
37+
return parent.nodeTypeNameForDiagnostics(inherit: inherit, allowSourceFile: allowSourceFile)
3238
}
3339
}
3440
return nil
@@ -135,6 +141,37 @@ public struct InvalidIdentifierError: ParserError {
135141
}
136142
}
137143

144+
public struct MissingNodeError: ParserError {
145+
public let missingNode: Syntax
146+
147+
public var message: String {
148+
var message: String
149+
var hasNamedParent = false
150+
if let parent = missingNode.parent,
151+
let childName = parent.childNameForDiagnostics(missingNode.index) {
152+
message = "Expected \(childName)"
153+
if let parent = missingNode.parent,
154+
let parentTypeName = parent.nodeTypeNameForDiagnostics(inherit: false) {
155+
message += " of \(parentTypeName)"
156+
hasNamedParent = true
157+
}
158+
} else {
159+
message = "Expected \(missingNode.nodeTypeNameForDiagnostics() ?? "syntax")"
160+
if let lastChild = missingNode.lastToken(viewMode: .fixedUp), lastChild.presence == .present {
161+
message += " after '\(lastChild.text)'"
162+
} else if let previousToken = missingNode.previousToken(viewMode: .fixedUp), previousToken.presence == .present {
163+
message += " after '\(previousToken.text)'"
164+
}
165+
}
166+
if !hasNamedParent {
167+
if let parent = missingNode.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics(allowSourceFile: false) {
168+
message += " in \(parentTypeName)"
169+
}
170+
}
171+
return message
172+
}
173+
}
174+
138175
public struct MissingTokenError: ParserError {
139176
public let missingToken: TokenSyntax
140177

Tests/SwiftParserTest/Attributes.swift

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ final class AttributeTests: XCTestCase {
1111
}
1212
""",
1313
diagnostics: [
14+
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected declaration"),
1415
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected 'for' in attribute argument"),
1516
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
1617
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ')' to end attribute"),
@@ -27,7 +28,9 @@ final class AttributeTests: XCTestCase {
2728
""",
2829
diagnostics: [
2930
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in '@differentiable' argument"),
31+
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected parameters of '@differentiable' argument"),
3032
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '=' in same type requirement"),
33+
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected right-hand type of same type requirement"),
3134
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ')' to end attribute"),
3235
]
3336
)
@@ -36,21 +39,23 @@ final class AttributeTests: XCTestCase {
3639
func testMissingClosingParenToAttribute() {
3740
AssertParse(
3841
"""
39-
@_specialize(e#^DIAG_1^#
42+
@_specialize(e#^DIAG^#
4043
""",
4144
diagnostics: [
42-
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
43-
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ')' to end attribute"),
45+
DiagnosticSpec(message: "Expected declaration"),
46+
DiagnosticSpec(message: "Expected ':' in attribute argument"),
47+
DiagnosticSpec(message: "Expected ')' to end attribute"),
4448
]
4549
)
4650
}
4751

4852
func testMultipleInvalidSpecializeParams() {
4953
AssertParse(
5054
"""
51-
@_specialize(e#^DIAG_1^#, exported#^DIAG_2^#)
55+
@_specialize(e#^DIAG_1^#, exported#^DIAG_2^#)#^DIAG_3^#
5256
""",
5357
diagnostics: [
58+
DiagnosticSpec(locationMarker: "DIAG_3", message: "Expected declaration after ')'"),
5459
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected ':' in attribute argument"),
5560
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected ':' in attribute argument"),
5661
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected 'false' in attribute argument"),

Tests/SwiftParserTest/Declarations.swift

Lines changed: 31 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ final class DeclarationTests: XCTestCase {
4141
DiagnosticSpec(locationMarker: "DIAG1", message: "Expected '' in function"),
4242
DiagnosticSpec(locationMarker: "DIAG1", message: "Expected argument list in function declaration"),
4343
DiagnosticSpec(locationMarker: "DIAG2", message: "Expected '=' in same type requirement"),
44+
DiagnosticSpec(locationMarker: "DIAG2", message: "Expected right-hand type of same type requirement"),
4445
])
4546
}
4647

@@ -77,13 +78,15 @@ final class DeclarationTests: XCTestCase {
7778
AssertParse("class T where t#^DIAG^#",
7879
diagnostics: [
7980
DiagnosticSpec(message: "Expected '=' in same type requirement"),
81+
DiagnosticSpec(message: "Expected right-hand type of same type requirement"),
8082
DiagnosticSpec(message: "Expected '{' to start class"),
8183
DiagnosticSpec(message: "Expected '}' to end class"),
8284
])
8385
AssertParse("class B<#^DIAG_1^#where g#^DIAG_2^#",
8486
diagnostics: [
8587
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected '>' to end generic parameter clause"),
8688
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '=' in same type requirement"),
89+
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected right-hand type of same type requirement"),
8790
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '{' to start class"),
8891
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '}' to end class"),
8992
])
@@ -147,7 +150,8 @@ final class DeclarationTests: XCTestCase {
147150
AssertParse(
148151
"_ = foo/* */?.description#^DIAG^#",
149152
diagnostics: [
150-
DiagnosticSpec(message: "Expected ':' after '? ...' in ternary expression")
153+
DiagnosticSpec(message: "Expected ':' after '? ...' in ternary expression"),
154+
DiagnosticSpec(message: "Expected expression"),
151155
]
152156
)
153157

@@ -426,9 +430,12 @@ final class DeclarationTests: XCTestCase {
426430
AssertParse(
427431
"""
428432
struct a {
429-
public
433+
public#^DIAG^#
430434
}
431-
"""
435+
""",
436+
diagnostics: [
437+
DiagnosticSpec(message: "Expected declaration after 'public' in struct")
438+
]
432439
)
433440
}
434441

@@ -519,7 +526,12 @@ final class DeclarationTests: XCTestCase {
519526

520527
func testExtraneousRightBraceRecovery() {
521528
AssertParse(
522-
"class ABC { let def = ghi(jkl: mno) } #^DIAG^#}",
529+
"""
530+
class ABC {
531+
let def = ghi(jkl: mno)
532+
}
533+
#^DIAG^#}
534+
""",
523535
diagnostics: [
524536
DiagnosticSpec(message: "Extraneous '}' at top level")
525537
]
@@ -534,8 +546,8 @@ final class DeclarationTests: XCTestCase {
534546
}
535547
""",
536548
diagnostics: [
537-
// FIXME: This diagnostic should be more contextual
538-
DiagnosticSpec(message: "Expected '->' in return clause")
549+
DiagnosticSpec(message: "Expected '->' in subscript"),
550+
DiagnosticSpec(message: "Expected return type in subscript"),
539551
]
540552
)
541553
}
@@ -575,16 +587,13 @@ final class DeclarationTests: XCTestCase {
575587
func testExpressionMember() {
576588
AssertParse(
577589
"""
578-
struct S {
579-
#^DIAG^#/ ###line 25 "line-directive.swift"
590+
struct S {#^EXPECTED_DECL^#
591+
#^UNEXPECTED_TEXT^#/ ###line 25 "line-directive.swift"
580592
}
581593
""",
582594
diagnostics: [
583-
DiagnosticSpec(
584-
message: """
585-
Unexpected text '/ ###line 25 "line-directive.swift"' in struct
586-
"""
587-
)
595+
DiagnosticSpec(locationMarker: "EXPECTED_DECL", message: "Expected declaration after '{' in struct"),
596+
DiagnosticSpec(locationMarker: "UNEXPECTED_TEXT", message: #"Unexpected text '/ ###line 25 "line-directive.swift"' in struct"#)
588597
]
589598
)
590599
}
@@ -758,12 +767,17 @@ final class DeclarationTests: XCTestCase {
758767
func testMalforedStruct() {
759768
AssertParse(
760769
"""
761-
struct n#^OPENINGBRACES^##if@#^ENDIF^##^CLOSINGBRACES^#
770+
struct n#^OPENING_BRACE^#
771+
#if#^AFTER_POUND_IF^#
772+
@#^END^#
762773
""",
763774
diagnostics: [
764-
DiagnosticSpec(locationMarker: "OPENINGBRACES", message: "Expected '{' to start struct"),
765-
DiagnosticSpec(locationMarker: "ENDIF", message: "Expected '#endif' in conditional compilation block"),
766-
DiagnosticSpec(locationMarker: "CLOSINGBRACES", message: "Expected '}' to end struct")
775+
DiagnosticSpec(locationMarker: "OPENING_BRACE", message: "Expected '{' to start struct"),
776+
DiagnosticSpec(locationMarker: "AFTER_POUND_IF", message: "Expected condition of conditional compilation clause"),
777+
DiagnosticSpec(locationMarker: "END", message: "Expected declaration after '@' in conditional compilation clause"),
778+
DiagnosticSpec(locationMarker: "END", message: "Expected name of attribute"),
779+
DiagnosticSpec(locationMarker: "END", message: "Expected '#endif' in conditional compilation block"),
780+
DiagnosticSpec(locationMarker: "END", message: "Expected '}' to end struct")
767781
]
768782
)
769783
}

0 commit comments

Comments
 (0)