Improve recovery in #if blocks

Author: ahoppen

Sources/SwiftParser/Declarations.swift

@@ -48,7 +48,7 @@ extension TokenConsumer {
     if allowRecovery {
       declStartKeyword = subparser.canRecoverTo(
         anyIn: DeclarationStart.self,
-        recoveryPrecedence: isAtTopLevel ? nil : .strongBracketedClose
+        recoveryPrecedence: isAtTopLevel ? nil : .closingBrace
       )?.0
     } else {
       declStartKeyword = subparser.at(anyIn: DeclarationStart.self)?.0

Sources/SwiftParser/Directives.swift

@@ -62,26 +62,24 @@ extension Parser {
   ///   - syntax: A function that aggregates the parsed conditional elements
   ///             into a syntax collection.
   @_spi(RawSyntax)
-  public mutating func parsePoundIfDirective<Element>(
+  public mutating func parsePoundIfDirective<Element: RawSyntaxNodeProtocol>(
     _ parseElement: (inout Parser) -> Element?,
     syntax: (inout Parser, [Element]) -> RawSyntax
   ) -> RawIfConfigDeclSyntax {
     var clauses = [RawIfConfigClauseSyntax]()
     do {
       var firstIteration = true
       var loopProgress = LoopProgressCondition()
-      while let poundIf = self.consume(ifAny: firstIteration ? [.poundIfKeyword] : [.poundIfKeyword, .poundElseifKeyword, .poundElseKeyword]),
+      while let poundIfHandle = self.canRecoverTo(any: firstIteration ? [.poundIfKeyword] : [.poundIfKeyword, .poundElseifKeyword, .poundElseKeyword]),
             loopProgress.evaluate(self.currentToken) {
+        let (unexpectedBeforePoundIf, poundIf) = self.eat(poundIfHandle)
         firstIteration = false
         // Parse the condition.
-        let unexpectedBeforePoundIf: RawUnexpectedNodesSyntax?
         let condition: RawExprSyntax?
         switch poundIf.tokenKind {
         case .poundIfKeyword, .poundElseifKeyword:
-          unexpectedBeforePoundIf = nil
           condition = RawExprSyntax(self.parseSequenceExpression(.basic, forDirective: true))
         case .poundElseKeyword:
-          unexpectedBeforePoundIf = nil
           condition = nil
         default:
           preconditionFailure("The loop condition should guarantee that we are at one of these tokens")
@@ -90,9 +88,8 @@ extension Parser {
         var elements = [Element]()
         do {
           var elementsProgress = LoopProgressCondition()
-          while !self.at(any: [.eof, .poundElseKeyword, .poundElseifKeyword, .poundEndifKeyword])
-                  && elementsProgress.evaluate(currentToken) {
-            guard let element = parseElement(&self) else {
+          while !self.at(any: [.eof, .poundElseKeyword, .poundElseifKeyword, .poundEndifKeyword]) && elementsProgress.evaluate(currentToken) {
+            guard let element = parseElement(&self), element.raw.byteLength > 0 else {
               break
             }
             elements.append(element)

Sources/SwiftParser/Parser.swift

@@ -275,13 +275,19 @@ extension Parser {
   }
 
   /// Eat a token that we know we are currently positioned at, based on `canRecoverTo(anyIn:)`.
-  mutating func eat(_ handle: RecoveryConsumptionHandle) -> (RawUnexpectedNodesSyntax, Token) {
-    var unexpectedTokens = [RawSyntax]()
-    for _ in 0..<handle.unexpectedTokens {
-      unexpectedTokens.append(RawSyntax(self.consumeAnyToken()))
+  mutating func eat(_ handle: RecoveryConsumptionHandle) -> (RawUnexpectedNodesSyntax?, Token) {
+    let unexpectedNodes: RawUnexpectedNodesSyntax?
+    if handle.unexpectedTokens > 0 {
+      var unexpectedTokens = [RawSyntax]()
+      for _ in 0..<handle.unexpectedTokens {
+        unexpectedTokens.append(RawSyntax(self.consumeAnyToken()))
+      }
+      unexpectedNodes = RawUnexpectedNodesSyntax(elements: unexpectedTokens, arena: self.arena)
+    } else {
+      unexpectedNodes = nil
     }
     let token = self.eat(handle.tokenConsumptionHandle)
-    return (RawUnexpectedNodesSyntax(elements: unexpectedTokens, arena: self.arena), token)
+    return (unexpectedNodes, token)
   }
 }
 

Sources/SwiftParser/RawTokenKindSubset.swift

@@ -499,6 +499,18 @@ enum PoundDeclarationStart: RawTokenKindSubset {
   }
 }
 
+enum SwitchCaseStart: RawTokenKindSubset {
+  case caseKeyword
+  case defaultKeyword
+
+  var rawTokenKind: RawTokenKind {
+    switch self {
+    case .caseKeyword: return .caseKeyword
+    case .defaultKeyword: return .defaultKeyword
+    }
+  }
+}
+
 // MARK: Expression start
 
 enum AwaitTry: RawTokenKindSubset {

Sources/SwiftParser/Statements.swift

@@ -631,21 +631,20 @@ extension Parser {
     var elementsProgress = LoopProgressCondition()
     while !self.at(any: [.eof, .rightBrace, .poundEndifKeyword, .poundElseifKeyword, .poundElseKeyword])
             && elementsProgress.evaluate(currentToken) {
-      if self.lookahead().isAtStartOfSwitchCase() {
+      if self.lookahead().isAtStartOfSwitchCase(allowRecovery: true) {
         elements.append(RawSyntax(self.parseSwitchCase()))
-      } else if self.at(.poundIfKeyword) {
+      } else if self.canRecoverTo(.poundIfKeyword) != nil {
         // '#if' in 'case' position can enclose zero or more 'case' or 'default'
         // clauses.
-        elements.append(RawSyntax(self.parsePoundIfDirective {
-          $0.parseSwitchCases()
-        }
-        syntax: { parser, cases in
-          guard cases.count == 1, let firstCase = cases.first else {
-            assert(cases.isEmpty)
-            return RawSyntax(RawSwitchCaseListSyntax(elements: [], arena: parser.arena))
-          }
-          return RawSyntax(firstCase)
-        }))
+        elements.append(RawSyntax(self.parsePoundIfDirective(
+          { $0.parseSwitchCases() },
+          syntax: { parser, cases in
+            guard cases.count == 1, let firstCase = cases.first else {
+              assert(cases.isEmpty)
+              return RawSyntax(RawSwitchCaseListSyntax(elements: [], arena: parser.arena))
+            }
+            return RawSyntax(firstCase)
+          })))
       } else {
         break
       }
@@ -688,9 +687,10 @@ extension Parser {
     }
 
     let label: RawSyntax
-    if self.at(.caseKeyword) {
+    switch self.canRecoverTo(anyIn: SwitchCaseStart.self) {
+    case (.caseKeyword, _)?:
       label = RawSyntax(self.parseSwitchCaseLabel())
-    } else {
+    case (.defaultKeyword, _)?, nil:
       label = RawSyntax(self.parseSwitchDefaultLabel())
     }
 
@@ -1039,7 +1039,7 @@ extension Parser.Lookahead {
 
   /// Returns whether the parser's current position is the start of a switch case,
   /// given that we're in the middle of a switch already.
-  func isAtStartOfSwitchCase() -> Bool {
+  func isAtStartOfSwitchCase(allowRecovery: Bool = false) -> Bool {
     // Check for and consume attributes. The only valid attribute is `@unknown`
     // but that's a semantic restriction.
     var lookahead = self.lookahead()
@@ -1053,7 +1053,11 @@ extension Parser.Lookahead {
       lookahead.expectIdentifierWithoutRecovery()
     }
 
-    return lookahead.at(any: [.caseKeyword, .defaultKeyword])
+    if allowRecovery {
+      return lookahead.canRecoverTo(anyIn: SwitchCaseStart.self) != nil
+    } else {
+      return lookahead.at(anyIn: SwitchCaseStart.self) != nil
+    }
   }
 
   func isStartOfConditionalSwitchCases() -> Bool {

Sources/SwiftParser/TokenPrecedence.swift

@@ -31,21 +31,25 @@ public enum TokenPrecedence: Comparable {
   case stmtKeyword
   /// The '{' token because it typically marks the body of a declaration.
   /// `closingDelimiter` must have type `strongPunctuator`
-  case strongBracketed(closingDelimiter: RawTokenKind)
+  case openingBrace(closingDelimiter: RawTokenKind)
   /// A punctuator that is a strong indicator that it separates two distinct parts of the source code, like two statements
   case strongPunctuator
   /// The closing delimiter of `strongBracketed`
-  case strongBracketedClose
+  case closingBrace
   /// Tokens that start a new declaration
   case declKeyword
+  case openingPoundIf
+  case closingPoundIf
 
   /// If the precedence is `weakBracketed` or `strongBracketed`, the closing delimeter of the bracketed group.
   var closingTokenKind: RawTokenKind? {
     switch self {
     case .weakBracketed(closingDelimiter: let closingDelimiter):
       return closingDelimiter
-    case .strongBracketed(closingDelimiter: let closingDelimiter):
+    case .openingBrace(closingDelimiter: let closingDelimiter):
       return closingDelimiter
+    case .openingPoundIf:
+      return .poundEndifKeyword
     default:
       return nil
     }
@@ -69,12 +73,16 @@ public enum TokenPrecedence: Comparable {
         return 5
       case .strongPunctuator:
         return 6
-      case .strongBracketed:
+      case .openingBrace:
         return 7
-      case .strongBracketedClose:
+      case .closingBrace:
         return 8
       case .declKeyword:
         return 9
+      case .openingPoundIf:
+        return 10
+      case .closingPoundIf:
+        return 11
       }
     }
 
@@ -162,9 +170,9 @@ public enum TokenPrecedence: Comparable {
 
       // MARK: Strong bracketet
     case .leftBrace:
-      self = .strongBracketed(closingDelimiter: .rightBrace)
+      self = .openingBrace(closingDelimiter: .rightBrace)
     case .poundElseifKeyword, .poundElseKeyword, .poundIfKeyword:
-      self = .strongBracketed(closingDelimiter: .poundEndifKeyword)
+      self = .openingPoundIf
 
       // MARK: Strong punctuator
     case
@@ -179,8 +187,10 @@ public enum TokenPrecedence: Comparable {
       self = .strongPunctuator
 
       // MARK: Strong bracket close
-    case .poundEndifKeyword, .rightBrace:
-      self = .strongBracketedClose
+    case .rightBrace:
+      self = .closingBrace
+    case  .poundEndifKeyword:
+      self = .closingPoundIf
 
       // MARK: Decl keywords
     case

Tests/SwiftParserTest/Directives.swift

@@ -84,17 +84,16 @@ final class DirectiveTests: XCTestCase {
     AssertParse(
       """
       #if os(iOS)
-        func foo() {}#^DIAG_1^#
-      #^DIAG_2^#}
+        func foo() {}
+      #^DIAG_1^#}
       #else
-        func bar() {}
         func baz() {}
-      } // expected-error{{unexpected '}' in conditional compilation block}}
+      #^DIAG_2^#}
       #endif
       """,
       diagnostics: [
-        DiagnosticSpec(locationMarker: "DIAG_1", message: "Expected '#endif' in conditional compilation block"),
-        DiagnosticSpec(locationMarker: "DIAG_2", message: "Extraneous code at top level"),
+        DiagnosticSpec(locationMarker: "DIAG_1", message: "Unexpected text '}' found in conditional compilation clause"),
+        DiagnosticSpec(locationMarker: "DIAG_2", message: "Unexpected text '}' found in conditional compilation block"),
       ]
     )
   }

Tests/SwiftParserTest/Statements.swift

@@ -184,17 +184,17 @@ final class StatementTests: XCTestCase {
     AssertParse(
       """
       switch x {
-        #^DIAG^#print()
+        #^FOO^#foo()
       #if true
-        print()
+        #^BAR^#bar()
       #endif
         case .A, .B:
           break
       }
       """,
       diagnostics: [
-        DiagnosticSpec(message: "Unexpected text found in 'switch' statement"),
-
+        DiagnosticSpec(locationMarker: "FOO", message: "Unexpected text 'foo()' found in conditional compilation clause"),
+        DiagnosticSpec(locationMarker: "BAR", message: "Unexpected text 'bar()' found in conditional compilation block"),
       ]
     )
 
@@ -213,7 +213,7 @@ final class StatementTests: XCTestCase {
       }
       """,
       diagnostics: [
-        DiagnosticSpec(message: "Unexpected text found in 'switch' statement")
+        DiagnosticSpec(message: "Unexpected text 'print()' found in conditional compilation clause")
       ]
     )
   }