Merge pull request #1429 from kimdv/kimdv/add-missing-switch-expression-error

Author: kimdv

Sources/SwiftParser/Expressions.swift

@@ -2174,9 +2174,24 @@ extension Parser {
     ifHandle: RecoveryConsumptionHandle
   ) -> RawIfExprSyntax {
     let (unexpectedBeforeIfKeyword, ifKeyword) = self.eat(ifHandle)
-    // A scope encloses the condition and true branch for any variables bound
-    // by a conditional binding. The else branch does *not* see these variables.
-    let conditions = self.parseConditionList()
+
+    let conditions: RawConditionElementListSyntax
+
+    if self.at(.leftBrace) {
+      conditions = RawConditionElementListSyntax(
+        elements: [
+          RawConditionElementSyntax(
+            condition: .expression(RawExprSyntax(RawMissingExprSyntax(arena: self.arena))),
+            trailingComma: nil,
+            arena: self.arena
+          )
+        ],
+        arena: self.arena
+      )
+    } else {
+      conditions = self.parseConditionList()
+    }
+
     let body = self.parseCodeBlock(introducer: ifKeyword)
 
     // The else branch, if any, is outside of the scope of the condition.
@@ -2222,7 +2237,16 @@ extension Parser {
   ) -> RawSwitchExprSyntax {
     let (unexpectedBeforeSwitchKeyword, switchKeyword) = self.eat(switchHandle)
 
-    let subject = 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 subject: RawExprSyntax
+    if self.at(.leftBrace) {
+      subject = RawExprSyntax(RawMissingExprSyntax(arena: self.arena))
+    } else {
+      subject = self.parseExpression(.basic)
+    }
+
     let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
 
     let cases = self.parseSwitchCases(allowStandaloneStmtRecovery: !lbrace.isMissing)

Sources/SwiftParser/Statements.swift

@@ -539,7 +539,22 @@ extension Parser {
   @_spi(RawSyntax)
   public mutating func parseWhileStatement(whileHandle: RecoveryConsumptionHandle) -> RawWhileStmtSyntax {
     let (unexpectedBeforeWhileKeyword, whileKeyword) = self.eat(whileHandle)
-    let conditions = self.parseConditionList()
+    let conditions: RawConditionElementListSyntax
+
+    if self.at(.leftBrace) {
+      conditions = RawConditionElementListSyntax(
+        elements: [
+          RawConditionElementSyntax(
+            condition: .expression(RawExprSyntax(RawMissingExprSyntax(arena: self.arena))),
+            trailingComma: nil,
+            arena: self.arena
+          )
+        ],
+        arena: self.arena
+      )
+    } else {
+      conditions = self.parseConditionList()
+    }
     let body = self.parseCodeBlock(introducer: whileKeyword)
     return RawWhileStmtSyntax(
       unexpectedBeforeWhileKeyword,
@@ -616,7 +631,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
   }
 
@@ -750,6 +755,18 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: IfExprSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if node.conditions.count == 1, node.conditions.first?.as(ConditionElementSyntax.self)?.condition.is(MissingExprSyntax.self) == true, !node.body.leftBrace.isMissingAllTokens {
+      addDiagnostic(node.conditions, MissingConditionInStatement(node: node), handledNodes: [node.conditions.id])
+    }
+
+    return .visitChildren
+  }
+
   public override func visit(_ node: InitializerClauseSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
@@ -1043,6 +1060,18 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: SwitchExprSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if node.expression.is(MissingExprSyntax.self) && !node.cases.isEmpty {
+      addDiagnostic(node.expression, MissingExpressionInStatement(node: node), handledNodes: [node.expression.id])
+    }
+
+    return .visitChildren
+  }
+
   public override func visit(_ node: SwitchCaseSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren
@@ -1191,6 +1220,18 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: WhileStmtSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+
+    if node.conditions.count == 1, node.conditions.first?.as(ConditionElementSyntax.self)?.condition.is(MissingExprSyntax.self) == true, !node.body.leftBrace.isMissingAllTokens {
+      addDiagnostic(node.conditions, MissingConditionInStatement(node: node), handledNodes: [node.conditions.id])
+    }
+
+    return .visitChildren
+  }
+
   //==========================================================================//
   // IMPORTANT: If you are tempted to add a `visit` method here, please       //
   // insert it in alphabetical order above                                    //

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 '='?")
   }
@@ -330,6 +333,30 @@ public struct MissingAttributeArgument: ParserError {
   }
 }
 
+public struct MissingConditionInStatement: ParserError {
+  let node: SyntaxProtocol
+
+  public var message: String {
+    if let name = node.nodeTypeNameForDiagnostics(allowBlockNames: false) {
+      return "missing condition in \(name)"
+    } else {
+      return "missing condition in statement"
+    }
+  }
+}
+
+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