Skip to content

Commit a68db20

Browse files
kimdvkimdv
committed
Add diagnostic for unexpected second identifier
1 parent 33e4b33 commit a68db20

File tree

8 files changed

+277
-117
lines changed

8 files changed

+277
-117
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 && self.at(TokenSpec(.identifier, allowAtStartOfLine: false)) {
225+
trailingComma = self.missingToken(RawTokenKind.comma)
226+
}
227+
223228
keepGoing = trailingComma != nil
224229
elements.append(
225230
RawTuplePatternElementSyntax(

‎Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -137,10 +137,10 @@ extension FixIt.Changes {
137137
}
138138
}
139139

140-
/// Transfers the leading and trivia trivia of `nodes` to the trailing trivia
141-
/// of the previous token. While doing this, it tries to be smart, merging trivia
142-
/// where it makes sense and refusing to add e.g. a space after punctuation,
143-
/// where it usually doesn't make sense.
140+
/// Transfers the leading and trailing trivia of `nodes` to the previous token
141+
/// While doing this, it tries to be smart, merging trivia where it makes sense
142+
/// and refusing to add e.g. a space after punctuation, where it usually
143+
/// doesn't make sense.
144144
static func transferTriviaAtSides<SyntaxType: SyntaxProtocol>(from nodes: [SyntaxType]) -> Self {
145145
let removedTriviaAtSides = Trivia.merged(nodes.first?.leadingTrivia ?? [], nodes.last?.trailingTrivia ?? [])
146146
if !removedTriviaAtSides.isEmpty, let previousToken = nodes.first?.previousToken(viewMode: .sourceAccurate) {

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -935,6 +935,117 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
935935
return .visitChildren
936936
}
937937

938+
public override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
939+
if shouldSkip(node) {
940+
return .skipChildren
941+
}
942+
943+
if let tuplePatternSyntax = node.pattern.as(TuplePatternSyntax.self) {
944+
for element in tuplePatternSyntax.elements where element.hasError && element.trailingComma?.isMissingAllTokens == true {
945+
if let previousToken = node.previousToken(viewMode: .sourceAccurate) {
946+
var fixIts: [FixIt] = []
947+
948+
if let identifierPatternSyntax = element.pattern.as(IdentifierPatternSyntax.self),
949+
let nextIdentifier = element.nextToken(viewMode: .sourceAccurate)?.as(TokenSyntax.self),
950+
nextIdentifier.rawTokenKind == .identifier
951+
{
952+
953+
if nextIdentifier.text.first?.isUppercase == true {
954+
fixIts += [
955+
FixIt(
956+
message: .joinIdentifiers,
957+
changes: [
958+
[
959+
.replaceTrailingTrivia(
960+
token: identifierPatternSyntax.identifier,
961+
newTrivia: []
962+
)
963+
],
964+
.makeMissing(element.trailingComma),
965+
]
966+
)
967+
]
968+
} else if let trailingComma = element.trailingComma {
969+
fixIts += [
970+
FixIt(
971+
message: .insertComma,
972+
changes: [
973+
.makePresent(trailingComma)
974+
]
975+
)
976+
]
977+
}
978+
979+
// let joined = previousToken.text + tokens.map(\.text).joined()
980+
// fixIts = [
981+
// FixIt(
982+
// message: .joinIdentifiers,
983+
// changes: [
984+
// [.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
985+
// .makeMissing(tokens),
986+
// ]
987+
// )
988+
// ]
989+
// if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
990+
// let joinedUsingCamelCase = previousToken.text + tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
991+
// fixIts.append(
992+
// FixIt(
993+
// message: .joinIdentifiersWithCamelCase,
994+
// changes: [
995+
// [.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
996+
// .makeMissing(tokens),
997+
// ]
998+
// )
999+
// )
1000+
// }
1001+
}
1002+
1003+
addDiagnostic(
1004+
element,
1005+
position: element.nextToken?.position,
1006+
SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []),
1007+
fixIts: fixIts
1008+
)
1009+
}
1010+
}
1011+
} else if node.typeAnnotation?.hasError == true,
1012+
let typeAnnotation = node.typeAnnotation,
1013+
let identifierPattern = node.pattern.as(IdentifierPatternSyntax.self),
1014+
let previousToken = node.previousToken(viewMode: .sourceAccurate),
1015+
typeAnnotation.colon.hasError,
1016+
let type = typeAnnotation.type.as(SimpleTypeIdentifierSyntax.self)
1017+
{
1018+
let tokens = [identifierPattern.identifier, type.name]
1019+
1020+
let joined = tokens.map(\.text).joined()
1021+
var fixIts: [FixIt] = [
1022+
FixIt(
1023+
message: .joinIdentifiers,
1024+
changes: [
1025+
[.replace(oldNode: Syntax(identifierPattern.identifier), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
1026+
.makeMissing(tokens),
1027+
]
1028+
)
1029+
]
1030+
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
1031+
let joinedUsingCamelCase = tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
1032+
fixIts.append(
1033+
FixIt(
1034+
message: .joinIdentifiersWithCamelCase,
1035+
changes: [
1036+
[.replace(oldNode: Syntax(identifierPattern.identifier), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
1037+
.makeMissing(tokens),
1038+
]
1039+
)
1040+
)
1041+
}
1042+
1043+
addDiagnostic(typeAnnotation.type, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []), fixIts: fixIts)
1044+
}
1045+
1046+
return .visitChildren
1047+
}
1048+
9381049
public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
9391050
if shouldSkip(node) {
9401051
return .skipChildren

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,9 @@ public struct SpaceSeparatedIdentifiersError: ParserError {
380380
if let name = firstToken.parent?.ancestorOrSelf(mapping: {
381381
$0.nodeTypeNameForDiagnostics(allowBlockNames: false)
382382
}) {
383-
return "found an unexpected second identifier in \(name)"
383+
return "found an unexpected second identifier in \(name); is there an accidental break?"
384384
} else {
385-
return "found an unexpected second identifier"
385+
return "found an unexpected second identifier; is there an accidental break?"
386386
}
387387
}
388388
}
@@ -495,6 +495,9 @@ extension FixItMessage where Self == StaticParserFixIt {
495495
public static var changeIndentationToMatchClosingDelimiter: Self {
496496
.init("change indentation of this line to match closing delimiter")
497497
}
498+
public static var insertComma: Self {
499+
.init("insert ','")
500+
}
498501
public static var insertSemicolon: Self {
499502
.init("insert ';'")
500503
}

‎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: 19 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,25 @@ 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+
assertParse(
27+
"""
28+
var foo 1️⃣Bar = 1
29+
""",
30+
diagnostics: [
31+
DiagnosticSpec(message: "expected ':' in type annotation"),
32+
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"]),
33+
],
34+
applyFixIts: [testCase.fixIt],
35+
fixedSource: testCase.fixedSource,
36+
line: line
37+
)
38+
}
2839
}
2940

3041
func testClosureParsing() {
@@ -65,7 +76,6 @@ final class TypeTests: XCTestCase {
6576
}
6677

6778
func testClosureSignatures() {
68-
6979
assertParse(
7080
"""
7181
simple { [] str in

‎Tests/SwiftParserTest/translated/InvalidTests.swift

Lines changed: 72 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -388,92 +388,85 @@ final class InvalidTests: XCTestCase {
388388
)
389389
}
390390

391-
func testInvalid23a() {
392-
assertParse(
393-
"""
394-
func dog 1️⃣cow() {}
395-
""",
396-
diagnostics: [
397-
DiagnosticSpec(
398-
message: "found an unexpected second identifier in function",
399-
fixIts: [
400-
"join the identifiers together",
401-
"join the identifiers together with camel-case",
402-
]
403-
)
404-
],
405-
applyFixIts: ["join the identifiers together"],
406-
fixedSource: "func dogcow() {}"
407-
)
408-
}
409-
410-
func testInvalid23b() {
411-
assertParse(
412-
"""
413-
func dog 1️⃣cow() {}
414-
""",
415-
diagnostics: [
416-
DiagnosticSpec(
417-
message: "found an unexpected second identifier in function",
418-
fixIts: [
419-
"join the identifiers together",
420-
"join the identifiers together with camel-case",
421-
]
422-
)
423-
],
424-
applyFixIts: ["join the identifiers together with camel-case"],
425-
fixedSource: "func dogCow() {}"
426-
)
391+
func testInvalid23() {
392+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
393+
#line: ("join the identifiers together", "func dogcow() {}"),
394+
#line: ("join the identifiers together with camel-case", "func dogCow() {}"),
395+
]
396+
397+
for (line, testCase) in testCases {
398+
assertParse(
399+
"""
400+
func dog 1️⃣cow() {}
401+
""",
402+
diagnostics: [
403+
DiagnosticSpec(
404+
message: "found an unexpected second identifier in function; is there an accidental break?",
405+
fixIts: [
406+
"join the identifiers together",
407+
"join the identifiers together with camel-case",
408+
]
409+
)
410+
],
411+
applyFixIts: [testCase.fixIt],
412+
fixedSource: testCase.fixedSource,
413+
line: line
414+
)
415+
}
427416
}
428417

429418
func testThreeIdentifersForFunctionName() {
430-
assertParse(
431-
"""
432-
func dog 1️⃣cow sheep() {}
433-
""",
434-
diagnostics: [
435-
DiagnosticSpec(
436-
message: "found an unexpected second identifier in function",
437-
fixIts: [
438-
"join the identifiers together",
439-
"join the identifiers together with camel-case",
440-
]
441-
)
442-
],
443-
applyFixIts: ["join the identifiers together with camel-case"],
444-
fixedSource: "func dogCowSheep() {}"
445-
)
446-
}
419+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
420+
#line: ("join the identifiers together", "func dogcowsheep() {}"),
421+
#line: ("join the identifiers together with camel-case", "func dogCowSheep() {}"),
422+
]
447423

448-
func testInvalid24() {
449-
assertParse(
450-
"""
451-
func cat 1️⃣Mouse() {}
452-
""",
453-
diagnostics: [
454-
DiagnosticSpec(message: "found an unexpected second identifier in function", fixIts: ["join the identifiers together"])
455-
],
456-
fixedSource: "func catMouse() {}"
457-
)
424+
for (line, testCase) in testCases {
425+
assertParse(
426+
"""
427+
func dog 1️⃣cow sheep() {}
428+
""",
429+
diagnostics: [
430+
DiagnosticSpec(
431+
message: "found an unexpected second identifier in function; is there an accidental break?",
432+
fixIts: [
433+
"join the identifiers together",
434+
"join the identifiers together with camel-case",
435+
]
436+
)
437+
],
438+
applyFixIts: [testCase.fixIt],
439+
fixedSource: testCase.fixedSource,
440+
line: line
441+
)
442+
}
458443
}
459444

460445
func testInvalid25() {
461-
assertParse(
462-
"""
463-
func friend 1️⃣ship<T>(x: T) {}
464-
""",
465-
diagnostics: [
466-
DiagnosticSpec(
467-
message: "found an unexpected second identifier in function",
468-
fixIts: [
469-
"join the identifiers together",
470-
"join the identifiers together with camel-case",
471-
]
472-
)
473-
],
474-
applyFixIts: ["join the identifiers together with camel-case"],
475-
fixedSource: "func friendShip<T>(x: T) {}"
476-
)
446+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
447+
#line: ("join the identifiers together", "func friendship<T>(x: T) {}"),
448+
#line: ("join the identifiers together with camel-case", "func friendShip<T>(x: T) {}"),
449+
]
450+
451+
for (line, testCase) in testCases {
452+
assertParse(
453+
"""
454+
func friend 1️⃣ship<T>(x: T) {}
455+
""",
456+
diagnostics: [
457+
DiagnosticSpec(
458+
message: "found an unexpected second identifier in function; is there an accidental break?",
459+
fixIts: [
460+
"join the identifiers together",
461+
"join the identifiers together with camel-case",
462+
]
463+
)
464+
],
465+
applyFixIts: [testCase.fixIt],
466+
fixedSource: testCase.fixedSource,
467+
line: line
468+
)
469+
}
477470
}
478471

479472
func testInvalid26() {

0 commit comments

Comments
 (0)