Skip to content

Commit 52a8828

Browse files
kimdvkimdv
authored
Merge pull request #1382 from kimdv/kimdv/Add-diagnostic-for-unexpected-second-identifier
Add diagnostic for unexpected second identifier
2 parents eff37e3 + 54a278e commit 52a8828

10 files changed

+161
-111
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
@@ -536,7 +536,7 @@ extension Parser {
536536
if let first = first,
537537
second == nil,
538538
colon?.isMissing == true,
539-
first.tokenText.description.first?.isUppercase == true
539+
first.tokenText.isStartingWithUppercase
540540
{
541541
elements.append(
542542
RawTupleTypeElementSyntax(

‎Sources/SwiftParserDiagnostics/DiagnosticExtensions.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ extension FixIt.MultiNodeChange {
164164
}
165165
}
166166

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

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

0 commit comments

Comments
 (0)