Skip to content

Commit 61ad113

Browse files
kimdvkimdv
committed
Merge pull request #1382 from kimdv/kimdv/Add-diagnostic-for-unexpected-second-identifier
Add diagnostic for unexpected second identifier
1 parent 222b7f4 commit 61ad113

File tree

9 files changed

+157
-107
lines changed

9 files changed

+157
-107
lines changed

‎Sources/SwiftParser/Patterns.swift

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,22 @@ extension Parser {
217217
while !self.at(.eof, .rightParen) && keepGoing && loopProgress.evaluate(currentToken) {
218218
// If the tuple element has a label, parse it.
219219
let labelAndColon = self.consume(if: .identifier, followedBy: .colon)
220-
let (label, colon) = (labelAndColon?.0, labelAndColon?.1)
220+
var (label, colon) = (labelAndColon?.0, labelAndColon?.1)
221+
222+
/// If we have something like `x SomeType`, use the indication that `SomeType` starts with a capital letter (and is thus probably a type name)
223+
/// as an indication that the user forgot to write the colon instead of forgetting to write the comma to separate two elements.
224+
if label == nil, colon == nil, peek().rawTokenKind == .identifier, peek().tokenText.isStartingWithUppercase {
225+
label = consumeAnyToken()
226+
colon = self.missingToken(.colon)
227+
}
228+
221229
let pattern = self.parsePattern()
222-
let trailingComma = self.consume(if: .comma)
230+
var trailingComma = self.consume(if: .comma)
231+
232+
if trailingComma == nil && self.at(TokenSpec(.identifier, allowAtStartOfLine: false)) {
233+
trailingComma = self.missingToken(RawTokenKind.comma)
234+
}
235+
223236
keepGoing = trailingComma != nil
224237
elements.append(
225238
RawTuplePatternElementSyntax(

‎Sources/SwiftParser/SyntaxUtils.swift

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,14 @@ extension SyntaxText {
8888
var isEditorPlaceholder: Bool {
8989
return self.starts(with: SyntaxText("<#")) && self.hasSuffix(SyntaxText("#>"))
9090
}
91+
92+
var isStartingWithUppercase: Bool {
93+
if !self.isEmpty, let firstCharacterByte = self.baseAddress?.pointee {
94+
return 65 <= firstCharacterByte && firstCharacterByte <= 90
95+
} else {
96+
return false
97+
}
98+
}
9199
}
92100

93101
extension RawTokenKind {

‎Sources/SwiftParser/Types.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -548,7 +548,7 @@ extension Parser {
548548
if let first = first,
549549
second == nil,
550550
colon?.isMissing == true,
551-
first.tokenText.description.first?.isUppercase == true
551+
first.tokenText.isStartingWithUppercase
552552
{
553553
elements.append(
554554
RawTupleTypeElementSyntax(

‎Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

Lines changed: 2 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-
FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joined), presence: .present)))),
330+
FixIt.MultiNodeChange(.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-
FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), presence: .present)))),
341+
FixIt.MultiNodeChange(.replace(oldNode: Syntax(previousToken), newNode: Syntax(TokenSyntax(.identifier(joinedUsingCamelCase), trailingTrivia: tokens.last?.trailingTrivia ?? [], presence: .present)))),
342342
.makeMissing(tokens),
343343
]
344344
)

‎Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -383,9 +383,9 @@ public struct SpaceSeparatedIdentifiersError: ParserError {
383383
if let name = firstToken.parent?.ancestorOrSelf(mapping: {
384384
$0.nodeTypeNameForDiagnostics(allowBlockNames: false)
385385
}) {
386-
return "found an unexpected second identifier in \(name)"
386+
return "found an unexpected second identifier in \(name); is there an accidental break?"
387387
} else {
388-
return "found an unexpected second identifier"
388+
return "found an unexpected second identifier; is there an accidental break?"
389389
}
390390
}
391391
}

‎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: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,17 @@
1515
import XCTest
1616

1717
final class TypeTests: XCTestCase {
18-
1918
func testMissingColonInType() {
2019
assertParse(
2120
"""
2221
var foo 1️⃣Bar = 1
2322
""",
2423
diagnostics: [
25-
DiagnosticSpec(message: "expected ':' in type annotation")
26-
]
24+
DiagnosticSpec(message: "expected ':' in type annotation", fixIts: ["insert ':'"])
25+
],
26+
fixedSource: """
27+
var foo: Bar = 1
28+
"""
2729
)
2830
}
2931

@@ -65,7 +67,6 @@ final class TypeTests: XCTestCase {
6567
}
6668

