Skip to content

Commit 618d2aa

Browse files
kimdvkimdv
committed
Add diagnostic for unexpected second identifier
1 parent 39b3336 commit 618d2aa

File tree

8 files changed

+303
-153
lines changed

8 files changed

+303
-153
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: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -896,6 +896,84 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
896896
return .visitChildren
897897
}
898898

899+
public override func visit(_ node: PatternBindingSyntax) -> SyntaxVisitorContinueKind {
900+
if shouldSkip(node) {
901+
return .skipChildren
902+
}
903+
904+
if let tuplePatternSyntax = node.pattern.as(TuplePatternSyntax.self) {
905+
for element in tuplePatternSyntax.elements where element.hasError && element.trailingComma?.isMissingAllTokens == true {
906+
if let previousToken = node.previousToken(viewMode: .sourceAccurate) {
907+
var fixIts: [FixIt] = []
908+
909+
if let identifierPatternSyntax = element.pattern.as(IdentifierPatternSyntax.self), let nextIdentifier = element.nextToken(viewMode: .sourceAccurate)?.as(TokenSyntax.self), nextIdentifier.rawTokenKind == .identifier {
910+
let tokens = [identifierPatternSyntax.identifier, nextIdentifier]
911+
912+
let joined = previousToken.text + tokens.map(\.text).joined()
913+
fixIts = [
914+
FixIt(
915+
message: .joinIdentifiers,
916+
changes: [
917+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
918+
.makeMissing(tokens),
919+
]
920+
)
921+
]
922+
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
923+
let joinedUsingCamelCase = previousToken.text + tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
924+
fixIts.append(
925+
FixIt(
926+
message: .joinIdentifiersWithCamelCase,
927+
changes: [
928+
[.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
929+
.makeMissing(tokens),
930+
]
931+
)
932+
)
933+
}
934+
}
935+
936+
addDiagnostic(element, position: element.nextToken?.position, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []), fixIts: fixIts)
937+
}
938+
}
939+
} else if node.typeAnnotation?.hasError == true,
940+
let typeAnnotation = node.typeAnnotation,
941+
let identifierPattern = node.pattern.as(IdentifierPatternSyntax.self),
942+
let previousToken = node.previousToken(viewMode: .sourceAccurate),
943+
typeAnnotation.colon.hasError,
944+
let type = typeAnnotation.type.as(SimpleTypeIdentifierSyntax.self)
945+
{
946+
let tokens = [identifierPattern.identifier, type.name]
947+
948+
let joined = tokens.map(\.text).joined()
949+
var fixIts: [FixIt] = [
950+
FixIt(
951+
message: .joinIdentifiers,
952+
changes: [
953+
[.replace(oldNode: Syntax(identifierPattern.identifier), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))],
954+
.makeMissing(tokens),
955+
]
956+
)
957+
]
958+
if tokens.contains(where: { $0.text.first?.isUppercase == false }) {
959+
let joinedUsingCamelCase = tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
960+
fixIts.append(
961+
FixIt(
962+
message: .joinIdentifiersWithCamelCase,
963+
changes: [
964+
[.replace(oldNode: Syntax(identifierPattern.identifier), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))],
965+
.makeMissing(tokens),
966+
]
967+
)
968+
)
969+
}
970+
971+
addDiagnostic(typeAnnotation.type, SpaceSeparatedIdentifiersError(firstToken: previousToken, additionalTokens: []), fixIts: fixIts)
972+
}
973+
974+
return .visitChildren
975+
}
976+
899977
public override func visit(_ node: PrecedenceGroupAssignmentSyntax) -> SyntaxVisitorContinueKind {
900978
if shouldSkip(node) {
901979
return .skipChildren

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -347,9 +347,9 @@ public struct SpaceSeparatedIdentifiersError: ParserError {
347347
if let name = firstToken.parent?.ancestorOrSelf(mapping: {
348348
$0.nodeTypeNameForDiagnostics(allowBlockNames: false)
349349
}) {
350-
return "found an unexpected second identifier in \(name)"
350+
return "found an unexpected second identifier in \(name); is there an accidental break?"
351351
} else {
352-
return "found an unexpected second identifier"
352+
return "found an unexpected second identifier; is there an accidental break?"
353353
}
354354
}
355355
}

‎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
@@ -387,92 +387,85 @@ final class InvalidTests: XCTestCase {
387387
)
388388
}
389389

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

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

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

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

478471
func testInvalid26() {

0 commit comments

Comments
 (0)