Skip to content

Commit 582a48f

Browse files
kimdvkimdv
committed
Add diagnostic for unexpected second identifier
1 parent 07c08da commit 582a48f

File tree

7 files changed

+141
-31
lines changed

7 files changed

+141
-31
lines changed

‎Sources/SwiftParser/Patterns.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,12 @@ extension Parser {
219219
let labelAndColon = self.consume(if: .identifier, followedBy: .colon)
220220
let (label, colon) = (labelAndColon?.0, labelAndColon?.1)
221221
let pattern = self.parsePattern()
222-
let trailingComma = self.consume(if: .comma)
222+
var trailingComma = self.consume(if: .comma)
223+
224+
if trailingComma == nil && currentToken.rawTokenKind == .identifier {
225+
trailingComma = self.missingToken(RawTokenKind.comma)
226+
}
227+
223228
keepGoing = trailingComma != nil
224229
elements.append(
225230
RawTuplePatternElementSyntax(

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -878,6 +878,84 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
878878
return .visitChildren
879879
}
880880

881+
public override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
882+
if shouldSkip(node) {
883+
return .skipChildren
884+
}
885+
886+
if let tuplePatternSyntax = node.pattern.as(TuplePatternSyntax.self) {
887+
for element in tuplePatternSyntax.elements where element.hasError && element.trailingComma?.isMissingAllTokens == true {
888+
if let previousToken = node.previousToken(viewMode: .sourceAccurate) {
889+
var fixIts: [FixIt] = []
890+
891+
if let identifierPatternSyntax = element.pattern.as(IdentifierPatternSyntax.self), let nextIdentifier = element.nextToken(viewMode: .sourceAccurate)?.as(TokenSyntax.self), nextIdentifier.rawTokenKind == .identifier {
892+
let tokens = [identifierPatternSyntax.identifier, nextIdentifier]
893+
894+
let joined = previousToken.text + tokens.map(\.text).joined()
895+
fixIts = [
896+
FixIt(
897+
message: .joinIdentifiers,
898+
changes: [
899+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
900+
.makeMissing(tokens),
901+
]
902+
)
903+
]
904+
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
905+
let joinedUsingCamelCase = previousToken.text + tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
906+
fixIts.append(
907+
FixIt(
908+
message: .joinIdentifiersWithCamelCase,
909+
changes: [
910+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
911+
.makeMissing(tokens),
912+
]
913+
)
914+
)
915+
}
916+
}
917+
918+
addDiagnostic(element, position: element.nextToken?.position, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []), fixIts: fixIts)
919+
}
920+
}
921+
} else if node.typeAnnotation?.hasError == true,
922+
let typeAnnotation = node.typeAnnotation,
923+
let identifierPattern = node.pattern.as(IdentifierPatternSyntax.self),
924+
let previousToken = node.previousToken(viewMode: .sourceAccurate),
925+
typeAnnotation.colon.hasError,
926+
let type = typeAnnotation.type.as(SimpleTypeIdentifierSyntax.self)
927+
{
928+
let tokens = [identifierPattern.identifier, type.name]
929+
930+
let joined = previousToken.text + tokens.map(\.text).joined()
931+
var fixIts: [FixIt] = [
932+
FixIt(
933+
message: .joinIdentifiers,
934+
changes: [
935+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
936+
.makeMissing(tokens),
937+
]
938+
)
939+
]
940+
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
941+
let joinedUsingCamelCase = previousToken.text + tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
942+
fixIts.append(
943+
FixIt(
944+
message: .joinIdentifiersWithCamelCase,
945+
changes: [
946+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
947+
.makeMissing(tokens),
948+
]
949+
)
950+
)
951+
}
952+
953+
addDiagnostic(typeAnnotation.type, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []), fixIts: fixIts)
954+
}
955+
956+
return .visitChildren
957+
}
958+
881959
public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
882960
if shouldSkip(node) {
883961
return .skipChildren

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -341,13 +341,16 @@ public struct SpaceSeparatedIdentifiersError: ParserError {
341341
public let additionalTokens: [TokenSyntax]
342342

343343
public var message: String {
344+
let text: String
344345
if let name = firstToken.parent?.ancestorOrSelf(mapping: {
345346
$0.nodeTypeNameForDiagnostics(allowBlockNames: false)
346347
}) {
347-
return "found an unexpected second identifier in \(name)"
348+
text = "found an unexpected second identifier in \(name)"
348349
} else {
349-
return "found an unexpected second identifier"
350+
text = "found an unexpected second identifier"
350351
}
352+
353+
return "\(text); is there an accidental break?"
351354
}
352355
}
353356

‎Tests/SwiftParserTest/ParserTests.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public class ParserTests: XCTestCase {
5454
}
5555
}
5656

