Skip to content

Commit 3370247

Browse files
kimdvkimdv
committed
Add diagnostic for unexpected second identifier
1 parent 9ad7a48 commit 3370247

File tree

8 files changed

+261
-115
lines changed

8 files changed

+261
-115
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: 95 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
327327
FixIt(
328328
message: .joinIdentifiers,
329329
changes: [
330-
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
330+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), trailingTrivia: tokens.last?.trailingTrivia ?? [], presence: .present)))],
331331
.makeMissing(tokens),
332332
]
333333
)
@@ -338,7 +338,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
338338
FixIt(
339339
message: .joinIdentifiersWithCamelCase,
340340
changes: [
341-
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
341+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), trailingTrivia: tokens.last?.trailingTrivia ?? [], presence: .present)))],
342342
.makeMissing(tokens),
343343
]
344344
)
@@ -935,6 +935,99 @@ 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+
980+
addDiagnostic(
981+
element,
982+
position: element.nextToken?.position,
983+
SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []),
984+
fixIts: fixIts
985+
)
986+
}
987+
}
988+
} else if node.typeAnnotation?.hasError == true,
989+
let typeAnnotation = node.typeAnnotation,
990+
let identifierPattern = node.pattern.as(IdentifierPatternSyntax.self),
991+
let previousToken = node.previousToken(viewMode: .sourceAccurate),
992+
typeAnnotation.colon.hasError,
993+
let type = typeAnnotation.type.as(SimpleTypeIdentifierSyntax.self)
994+
{
995+
let tokens = [identifierPattern.identifier, type.name]
996+
997+
let joined = tokens.map(\.text).joined()
998+
var fixIts: [FixIt] = [
999+
FixIt(
1000+
message: .joinIdentifiers,
1001+
changes: [
1002+
[.replace(oldNode: Syntax(identifierPattern.identifier), newNode: Syntax(TokenSyntax(.identifier(joined), trailingTrivia: type.trailingTrivia ?? [], presence: .present)))],
1003+
.makeMissing(tokens),
1004+
]
1005+
)
1006+
]
1007+
if type.name.text.first?.isUppercase == false {
1008+
let joinedUsingCamelCase = "\(identifierPattern.identifier.text)\(type.name.text.withFirstLetterUppercased())"
1009+
fixIts.append(
1010+
FixIt(
1011+
message: .joinIdentifiersWithCamelCase,
1012+
changes: [
1013+
[.replace(oldNode: Syntax(identifierPattern.identifier), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
1014+
.makeMissing(tokens),
1015+
]
1016+
)
1017+
)
1018+
}
1019+
1020+
addDiagnostic(
1021+
typeAnnotation.type,
1022+
SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []),
1023+
fixIts: fixIts,
1024+
handledNodes: [identifierPattern.id, type.id]
1025+
)
1026+
}
1027+
1028+
return .visitChildren
1029+
}
1030+
9381031
public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
9391032
if shouldSkip(node) {
9401033
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: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,30 @@
1515
import XCTest
1616

1717
final class TypeTests: XCTestCase {
18-
1918
func testMissingColonInType() {
20-
assertParse(
21-
"""
22-
var foo 1️⃣Bar = 1
23-
""",
24-
diagnostics: [
25-
DiagnosticSpec(message: "expected ':' in type annotation")
26-
]
27-
)
19+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
20+
#line: ("join the identifiers together", "var fooBar = 1"),
21+
#line: ("insert ':'", "var foo: Bar = 1"),
22+
]
23+
24+
for (line, testCase) in testCases {
25+
assertParse(
26+
"""
27+
var foo 1️⃣Bar = 1
28+
""",
29+
diagnostics: [
30+
DiagnosticSpec(message: "expected ':' in type annotation", fixIts: ["insert ':'"], line: line),
31+
DiagnosticSpec(
32+
message: "found an unexpected second identifier in variable; is there an accidental break?",
33+
fixIts: ["join the identifiers together"],
34+
line: line
35+
),
36+
],
37+
applyFixIts: [testCase.fixIt],
38+
fixedSource: testCase.fixedSource,
39+
line: line
40+
)
41+
}
2842
}
2943

3044
func testClosureParsing() {
@@ -65,7 +79,6 @@ final class TypeTests: XCTestCase {
6579
}
6680

6781
func testClosureSignatures() {
68-
6982
assertParse(
7083
"""
7184
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)