Skip to content

Commit e88fb15

Browse files
kimdvbnbarham
kimdv
authored and
bnbarham
committed
Add missing comma diagnostic if it's inside an array or dictionary
(cherry picked from commit 9a2da83)
1 parent a34f554 commit e88fb15

File tree

4 files changed

+120
-46
lines changed

4 files changed

+120
-46
lines changed

‎Sources/SwiftParser/Expressions.swift

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1498,17 +1498,38 @@ extension Parser {
14981498
var elements = [RawSyntax]()
14991499
do {
15001500
var collectionProgress = LoopProgressCondition()
1501-
COLLECTION_LOOP: while self.hasProgressed(&collectionProgress) {
1501+
var keepGoing: RawTokenSyntax?
1502+
COLLECTION_LOOP: repeat {
15021503
elementKind = self.parseCollectionElement(elementKind)
15031504

1505+
/// Whether expression of an array element or the value of a dictionary
1506+
/// element is missing. If this is the case, we shouldn't recover from
1507+
/// a missing comma since most likely the closing `]` is missing.
1508+
var elementIsMissingExpression: Bool {
1509+
switch elementKind! {
1510+
case .dictionary(_, _, _, let value):
1511+
return value.is(RawMissingExprSyntax.self)
1512+
case .array(let rawExprSyntax):
1513+
return rawExprSyntax.is(RawMissingExprSyntax.self)
1514+
}
1515+
}
1516+
15041517
// Parse the ',' if exists.
1505-
let comma = self.consume(if: .comma)
1518+
if let token = self.consume(if: .comma) {
1519+
keepGoing = token
1520+
} else if !self.at(.rightSquare, .endOfFile) && !self.atStartOfLine && !elementIsMissingExpression && !self.atStartOfDeclaration()
1521+
&& !self.atStartOfStatement(preferExpr: false)
1522+
{
1523+
keepGoing = missingToken(.comma)
1524+
} else {
1525+
keepGoing = nil
1526+
}
15061527

15071528
switch elementKind! {
15081529
case .array(let el):
15091530
let element = RawArrayElementSyntax(
15101531
expression: el,
1511-
trailingComma: comma,
1532+
trailingComma: keepGoing,
15121533
arena: self.arena
15131534
)
15141535
if element.isEmpty {
@@ -1522,7 +1543,7 @@ extension Parser {
15221543
unexpectedBeforeColon,
15231544
colon: colon,
15241545
value: value,
1525-
trailingComma: comma,
1546+
trailingComma: keepGoing,
15261547
arena: self.arena
15271548
)
15281549
if element.isEmpty {
@@ -1531,29 +1552,7 @@ extension Parser {
15311552
elements.append(RawSyntax(element))
15321553
}
15331554
}
1534-
1535-
// If we saw a comma, that's a strong indicator we have more elements
1536-
// to process. If that's not the case, we have to do some legwork to
1537-
// determine if we should bail out.
1538-
guard comma == nil || self.at(.rightSquare, .endOfFile) else {
1539-
continue
1540-
}
1541-
1542-
// If we found EOF or the closing square bracket, bailout.
1543-
if self.at(.rightSquare, .endOfFile) {
1544-
break
1545-
}
1546-
1547-
// If the next token is at the beginning of a new line and can never start
1548-
// an element, break.
1549-
if self.atStartOfLine
1550-
&& (self.at(.rightBrace, .poundEndif)
1551-
|| self.atStartOfDeclaration()
1552-
|| self.atStartOfStatement(preferExpr: false))
1553-
{
1554-
break
1555-
}
1556-
}
1555+
} while keepGoing != nil && self.hasProgressed(&collectionProgress)
15571556
}
15581557

15591558
let (unexpectedBeforeRSquare, rsquare) = self.expect(.rightSquare)

‎Tests/SwiftParserTest/ExpressionTests.swift

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2742,4 +2742,65 @@ final class StatementExpressionTests: ParserTestCase {
27422742
"""
27432743
)
27442744
}
2745+
2746+
func testArrayExprWithNoCommas() {
2747+
assertParse("[() ()]")
2748+
2749+
assertParse(
2750+
"[1 1️⃣2]",
2751+
diagnostics: [
2752+
DiagnosticSpec(
2753+
message: "expected ',' in array element",
2754+
fixIts: ["insert ','"]
2755+
)
2756+
],
2757+
fixedSource: "[1, 2]"
2758+
)
2759+
2760+
assertParse(
2761+
#"["hello" 1️⃣"world"]"#,
2762+
diagnostics: [
2763+
DiagnosticSpec(
2764+
message: "expected ',' in array element",
2765+
fixIts: ["insert ','"]
2766+
)
2767+
],
2768+
fixedSource: #"["hello", "world"]"#
2769+
)
2770+
}
2771+
2772+
func testDictionaryExprWithNoCommas() {
2773+
assertParse(
2774+
"[1: () 1️⃣2: ()]",
2775+
diagnostics: [
2776+
DiagnosticSpec(
2777+
message: "expected ',' in dictionary element",
2778+
fixIts: ["insert ','"]
2779+
)
2780+
],
2781+
fixedSource: #"[1: (), 2: ()]"#
2782+
)
2783+
2784+
assertParse(
2785+
#"["foo": 1 1️⃣"bar": 2]"#,
2786+
diagnostics: [
2787+
DiagnosticSpec(
2788+
message: "expected ',' in dictionary element",
2789+
fixIts: ["insert ','"]
2790+
)
2791+
],
2792+
fixedSource: #"["foo": 1, "bar": 2]"#
2793+
)
2794+
2795+
assertParse(
2796+
#"[1: "hello" 1️⃣2: "world"]"#,
2797+
diagnostics: [
2798+
DiagnosticSpec(
2799+
message: "expected ',' in dictionary element",
2800+
fixIts: ["insert ','"]
2801+
)
2802+
],
2803+
fixedSource: #"[1: "hello", 2: "world"]"#
2804+
)
2805+
}
27452806
}

‎Tests/SwiftParserTest/translated/ObjectLiteralsTests.swift

Lines changed: 27 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -18,41 +18,44 @@ final class ObjectLiteralsTests: ParserTestCase {
1818
func testObjectLiterals1a() {
1919
assertParse(
2020
"""
21-
let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha)#1️⃣]
21+
let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha)1️⃣#2️⃣]
2222
""",
2323
diagnostics: [
24-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
24+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
25+
DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
2526
],
2627
fixedSource: """
27-
let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha)#<#identifier#>]
28+
let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha), #<#identifier#>]
2829
"""
2930
)
3031
}
3132

3233
func testObjectLiterals1b() {
3334
assertParse(
3435
"""
35-
let _ = [#Image(imageLiteral: localResourceNameAsString)#1️⃣]
36+
let _ = [#Image(imageLiteral: localResourceNameAsString)1️⃣#2️⃣]
3637
""",
3738
diagnostics: [
38-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
39+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
40+
DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
3941
],
4042
fixedSource: """
41-
let _ = [#Image(imageLiteral: localResourceNameAsString)#<#identifier#>]
43+
let _ = [#Image(imageLiteral: localResourceNameAsString), #<#identifier#>]
4244
"""
4345
)
4446
}
4547

4648
func testObjectLiterals1c() {
4749
assertParse(
4850
"""
49-
let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString)#1️⃣]
51+
let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString)1️⃣#2️⃣]
5052
""",
5153
diagnostics: [
52-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
54+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
55+
DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
5356
],
5457
fixedSource: """
55-
let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString)#<#identifier#>]
58+
let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString), #<#identifier#>]
5659
"""
5760
)
5861
}
@@ -112,22 +115,22 @@ final class ObjectLiteralsTests: ParserTestCase {
112115
""",
113116
diagnostics: [
114117
DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
118+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
115119
DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
116120
],
117121
fixedSource: """
118-
let _ = [#<#identifier#> #<#identifier#>]
122+
let _ = [#<#identifier#>, #<#identifier#>]
119123
"""
120124
)
121125
}
122126

