Skip to content

Improve diagnostic message if unexpected tokens are found at the start of a layout node #736

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 8, 2022
Merged

Improve diagnostic message if unexpected tokens are found at the start of a layout node #736

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 23 additions & 15 deletions Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,20 @@ import SwiftSyntax
let diagnosticDomain: String = "SwiftParser"

extension SyntaxProtocol {
var nodeTypeNameForDiagnostics: String? {
/// Return a name of this syntax node that can be used to describe it in
/// diagnostics.
/// Nodes that mostly exist for the syntax tree's structure and don't have a
/// correspondence in the source code that's meeingful to the user by default
/// use the name of the parent node that encloses it. Pass `false` to `inherit`
/// to prevent this name inheritance.
Comment on lines +19 to +24
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is already merged, but FWIW there's meeingful + that sentence is a little hard to parse. Maybe:

Return the name of this syntax node to be used to describe it in diagnostics. If the node itself has no diagnostic name (eg. it exists for the syntax tree's structure), use the name of the enclosing parent node by default. Pass inherit: false to prevent this behavior, in which case nil will be returned instead.

Copy link
Member Author

@ahoppen ahoppen Sep 9, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the suggestion. I put up a PR to incorporate the new description #760

func nodeTypeNameForDiagnostics(inherit: Bool = true) -> String? {
if let name = Syntax(self).as(SyntaxEnum.self).nameForDiagnostics {
return name
}
if let parent = self.parent {
return parent.nodeTypeNameForDiagnostics
if inherit {
if let parent = self.parent {
return parent.nodeTypeNameForDiagnostics(inherit: inherit)
}
}
return nil
}
Expand Down Expand Up @@ -118,7 +126,7 @@ public struct MissingTokenError: ParserError {
public let missingToken: TokenSyntax

public var message: String {
guard let parent = missingToken.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics else {
guard let parent = missingToken.parent, let parentTypeName = parent.nodeTypeNameForDiagnostics() else {
return "Expected '\(missingToken.text)'"
}
switch missingToken.tokenKind {
Expand All @@ -141,17 +149,17 @@ public struct UnexpectedNodesError: ParserError {
public let unexpectedNodes: UnexpectedNodesSyntax

public var message: String {
let parentTypeName = unexpectedNodes.parent?.nodeTypeNameForDiagnostics
let shortContent = unexpectedNodes.contentForDiagnosticsIfShortSingleLine
switch (parentTypeName, shortContent) {
case (let parentTypeName?, let shortContent?):
return "Unexpected text '\(shortContent)' found in \(parentTypeName)"
case (let parentTypeName?, nil):
return "Unexpected text found in \(parentTypeName)"
case (nil, let shortContent?):
return "Unexpected text '\(shortContent)'"
case (nil, nil):
return "Unexpected text"
var message = "Unexpected text"
if let shortContent = unexpectedNodes.contentForDiagnosticsIfShortSingleLine {
message += " '\(shortContent)'"
}
if let parent = unexpectedNodes.parent {
if let parentTypeName = parent.nodeTypeNameForDiagnostics(inherit: false), parent.children(viewMode: .sourceAccurate).first?.id == unexpectedNodes.id {
message += " before \(parentTypeName)"
} else if let parentTypeName = parent.nodeTypeNameForDiagnostics() {
message += " in \(parentTypeName)"
}
}
return message
}
}
18 changes: 9 additions & 9 deletions Tests/SwiftParserTest/Declarations.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ final class DeclarationTests: XCTestCase {
func foo() {}
""",
diagnostics: [
DiagnosticSpec(message: "Unexpected text '}' found in function")
DiagnosticSpec(message: "Unexpected text '}' before function")
]
)
}
Expand Down Expand Up @@ -115,7 +115,7 @@ final class DeclarationTests: XCTestCase {
actor Foo {}
""",
diagnostics: [
DiagnosticSpec(message: "Unexpected text '}' found in actor")
DiagnosticSpec(message: "Unexpected text '}' before actor")
]
)
}
Expand Down Expand Up @@ -417,7 +417,7 @@ final class DeclarationTests: XCTestCase {
"(first second #^DIAG^#third fourth: Int)",
{ $0.parseFunctionSignature() },
diagnostics: [
DiagnosticSpec(message: "Unexpected text 'third fourth' found in function parameter")
DiagnosticSpec(message: "Unexpected text 'third fourth' in function parameter")
]
)
}
Expand Down Expand Up @@ -559,7 +559,7 @@ final class DeclarationTests: XCTestCase {
diagnostics: [
DiagnosticSpec(
message: """
Unexpected text '/ ###line 25 "line-directive.swift"' found in struct
Unexpected text '/ ###line 25 "line-directive.swift"' in struct
"""
)
]
Expand All @@ -574,7 +574,7 @@ final class DeclarationTests: XCTestCase {
}
""",
diagnostics: [
DiagnosticSpec(message: "Unexpected text 'bogus rethrows set' found in variable")
DiagnosticSpec(message: "Unexpected text 'bogus rethrows set' in variable")
]
)
}
Expand Down Expand Up @@ -606,7 +606,7 @@ final class DeclarationTests: XCTestCase {
trailingComma: nil
)),
diagnostics: [
DiagnosticSpec(message: "Unexpected text 'third' found in function parameter")
DiagnosticSpec(message: "Unexpected text 'third' in function parameter")
]
)
}
Expand All @@ -627,7 +627,7 @@ final class DeclarationTests: XCTestCase {
trailingComma: nil
)),
diagnostics: [
DiagnosticSpec(message: "Unexpected text 'third fourth' found in function parameter")
DiagnosticSpec(message: "Unexpected text 'third fourth' in function parameter")
]
)
}
Expand Down Expand Up @@ -678,7 +678,7 @@ final class DeclarationTests: XCTestCase {
trailingComma: nil
)),
diagnostics: [
DiagnosticSpec(message: "Unexpected text '[third fourth]' found in function parameter")
DiagnosticSpec(message: "Unexpected text '[third fourth]' in function parameter")
]
)
}
Expand All @@ -703,7 +703,7 @@ final class DeclarationTests: XCTestCase {
diagnostics: [
DiagnosticSpec(locationMarker: "COLON", message: "Expected ':' in function parameter"),
DiagnosticSpec(locationMarker: "END_ARRAY" , message: "Expected ']' to end array type"),
DiagnosticSpec(locationMarker: "END_ARRAY", message: "Unexpected text 'fourth: Int' found in parameter clause")
DiagnosticSpec(locationMarker: "END_ARRAY", message: "Unexpected text 'fourth: Int' in parameter clause")
]
)
}
Expand Down
4 changes: 2 additions & 2 deletions Tests/SwiftParserTest/Directives.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,8 @@ final class DirectiveTests: XCTestCase {
#endif
""",
diagnostics: [
DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text '}' found in conditional compilation clause"),
DiagnosticSpec(locationMarker: "DIAG_2", message: "Unexpected text '}' found in conditional compilation block"),
DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text '}' before conditional compilation clause"),
DiagnosticSpec(locationMarker: "DIAG_2", message: "Unexpected text '}' in conditional compilation block"),
]
)
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/SwiftParserTest/Expressions.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ final class ExpressionTests: XCTestCase {
" >> \( abc #^DIAG^#} ) << "
"""#,
diagnostics: [
DiagnosticSpec(message: "Unexpected text '}' found in string literal")
DiagnosticSpec(message: "Unexpected text '}' in string literal")
]
)

Expand Down Expand Up @@ -426,7 +426,7 @@ final class ExpressionTests: XCTestCase {
"[(Int) -> #^DIAG^#throws Int]()",
diagnostics: [
// FIXME: We should suggest to move 'throws' in front of '->'
DiagnosticSpec(message: "Unexpected text 'throws Int' found in array")
DiagnosticSpec(message: "Unexpected text 'throws Int' in array")
]
)

Expand Down Expand Up @@ -456,7 +456,7 @@ final class ExpressionTests: XCTestCase {
let :(#^DIAG_1^#..)->
""",
diagnostics: [
DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text '..' found in function type"),
DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text '..' in function type"),
]
)
}
Expand Down
12 changes: 6 additions & 6 deletions Tests/SwiftParserTest/Statements.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ final class StatementTests: XCTestCase {
""",
diagnostics: [
DiagnosticSpec(message: "Expected '=' in pattern matching"),
DiagnosticSpec(message: "Unexpected text '* ! = x' found in 'if' statement"),
DiagnosticSpec(message: "Unexpected text '* ! = x' in 'if' statement"),
]
)
}
Expand Down Expand Up @@ -174,8 +174,8 @@ final class StatementTests: XCTestCase {
}
""",
diagnostics: [
DiagnosticSpec(locationMarker: "TEST_1", message: "Unexpected text '@s return' found in function"),
DiagnosticSpec(locationMarker: "TEST_2", message: "Unexpected text '@unknown return' found in function")
DiagnosticSpec(locationMarker: "TEST_1", message: "Unexpected text '@s return' in function"),
DiagnosticSpec(locationMarker: "TEST_2", message: "Unexpected text '@unknown return' in function")
]
)
}
Expand All @@ -193,8 +193,8 @@ final class StatementTests: XCTestCase {
}
""",
diagnostics: [
DiagnosticSpec(locationMarker: "FOO", message: "Unexpected text 'foo()' found in conditional compilation clause"),
DiagnosticSpec(locationMarker: "BAR", message: "Unexpected text 'bar()' found in conditional compilation block"),
DiagnosticSpec(locationMarker: "FOO", message: "Unexpected text 'foo()' before conditional compilation clause"),
DiagnosticSpec(locationMarker: "BAR", message: "Unexpected text 'bar()' in conditional compilation block"),
]
)

Expand All @@ -213,7 +213,7 @@ final class StatementTests: XCTestCase {
}
""",
diagnostics: [
DiagnosticSpec(message: "Unexpected text 'print()' found in conditional compilation clause")
DiagnosticSpec(message: "Unexpected text 'print()' before conditional compilation clause")
]
)
}
Expand Down
6 changes: 3 additions & 3 deletions Tests/SwiftParserTest/Types.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class TypeTests: XCTestCase {

func testFunctionTypes() throws {
AssertParse("t as(#^DIAG^#..)->", diagnostics: [
DiagnosticSpec(message: "Unexpected text '..' found in function type")
DiagnosticSpec(message: "Unexpected text '..' in function type")
])
}

Expand All @@ -49,14 +49,14 @@ final class TypeTests: XCTestCase {
{ $0.parseClosureExpression() },
diagnostics: [
DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected '' in closure capture item"),
DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text 'class' found in closure capture signature"),
DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text 'class' in closure capture signature"),
DiagnosticSpec(locationMarker: "DIAG_2", message: "Expected '}' to end closure"),
])

AssertParse("{[n#^DIAG^#`]in}",
{ $0.parseClosureExpression() },
diagnostics: [
DiagnosticSpec(message: "Unexpected text '`' found in closure capture signature")
DiagnosticSpec(message: "Unexpected text '`' in closure capture signature")
])
}
}