Add missing comma diagnostic if it's inside an array or dictionary

Author: kimdv

Sources/SwiftParser/Expressions.swift

@@ -1500,8 +1500,24 @@ extension Parser {
       COLLECTION_LOOP: while self.hasProgressed(&collectionProgress) {
         elementKind = self.parseCollectionElement(elementKind)
 
+        var hasElementKindError: Bool {
+          switch elementKind! {
+          case .dictionary(let key, _, let colon, let value):
+            return key.hasError || colon.hasError || value.hasError
+          case .array(let rawExprSyntax):
+            return rawExprSyntax.hasError
+          }
+        }
+
         // Parse the ',' if exists.
-        let comma = self.consume(if: .comma)
+        let comma: Token?
+        if let token = self.consume(if: .comma) {
+          comma = token
+        } else if !self.atStartOfLine, !self.at(.rightSquare, .endOfFile), !hasElementKindError {
+          comma = missingToken(.comma)
+        } else {
+          comma = nil
+        }
 
         switch elementKind! {
         case .array(let el):

Tests/SwiftParserTest/ExpressionTests.swift

@@ -2826,4 +2826,65 @@ final class StatementExpressionTests: ParserTestCase {
       experimentalFeatures: .typedThrows
     )
   }
+
+  func testArrayExprWithNoCommas() {
+    assertParse("[() ()]")
+
+    assertParse(
+      "[1 1️⃣2]",
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected ',' in array element",
+          fixIts: ["insert ','"]
+        )
+      ],
+      fixedSource: "[1, 2]"
+    )
+
+    assertParse(
+      #"["hello" 1️⃣"world"]"#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected ',' in array element",
+          fixIts: ["insert ','"]
+        )
+      ],
+      fixedSource: #"["hello", "world"]"#
+    )
+  }
+
+  func testDictionaryExprWithNoCommas() {
+    assertParse(
+      "[1: () 1️⃣2: ()]",
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected ',' in dictionary element",
+          fixIts: ["insert ','"]
+        )
+      ],
+      fixedSource: #"[1: (), 2: ()]"#
+    )
+
+    assertParse(
+      #"["foo": 1 1️⃣"bar": 2]"#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected ',' in dictionary element",
+          fixIts: ["insert ','"]
+        )
+      ],
+      fixedSource: #"["foo": 1, "bar": 2]"#
+    )
+
+    assertParse(
+      #"[1: "hello" 1️⃣2: "world"]"#,
+      diagnostics: [
+        DiagnosticSpec(
+          message: "expected ',' in dictionary element",
+          fixIts: ["insert ','"]
+        )
+      ],
+      fixedSource: #"[1: "hello", 2: "world"]"#
+    )
+  }
 }

Tests/SwiftParserTest/translated/ObjectLiteralsTests.swift

@@ -18,41 +18,44 @@ final class ObjectLiteralsTests: ParserTestCase {
   func testObjectLiterals1a() {
     assertParse(
       """
-      let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha)#1️⃣]
+      let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha)1️⃣#2️⃣]
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
       ],
       fixedSource: """
-        let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha)#<#identifier#>]
+        let _ = [#Color(colorLiteralRed: red, green: green, blue: blue, alpha: alpha), #<#identifier#>]
         """
     )
   }
 
   func testObjectLiterals1b() {
     assertParse(
       """
-      let _ = [#Image(imageLiteral: localResourceNameAsString)#1️⃣]
+      let _ = [#Image(imageLiteral: localResourceNameAsString)1️⃣#2️⃣]
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
       ],
       fixedSource: """
-        let _ = [#Image(imageLiteral: localResourceNameAsString)#<#identifier#>]
+        let _ = [#Image(imageLiteral: localResourceNameAsString), #<#identifier#>]
         """
     )
   }
 
   func testObjectLiterals1c() {
     assertParse(
       """
-      let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString)#1️⃣]
+      let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString)1️⃣#2️⃣]
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
       ],
       fixedSource: """
-        let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString)#<#identifier#>]
+        let _ = [#FileReference(fileReferenceLiteral: localResourceNameAsString), #<#identifier#>]
         """
     )
   }
@@ -123,11 +126,10 @@ final class ObjectLiteralsTests: ParserTestCase {
   func testObjectLiterals5() {
     assertParse(
       """
-      let _ = ℹ️[#Color(_: 1, green: 1, 2)2️⃣
+      let _ = ℹ️[#Color(_: 1, green: 1, 2)1️⃣
       """,
       diagnostics: [
         DiagnosticSpec(
-          locationMarker: "2️⃣",
           message: "expected ']' to end array",
           notes: [NoteSpec(message: "to match this opening '['")],
           fixIts: ["insert ']'"]
@@ -142,37 +144,43 @@ final class ObjectLiteralsTests: ParserTestCase {
   func testObjectLiterals6() {
     assertParse(
       """
-      let _ = ℹ️[1️⃣#Color(red: 1, green: 1, blue: 1)#2️⃣3️⃣
+      let _ = ℹ️[#Color(red: 1, green: 1, blue: 1)1️⃣#2️⃣
       """,
       diagnostics: [
+        DiagnosticSpec(
+          locationMarker: "1️⃣",
+          message: "expected ',' in array element",
+          fixIts: ["insert ','"]
+        ),
         DiagnosticSpec(
           locationMarker: "2️⃣",
           message: "expected identifier in macro expansion",
           fixIts: ["insert identifier"]
         ),
         DiagnosticSpec(
-          locationMarker: "3️⃣",
+          locationMarker: "2️⃣",
           message: "expected ']' to end array",
           notes: [NoteSpec(message: "to match this opening '['")],
           fixIts: ["insert ']'"]
         ),
       ],
       fixedSource: """
-        let _ = [#Color(red: 1, green: 1, blue: 1)#<#identifier#>]
+        let _ = [#Color(red: 1, green: 1, blue: 1), #<#identifier#>]
         """
     )
   }
 
   func testObjectLiterals7() {
     assertParse(
       """
-      let _ = [#Color(withRed: 1, green: 1, whatever: 2)#1️⃣]
+      let _ = [#Color(withRed: 1, green: 1, whatever: 2)1️⃣#2️⃣]
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"])
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected ',' in array element", fixIts: ["insert ','"]),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "expected identifier in macro expansion", fixIts: ["insert identifier"]),
       ],
       fixedSource: """
-        let _ = [#Color(withRed: 1, green: 1, whatever: 2)#<#identifier#>]
+        let _ = [#Color(withRed: 1, green: 1, whatever: 2), #<#identifier#>]
         """
     )
   }