123127
func testObjectLiterals5() {
124128
assertParse(
125129
"""
126-
let _ = ℹ️[#Color(_: 1, green: 1, 2)2️⃣
130+
let _ = ℹ️[#Color(_: 1, green: 1, 2)1️⃣
127131
""",
128132
diagnostics: [
129133
DiagnosticSpec(
130-
locationMarker: "2️⃣",
131134
message: "expected ']' to end array",
132135
notes: [NoteSpec(message: "to match this opening '['")],
133136
fixIts: ["insert ']'"]
@@ -142,37 +145,43 @@ final class ObjectLiteralsTests: ParserTestCase {
142145
func testObjectLiterals6() {
143146
assertParse(
144147
"""
145-
let _ = ℹ️[1️⃣#Color(red: 1, green: 1, blue: 1)#2️⃣3️⃣
148+
let _ = ℹ️[#Color(red: 1, green: 1, blue: 1)1️⃣#2️⃣
146149
""",
147150
diagnostics: [
151+
DiagnosticSpec(
152+
locationMarker: "1️⃣",
153+
message: "expected ',' in array element",
154+
fixIts: ["insert ','"]
155+
),
148156
DiagnosticSpec(
149157
locationMarker: "2️⃣",
150158
message: "expected identifier in macro expansion",
151159
fixIts: ["insert identifier"]
152160
),
153161
DiagnosticSpec(
154-
locationMarker: "3️⃣",
162+
locationMarker: "2️⃣",
155163
message: "expected ']' to end array",
156164
notes: [NoteSpec(message: "to match this opening '['")],
157165
fixIts: ["insert ']'"]
158166
),
159167
],
160168
fixedSource: """
161-
let _ = [#Color(red: 1, green: 1, blue: 1)#<#identifier#>]
169+
let _ = [#Color(red: 1, green: 1, blue: 1), #<#identifier#>]
162170
"""
163171
)
164172
}
165173

166174
func testObjectLiterals7() {
167175
assertParse(
168176
"""
169-
let _ = [#Color(withRed: 1, green: 1, whatever: 2)#1️⃣]
177+
let _ = [#Color(withRed: 1, green: 1, whatever: 2)1️⃣#2️⃣]
170178
""",
171179
diagnostics: [
172-
DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
180+
DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
181+
DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
173182
],
174183
fixedSource: """
175-
let _ = [#Color(withRed: 1, green: 1, whatever: 2)#<#identifier#>]
184+
let _ = [#Color(withRed: 1, green: 1, whatever: 2), #<#identifier#>]
176185
"""
177186
)
178187
}

‎Tests/SwiftParserTest/translated/RecoveryTests.swift

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2423,6 +2423,11 @@ final class RecoveryTests: ParserTestCase {
24232423
locationMarker: "4️⃣",
24242424
message: "unexpected code ') -> Int {}' in closure"
24252425
),
2426+
DiagnosticSpec(
2427+
locationMarker: "5️⃣",
2428+
message: "expected ',' in array element",
2429+
fixIts: ["insert ','"]
2430+
),
24262431
DiagnosticSpec(
24272432
locationMarker: "5️⃣",
24282433
message: "expected ']' to end array",
@@ -2439,7 +2444,7 @@ final class RecoveryTests: ParserTestCase {
24392444
struct Foo19605164 {
24402445
func a(s: S)
24412446
}[{{g) -> Int {}
2442-
}}]}
2447+
}},]}
24432448
#endif
24442449
"""
24452450
)

0 commit comments

Comments
 (0)