Add diagnostic for missing sequence expr in for-loop

Author: kimdv

Sources/SwiftParser/Statements.swift

@@ -616,7 +616,15 @@ extension Parser {
 
     let (unexpectedBeforeInKeyword, inKeyword) = self.expect(.keyword(.in))
 
-    let expr = self.parseExpression(.basic)
+    // If there is no expression, like `switch { default: return false }` then left brace would parsed as
+    // a `RawClosureExprSyntax` in the condition, which is most likely not what the user meant.
+    // Create a missing condition instead and use the `{` for the start of the body.
+    let expr: RawExprSyntax
+    if self.at(.leftBrace) {
+      expr = RawExprSyntax(RawMissingExprSyntax(arena: self.arena))
+    } else {
+      expr = self.parseExpression(.basic)
+    }
 
     // Parse the 'where' expression if present.
     let whereClause: RawWhereClauseSyntax?

Sources/SwiftParserDiagnostics/ParseDiagnosticsGenerator.swift

@@ -601,7 +601,12 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
         ] as [Syntax?]).compactMap({ $0 }),
         handledNodes: [node.inKeyword.id, node.sequenceExpr.id, unexpectedCondition.id]
       )
+    } else {  // If it's not a C-style for loop
+      if node.sequenceExpr.is(MissingExprSyntax.self) {
+        addDiagnostic(node.sequenceExpr, .expectedSequenceExpressionInForEachLoop, handledNodes: [node.sequenceExpr.id])
+      }
     }
+
     return .visitChildren
   }
 
@@ -1049,7 +1054,7 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     }
 
     if node.expression.is(MissingExprSyntax.self) && !node.cases.isEmpty {
-      addDiagnostic(node.expression, .missingExpressionInSwitchStatement, handledNodes: [node.expression.id])
+      addDiagnostic(node.expression, MissingExpressionInStatement(node: node), handledNodes: [node.expression.id])
     }
 
     return .visitChildren

Sources/SwiftParserDiagnostics/ParserDiagnosticMessages.swift

@@ -128,6 +128,9 @@ extension DiagnosticMessage where Self == StaticParserError {
   public static var expectedExpressionAfterTry: Self {
     .init("expected expression after 'try'")
   }
+  public static var expectedSequenceExpressionInForEachLoop: Self {
+    .init("expected Sequence expression for for-each loop")
+  }
   public static var initializerInPattern: Self {
     .init("unexpected initializer in pattern; did you mean to use '='?")
   }
@@ -164,9 +167,6 @@ extension DiagnosticMessage where Self == StaticParserError {
   public static var missingConformanceRequirement: Self {
     .init("expected ':' or '==' to indicate a conformance or same-type requirement")
   }
-  public static var missingExpressionInSwitchStatement: Self {
-    .init("expected expression in 'switch' statement")
-  }
   public static var misspelledAsync: Self {
     .init("expected async specifier; did you mean 'async'?")
   }
@@ -333,6 +333,18 @@ public struct MissingAttributeArgument: ParserError {
   }
 }
 
+public struct MissingExpressionInStatement: ParserError {
+  let node: SyntaxProtocol
+
+  public var message: String {
+    if let name = node.nodeTypeNameForDiagnostics(allowBlockNames: false) {
+      return "expected expression in \(name)"
+    } else {
+      return "expected expression in statement"
+    }
+  }
+}
+
 public struct NegatedAvailabilityCondition: ParserError {
   public let avaialabilityCondition: AvailabilityConditionSyntax
   public let negatedAvailabilityKeyword: TokenSyntax

Tests/SwiftParserTest/translated/RecoveryTests.swift

@@ -459,13 +459,10 @@ final class RecoveryTests: XCTestCase {
     assertParse(
       """
       for 1️⃣{
-      }2️⃣
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected pattern
-        // TODO: Old parser expected error on line 1: expected Sequence expression for for-each loop
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected pattern and 'in' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected code block in 'for' statement"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected pattern, 'in', and expression in 'for' statement")
       ]
     )
   }
@@ -475,12 +472,10 @@ final class RecoveryTests: XCTestCase {
       """
       for1️⃣
       {
-      }2️⃣
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected Sequence expression for for-each loop
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected pattern and 'in' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected code block in 'for' statement"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected pattern, 'in', and expression in 'for' statement")
       ]
     )
   }
@@ -489,12 +484,10 @@ final class RecoveryTests: XCTestCase {
     assertParse(
       """
       for i 1️⃣{
-      }2️⃣
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected Sequence expression for for-each loop
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'in' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected code block in 'for' statement"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'in' and expression in 'for' statement")
       ]
     )
   }
@@ -503,12 +496,10 @@ final class RecoveryTests: XCTestCase {
     assertParse(
       """
       for var i 1️⃣{
-      }2️⃣
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected Sequence expression for for-each loop
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'in' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected code block in 'for' statement"),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'in' and expression in 'for' statement")
       ]
     )
   }
@@ -517,14 +508,11 @@ final class RecoveryTests: XCTestCase {
     assertParse(
       """
       for 1️⃣in 2️⃣{
-      }3️⃣
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected pattern
-        // TODO: Old parser expected error on line 1: expected Sequence expression for for-each loop
         DiagnosticSpec(locationMarker: "1️⃣", message: "keyword 'in' cannot be used as an identifier here"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected 'in' in 'for' statement"),
-        DiagnosticSpec(locationMarker: "3️⃣", message: "expected code block in 'for' statement"),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "expected 'in' and expression in 'for' statement"),
       ]
     )
   }
@@ -544,25 +532,24 @@ final class RecoveryTests: XCTestCase {
   func testRecovery41() {
     assertParse(
       """
-      for 1️⃣for in {
-      }2️⃣
+      for 1️⃣for in 2️⃣{
+      }
       """,
       diagnostics: [
         DiagnosticSpec(locationMarker: "1️⃣", message: "keyword 'for' cannot be used as an identifier here"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected code block in 'for' statement"),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "expected Sequence expression for for-each loop"),
       ]
     )
   }
 
   func testRecovery42() {
     assertParse(
       """
-      for i in {
-      }1️⃣
+      for i in 1️⃣{
+      }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 1: expected Sequence expression for for-each loop
-        DiagnosticSpec(message: "expected code block in 'for' statement")
+        DiagnosticSpec(message: "expected Sequence expression for for-each loop")
       ]
     )
   }