Merge pull request #958 from ahoppen/ahoppen/colon-and-expr-missing-in-ternary

Improve diagnostic if colon and expression are missing in ternary expression

Author: ahoppen

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -633,7 +633,22 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
       return .skipChildren
     }
     if node.colonMark.presence == .missing {
-      addDiagnostic(node.colonMark, .missingColonInTernaryExprDiagnostic, handledNodes: [node.colonMark.id])
+      if let siblings = node.parent?.children(viewMode: .all),
+         let nextSibling = siblings[siblings.index(after: node.index)...].first,
+         nextSibling.is(MissingExprSyntax.self) {
+        addDiagnostic(node.colonMark, .missingColonAndExprInTernaryExpr, fixIts: [
+          FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.colonMark), Syntax(nextSibling)]), changes: [
+            .makePresent(node: node.colonMark),
+            .makePresent(node: nextSibling),
+          ])
+        ], handledNodes: [node.colonMark.id, nextSibling.id])
+      } else {
+        addDiagnostic(node.colonMark, .missingColonInTernaryExpr, fixIts: [
+          FixIt(message: InsertTokenFixIt(missingNodes: [Syntax(node.colonMark)]), changes: [
+            .makePresent(node: node.colonMark),
+          ])
+        ], handledNodes: [node.colonMark.id])
+      }
     }
     return .visitChildren
   }

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -76,7 +76,8 @@ public enum StaticParserError: String, DiagnosticMessage {
   case editorPlaceholderInSourceFile = "editor placeholder in source file"
   case expectedExpressionAfterTry = "expected expression after 'try'"
   case invalidFlagAfterPrecedenceGroupAssignment = "expected 'true' or 'false' after 'assignment'"
-  case missingColonInTernaryExprDiagnostic = "expected ':' after '? ...' in ternary expression"
+  case missingColonAndExprInTernaryExpr = "expected ':' and expression after '? ...' in ternary expression"
+  case missingColonInTernaryExpr = "expected ':' after '? ...' in ternary expression"
   case operatorShouldBeDeclaredWithoutBody = "operator should not be declared with body"
   case standaloneSemicolonStatement = "standalone ';' statements are not allowed"
   case subscriptsCannotHaveNames = "subscripts cannot have a name"

Tests/SwiftParserTest/Declarations.swift

@@ -189,8 +189,7 @@ final class DeclarationTests: XCTestCase {
     AssertParse(
       "_ = foo/* */?.description1️⃣",
       diagnostics: [
-        DiagnosticSpec(message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(message: "expected expression"),
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression"),
       ]
     )
 

Tests/SwiftParserTest/Expressions.swift

@@ -588,8 +588,7 @@ final class ExpressionTests: XCTestCase {
     AssertParse(
       "foo ? 11️⃣",
       diagnostics: [
-        DiagnosticSpec(message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(message: "expected expression"),
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression"),
       ]
     )
   }

Tests/SwiftParserTest/translated/InvalidIfExprTests.swift

@@ -6,30 +6,46 @@ final class InvalidIfExprTests: XCTestCase {
   func testInvalidIfExpr1() {
     AssertParse(
       """
-      func unbalanced_question(a: Bool, b: Bool, c: Bool, d: Bool) {
-        (a ? b1️⃣) 
-        (a ? b : c ? d2️⃣) 
-        (a ? b ? c : d3️⃣) 
-        (a ? b ? c4️⃣) 
-      }
+      (a ? b1️⃣)
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 2: expected ':' after '? ...' in ternary
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected expression in tuple"),
-        // TODO: Old parser expected error on line 3: expected ':' after '? ...' in ternary
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected expression in tuple"),
-        // TODO: Old parser expected error on line 4: expected ':' after '? ...' in ternary
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected expression in tuple"),
-        // TODO: Old parser expected error on line 5: expected ':' after '? ...' in ternary
-        // TODO: Old parser expected error on line 5: expected ':' after '? ...' in ternary
-        DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(locationMarker: "4️⃣", message: "expected expression in tuple"),
-        DiagnosticSpec(locationMarker: "4️⃣", message: "expected ':' after '? ...' in ternary expression"),
-        DiagnosticSpec(locationMarker: "4️⃣", message: "expected expression in tuple"),
-      ]
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression", fixIts: ["insert ':' and expression"]),
+      ], fixedSource: "(a ? b: <#expression#>)"
+    )
+  }
+
+  func testInvalidIfExpr2() {
+    AssertParse(
+      """
+      (a ? b : c ? d1️⃣)
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression", fixIts: ["insert ':' and expression"]),
+     ], fixedSource: "(a ? b : c ? d: <#expression#>)"
+    )
+  }
+
+  func testInvalidIfExpr3() {
+    AssertParse(
+      """
+      (a ? b ? c : d1️⃣
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression", fixIts: ["insert ':' and expression"]),
+        DiagnosticSpec(message: "expected ')' to end tuple")
+      ], fixedSource: "(a ? b ? c : d: <#expression#>)"
+    )
+  }
+
+  func testInvalidIfExpr4() {
+    AssertParse(
+      """
+      (a ? b ? c1️⃣)
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression", fixIts: ["insert ':' and expression"]),
+        DiagnosticSpec(message: "expected ':' and expression after '? ...' in ternary expression", fixIts: ["insert ':' and expression"]),
+      ], fixedSource: "(a ? b ? c: <#expression#>: <#expression#>)"
     )
   }