57-
/// Run parsr tests on all of the Swift files in the given path, recursively.
57+
/// Run parser tests on all of the Swift files in the given path, recursively.
5858
func runParserTests(
5959
name: String,
6060
path: URL,

‎Tests/SwiftParserTest/TypeTests.swift

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,26 @@ import XCTest
1717
final class TypeTests: XCTestCase {
1818

1919
func testMissingColonInType() {
20-
AssertParse(
21-
"""
22-
var foo 1️⃣Bar = 1
23-
""",
24-
diagnostics: [
25-
DiagnosticSpec(message: "expected ':' in type annotation")
26-
]
27-
)
20+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
21+
#line: ("join the identifiers together", "var fooBar = 1"),
22+
#line: ("join the identifiers together with camel-case", "var fooBar = 1"),
23+
]
24+
25+
for (line, testCase) in testCases {
26+
27+
AssertParse(
28+
"""
29+
var foo 1️⃣Bar = 1
30+
""",
31+
diagnostics: [
32+
DiagnosticSpec(message: "expected ':' in type annotation"),
33+
DiagnosticSpec(message: "found an unexpected second identifier in variable; is there an accidental break?", fixIts: ["join the identifiers together", "join the identifiers together with camel-case"]),
34+
],
35+
applyFixIts: [testCase.fixIt],
36+
fixedSource: testCase.fixedSource,
37+
line: line
38+
)
39+
}
2840
}
2941

3042
func testClosureParsing() {

‎Tests/SwiftParserTest/translated/InvalidTests.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -394,7 +394,7 @@ final class InvalidTests: XCTestCase {
394394
""",
395395
diagnostics: [
396396
DiagnosticSpec(
397-
message: "found an unexpected second identifier in function",
397+
message: "found an unexpected second identifier in function; is there an accidental break?",
398398
fixIts: [
399399
"join the identifiers together",
400400
"join the identifiers together with camel-case",
@@ -413,7 +413,7 @@ final class InvalidTests: XCTestCase {
413413
""",
414414
diagnostics: [
415415
DiagnosticSpec(
416-
message: "found an unexpected second identifier in function",
416+
message: "found an unexpected second identifier in function; is there an accidental break?",
417417
fixIts: [
418418
"join the identifiers together",
419419
"join the identifiers together with camel-case",
@@ -432,7 +432,7 @@ final class InvalidTests: XCTestCase {
432432
""",
433433
diagnostics: [
434434
DiagnosticSpec(
435-
message: "found an unexpected second identifier in function",
435+
message: "found an unexpected second identifier in function; is there an accidental break?",
436436
fixIts: [
437437
"join the identifiers together",
438438
"join the identifiers together with camel-case",
@@ -450,7 +450,7 @@ final class InvalidTests: XCTestCase {
450450
func cat 1️⃣Mouse() {}
451451
""",
452452
diagnostics: [
453-
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: ["join the identifiers together"])
453+
DiagnosticSpec(message: "found an unexpected second identifier in function; is there an accidental break?", fixIts: ["join the identifiers together"])
454454
],
455455
fixedSource: "func catMouse() {}"
456456
)
@@ -463,7 +463,7 @@ final class InvalidTests: XCTestCase {
463463
""",
464464
diagnostics: [
465465
DiagnosticSpec(
466-
message: "found an unexpected second identifier in function",
466+
message: "found an unexpected second identifier in function; is there an accidental break?",
467467
fixIts: [
468468
"join the identifiers together",
469469
"join the identifiers together with camel-case",

‎Tests/SwiftParserTest/translated/RecoveryTests.swift

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -760,14 +760,24 @@ final class RecoveryTests: XCTestCase {
760760

761761
//===--- Recovery for multiple identifiers in decls
762762
func testRecovery58() {
763-
AssertParse(
764-
"""
765-
protocol Multi 1️⃣ident {}
766-
""",
767-
diagnostics: [
768-
DiagnosticSpec(message: "found an unexpected second identifier in protocol")
769-
]
770-
)
763+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
764+
#line: ("join the identifiers together", "protocol Multiident{}"),
765+
#line: ("join the identifiers together with camel-case", "protocol MultiIdent{}"),
766+
]
767+
768+
for (line, testCase) in testCases {
769+
AssertParse(
770+
"""
771+
protocol Multi 1️⃣ident {}
772+
""",
773+
diagnostics: [
774+
DiagnosticSpec(message: "found an unexpected second identifier in protocol; is there an accidental break?", fixIts: ["join the identifiers together", "join the identifiers together with camel-case"])
775+
],
776+
applyFixIts: [testCase.fixIt],
777+
fixedSource: testCase.fixedSource,
778+
line: line
779+
)
780+
}
771781
}
772782

773783
func testRecovery60() {
@@ -776,7 +786,7 @@ final class RecoveryTests: XCTestCase {
776786
class CCC 1️⃣CCC<T> {}
777787
""",
778788
diagnostics: [
779-
DiagnosticSpec(message: "found an unexpected second identifier in class")
789+
DiagnosticSpec(message: "found an unexpected second identifier in class; is there an accidental break?", fixIts: ["join the identifiers together", "join the identifiers together with camel-case"])
780790
],
781791
fixedSource: """
782792
class CCCCCC<T> {}
@@ -793,7 +803,7 @@ final class RecoveryTests: XCTestCase {
793803
}
794804
""",
795805
diagnostics: [
796-
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum"),
806+
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum; is there an accidental break?", fixIts: ["join the identifiers together", "join the identifiers together with camel-case"]),
797807
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'"),
798808
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
799809
]
@@ -812,23 +822,25 @@ final class RecoveryTests: XCTestCase {
812822
}
813823
"""#,
814824
diagnostics: [
815-
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct"),
825+
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct; is there an accidental break?", fixIts: ["join the identifiers together"]),
816826
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' in type annotation"),
827+
DiagnosticSpec(locationMarker: "2️⃣", message: "found an unexpected second identifier in variable; is there an accidental break?", fixIts: ["join the identifiers together"]),
817828
DiagnosticSpec(locationMarker: "3️⃣", message: #"unexpected code ': Int = ""' before function"#),
818-
// TODO: (good first issue) Old parser expected error on line 4: found an unexpected second identifier in variable declaration; is there an accidental break?
819829
DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' in type annotation"),
830+
DiagnosticSpec(locationMarker: "4️⃣", message: "found an unexpected second identifier in variable; is there an accidental break?", fixIts: ["join the identifiers together"]),
820831
]
821832
)
822833
}
823834

824835
func testRecovery64() {
836+
825837
AssertParse(
826838
"""
827839
let (efg 1️⃣hij, foobar) = (5, 6)
828840
""",
829841
diagnostics: [
830-
// TODO: (good first issue) Old parser expected error on line 1: found an unexpected second identifier in constant declaration; is there an accidental break?
831-
DiagnosticSpec(message: "unexpected code 'hij, foobar' in tuple pattern")
842+
DiagnosticSpec(message: "expected ',' in tuple pattern"),
843+
DiagnosticSpec(message: "found an unexpected second identifier in variable; is there an accidental break?", fixIts: ["join the identifiers together", "join the identifiers together with camel-case"]),
832844
]
833845
)
834846
}

0 commit comments

Comments
 (0)