Skip to content

Commit ec13f3d

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

File tree

8 files changed

+303
-152
lines changed

8 files changed

+303
-152
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
@@ -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 = tokens.map(\.text).joined()
931+
var fixIts: [FixIt] = [
932+
FixIt(
933+
message: .joinIdentifiers,
934+
changes: [
935+
[.replace(oldNode: Syntax(identifierPattern.identifier), 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 = tokens.map({ $0.text.withFirstLetterUppercased() }).joined()
942+
fixIts.append(
943+
FixIt(
944+
message: .joinIdentifiersWithCamelCase,
945+
changes: [
946+
[.replace(oldNode: Syntax(identifierPattern.identifier), 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: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -344,9 +344,9 @@ public struct SpaceSeparatedIdentifiersError: ParserError {
344344
if let name = firstToken.parent?.ancestorOrSelf(mapping: {
345345
$0.nodeTypeNameForDiagnostics(allowBlockNames: false)
346346
}) {
347-
return "found an unexpected second identifier in \(name)"
347+
return "found an unexpected second identifier in \(name); is there an accidental break?"
348348
} else {
349-
return "found an unexpected second identifier"
349+
return "found an unexpected second identifier; is there an accidental break?"
350350
}
351351
}
352352
}

‎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 & 8 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() {

‎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)