6769
func testClosureSignatures() {
68-
6970
assertParse(
7071
"""
7172
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() {

‎Tests/SwiftParserTest/translated/RecoveryTests.swift

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -742,14 +742,30 @@ final class RecoveryTests: XCTestCase {
742742

743743
//===--- Recovery for multiple identifiers in decls
744744
func testRecovery58() {
745-
assertParse(
746-
"""
747-
protocol Multi 1️⃣ident {}
748-
""",
749-
diagnostics: [
750-
DiagnosticSpec(message: "found an unexpected second identifier in protocol", highlight: "ident")
751-
]
752-
)
745+
let testCases: [UInt: (fixIt: String, fixedSource: String)] = [
746+
#line: ("join the identifiers together", "protocol Multiident {}"),
747+
#line: ("join the identifiers together with camel-case", "protocol MultiIdent {}"),
748+
]
749+
750+
for (line, testCase) in testCases {
751+
assertParse(
752+
"""
753+
protocol Multi 1️⃣ident {}
754+
""",
755+
diagnostics: [
756+
DiagnosticSpec(
757+
message: "found an unexpected second identifier in protocol; is there an accidental break?",
758+
highlight: "ident",
759+
fixIts: ["join the identifiers together", "join the identifiers together with camel-case"],
760+
line: line
761+
)
762+
],
763+
applyFixIts: [testCase.fixIt],
764+
fixedSource: testCase.fixedSource,
765+
line: line
766+
)
767+
}
768+
753769
}
754770

755771
func testRecovery60() {
@@ -758,7 +774,11 @@ final class RecoveryTests: XCTestCase {
758774
class CCC 1️⃣CCC<T> {}
759775
""",
760776
diagnostics: [
761-
DiagnosticSpec(message: "found an unexpected second identifier in class")
777+
DiagnosticSpec(
778+
message: "found an unexpected second identifier in class; is there an accidental break?",
779+
highlight: "CCC<T>",
780+
fixIts: ["join the identifiers together"]
781+
)
762782
],
763783
fixedSource: """
764784
class CCCCCC<T> {}
@@ -775,7 +795,7 @@ final class RecoveryTests: XCTestCase {
775795
}
776796
""",
777797
diagnostics: [
778-
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum"),
798+
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in enum; is there an accidental break?", fixIts: ["join the identifiers together"]),
779799
DiagnosticSpec(locationMarker: "2️⃣", message: "consecutive declarations on a line must be separated by ';'"),
780800
DiagnosticSpec(locationMarker: "3️⃣", message: "unexpected code 'a' before enum case"),
781801
]
@@ -794,24 +814,39 @@ final class RecoveryTests: XCTestCase {
794814
}
795815
"""#,
796816
diagnostics: [
797-
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct"),
817+
DiagnosticSpec(locationMarker: "1️⃣", message: "found an unexpected second identifier in struct; is there an accidental break?", fixIts: ["join the identifiers together"]),
798818
DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' in type annotation"),
799819
DiagnosticSpec(locationMarker: "3️⃣", message: #"unexpected code ': Int = ""' before function"#),
800-
// TODO: (good first issue) Old parser expected error on line 4: found an unexpected second identifier in variable declaration; is there an accidental break?
801820
DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' in type annotation"),
802821
]
803822
)
804823
}
805824

806-
func testRecovery64() {
825+
func testRecovery64a() {
807826
assertParse(
808827
"""
809828
let (efg 1️⃣hij, foobar) = (5, 6)
810829
""",
811830
diagnostics: [
812-
// TODO: (good first issue) Old parser expected error on line 1: found an unexpected second identifier in constant declaration; is there an accidental break?
813-
DiagnosticSpec(message: "unexpected code 'hij, foobar' in tuple pattern")
814-
]
831+
DiagnosticSpec(message: "expected ',' in tuple pattern", fixIts: ["insert ','"])
832+
],
833+
fixedSource: """
834+
let (efg, hij, foobar) = (5, 6)
835+
"""
836+
)
837+
}
838+
839+
func testRecovery64b() {
840+
assertParse(
841+
"""
842+
let (efg 1️⃣Hij, foobar) = (5, 6)
843+
""",
844+
diagnostics: [
845+
DiagnosticSpec(message: "expected ':' in tuple pattern", fixIts: ["insert ':'"])
846+
],
847+
fixedSource: """
848+
let (efg: Hij, foobar) = (5, 6)
849+
"""
815850
)
816851
}
817852

0 commit comments

Comments
 (0)