Remove old recover method

Recovery should be handled as unexpected nodes, which are usually produced by `expect`.

Author: ahoppen

Sources/SwiftParser/Attributes.swift

@@ -340,7 +340,7 @@ extension Parser {
     guard self.at(.leftParen) else {
       // If no opening '(' for parameter list, parse a single parameter.
       let param = self.parseDifferentiabilityParameter().map(RawSyntax.init(_:))
-                  ?? RawSyntax(RawTokenListSyntax(elements: self.recover(), arena: self.arena))
+                  ?? RawSyntax(RawMissingSyntax(arena: self.arena))
       return RawDifferentiabilityParamsClauseSyntax(
         wrtLabel: wrt,
         unexpectedBeforeColon,

Sources/SwiftParser/Declarations.swift

@@ -52,20 +52,6 @@ extension Parser {
     if self.at(.poundIfKeyword) {
       return RawDeclSyntax(self.parsePoundIfDirective { parser in
         var parsedDecl = parser.parseDeclaration()
-        if parsedDecl.is(RawMissingDeclSyntax.self) {
-          // Try to recover from a bogus decl.
-          var tokenList = [RawTokenSyntax]()
-          while !parser.at(.eof) && !parser.at(.poundElseKeyword) &&
-                  !parser.at(.poundElseifKeyword) && !parser.at(.poundEndifKeyword) {
-            let tokens = parser.recover()
-            guard !tokens.isEmpty else {
-              break
-            }
-            tokenList.append(contentsOf: tokens)
-          }
-          let unexpected = RawUnexpectedNodesSyntax(elements: tokenList.map(RawSyntax.init), arena: parser.arena)
-          parsedDecl = RawDeclSyntax(RawMissingDeclSyntax(unexpected, attributes: nil, modifiers: nil, arena: parser.arena))
-        }
         let semicolon = parser.consume(if: .semicolon)
         return RawMemberDeclListItemSyntax(
           decl: parsedDecl,
@@ -1504,22 +1490,6 @@ extension Parser {
           // There can only be an implicit getter if no other accessors were
           // seen before this one.
           guard elements.isEmpty else {
-            // Recover until the matching right brace. It's a little
-            // presumptuous of us to assume everything between here and there
-            // is an accessor, but we cannot stick unexpected anywhere for the
-            // moment...
-            while !self.at(.eof) && !self.at(.rightBrace) {
-              for token in self.recover() {
-                elements.append(RawAccessorDeclSyntax(
-                  attributes: nil, modifier: nil,
-                  accessorKind: token,
-                  parameter: nil,
-                  asyncKeyword: nil, throwsKeyword: nil,
-                  body: nil,
-                  arena: self.arena))
-              }
-            }
-
             let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
             return RawSyntax(RawAccessorBlockSyntax(
               leftBrace: lbrace,
@@ -1723,7 +1693,7 @@ extension Parser {
     let (unexpectedBeforeLBrace, lbrace) = self.expect(.leftBrace)
     var elements = [RawSyntax]()
     do {
-      while !self.at(.eof) && !self.at(.rightBrace) {
+      LOOP: while !self.at(.eof) && !self.at(.rightBrace) {
         switch self.currentToken.tokenText {
         case "associativity":
           let associativity = self.consumeIdentifier()
@@ -1776,15 +1746,7 @@ extension Parser {
             otherNames: RawPrecedenceGroupNameListSyntax(elements: names, arena: self.arena),
             arena: self.arena)))
         default:
-          var tokenList = [RawTokenSyntax]()
-          while !self.at(.eof) && !self.at(.rightBrace) {
-            let tokens = self.recover()
-            guard !tokens.isEmpty else {
-              break
-            }
-            tokenList.append(contentsOf: tokens)
-          }
-          elements.append(RawSyntax(RawTokenListSyntax(elements: tokenList, arena: self.arena)))
+          break LOOP
         }
       }
     }

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -15,16 +15,28 @@ import SwiftSyntax
 
 let diagnosticDomain: String = "SwiftParser"
 
-extension Syntax {
+extension SyntaxProtocol {
   var nodeTypeNameForDiagnostics: String? {
-    if let name = self.as(SyntaxEnum.self).nameForDiagnostics {
+    if let name = Syntax(self).as(SyntaxEnum.self).nameForDiagnostics {
       return name
     }
     if let parent = self.parent {
       return parent.nodeTypeNameForDiagnostics
     }
     return nil
   }
+
+  /// If the syntax node (excluding leading and trailing trivia) only spans a
+  /// single line and has less than 100 characters (and thus fits into a
+  /// diagnostic message), return that. Otherwise, return `nil`.
+  var contentForDiagnosticsIfShortSingleLine: String? {
+    let contentWithoutTrivia = self.withoutLeadingTrivia().withoutTrailingTrivia().description
+    if contentWithoutTrivia.contains("\n") || contentWithoutTrivia.count > 100 {
+      return nil
+    } else {
+      return contentWithoutTrivia
+    }
+  }
 }
 
 /// A error diagnostic whose ID is determined by the diagnostic's type.
@@ -94,13 +106,10 @@ public struct ExtaneousCodeAtTopLevel: ParserError {
   public let extraneousCode: UnexpectedNodesSyntax
 
   public var message: String {
-    let extraneousCodeStr = extraneousCode.withoutLeadingTrivia().withoutTrailingTrivia().description
-    // If the extraneous code is multi-line or long (100 is in arbitrarily chosen value),
-    // it just spams the diagnostic. Just show a generic diagnostic in this case.
-    if extraneousCodeStr.contains("\n") || extraneousCodeStr.count > 100 {
-      return "Extraneous code at top level"
+    if let shortContent = extraneousCode.contentForDiagnosticsIfShortSingleLine {
+      return "Extraneous '\(shortContent)' at top level"
     } else {
-      return "Extraneous '\(extraneousCodeStr)' at top level"
+      return "Extraneous code at top level"
     }
   }
 }
@@ -132,10 +141,17 @@ public struct UnexpectedNodesError: ParserError {
   public let unexpectedNodes: UnexpectedNodesSyntax
 
   public var message: String {
-    if let parentTypeName = unexpectedNodes.parent?.nodeTypeNameForDiagnostics {
-      return "Unexpected text '\(unexpectedNodes.description)' found in \(parentTypeName)"
-    } else {
-      return "Unexpected text '\(unexpectedNodes.description)'"
+    let parentTypeName = unexpectedNodes.parent?.nodeTypeNameForDiagnostics
+    let shortContent = unexpectedNodes.contentForDiagnosticsIfShortSingleLine
+    switch (parentTypeName, shortContent) {
+    case (let parentTypeName?, let shortContent?):
+      return "Unexpected text '\(shortContent)' found in \(parentTypeName)"
+    case (let parentTypeName?, nil):
+      return "Unexpected text found in \(parentTypeName)"
+    case (nil, let shortContent?):
+      return "Unexpected text '\(shortContent)'"
+    case (nil, nil):
+      return "Unexpected text"
     }
   }
 }

Sources/SwiftParser/Expressions.swift

@@ -1718,7 +1718,7 @@ extension Parser {
   }
 
   @_spi(RawSyntax)
-  public mutating func parseClosureCaptureSpecifiers() -> RawTokenListSyntax {
+  public mutating func parseClosureCaptureSpecifiers() -> RawTokenListSyntax? {
     var specifiers = [RawTokenSyntax]()
     do {
       // Check for the strength specifier: "weak", "unowned", or
@@ -1738,26 +1738,14 @@ extension Parser {
         guard next.tokenKind == .equal || next.tokenKind == .comma
             || next.tokenKind == .rightSquareBracket || next.tokenKind == .period
         else {
-          // Recover from unexpected in the capture specifiers.
-          //
-          // FIXME: This is quite poor modeling in SwiftSyntax.
-          specifiers.append(contentsOf: self.recover())
-          return RawTokenListSyntax(elements: specifiers, arena: self.arena)
+          return nil
         }
       } else {
-        // Recover from unexpected in the capture specifiers.
-        //
-        // FIXME: This is quite poor modeling in SwiftSyntax.
-        specifiers.append(contentsOf: self.recover())
-        return RawTokenListSyntax(elements: specifiers, arena: self.arena)
+        return nil
       }
 
       guard self.currentToken.isIdentifier || self.at(.selfKeyword) else {
-        // Recover from unexpected in the capture specifiers.
-        //
-        // FIXME: This is quite poor modeling in SwiftSyntax.
-        specifiers.append(contentsOf: self.recover())
-        return RawTokenListSyntax(elements: specifiers, arena: self.arena)
+        return nil
       }
     }
     // Squash all tokens, if any, as the specifier of the captured item.

Sources/SwiftParser/Recovery.swift

@@ -12,101 +12,6 @@
 
 @_spi(RawSyntax) import SwiftSyntax
 
-extension Parser {
-  /// Implements general-purpose, balanced delimiter parser recovery.
-  ///
-  /// This function implements a very coarse recovery algorithm that tries to
-  /// skip over as much balanced token structure as it can before yielding back
-  /// to the parser. For example, this enables the parser to recover from
-  ///
-  /// ```swift
-  /// case: { ("Hello World") }
-  /// ```
-  ///
-  /// Item parsing runs recovery in a loop until it encounters a
-  /// token that could be the start of a new item. After eating the erroneous
-  /// `case` at the top level, the parser calls `recover()` to eat the colon,
-  /// then calls it again to eat the braces. The final time around, this
-  /// routine will eat not just the braces, but also the parenthesized
-  /// `"Hello World"` string token contained therein and its paren tokens.
-  ///
-  /// Using Recovery
-  /// ==============
-  ///
-  /// Parser recovery is a method of last resort. It indicates that the parser
-  /// has encountered some part of the input that it believes to be so erroneous
-  /// it cannot possibly assign it any structure. The resulting syntax tree
-  /// thus contains a sequence of unparsed tokens. As such, this function should
-  /// be considered only when the parser would otherwise be unable to make
-  /// forward progress, such as when an entire missing syntax node is
-  /// encountered. For simple cases like a missing token, it is more appropriate
-  /// to use ``Parser/expect(_:)``, which will correctly handle looking ahead
-  /// to try to reach the desired point in the token stream.
-  ///
-  /// - Returns: A non-empty list of tokens that were eaten during the recovery process.
-  mutating func recover() -> [RawTokenSyntax] {
-    var tokens = [RawTokenSyntax]()
-    switch self.currentToken.tokenKind {
-    case .leftParen:
-      tokens.append(self.consumeAnyToken())
-      while !self.at(.eof) && !self.at(.rightParen)
-              && !self.at(.poundEndifKeyword) && !self.at(.poundElseKeyword) && !self.at(.poundElseifKeyword) {
-        tokens.append(contentsOf: self.recover())
-      }
-      if let token = self.consume(if: .rightParen) {
-        tokens.append(token)
-      }
-      return tokens
-    case .leftBrace:
-      tokens.append(self.consumeAnyToken())
-      while !self.at(.eof) && !self.at(.rightBrace)
-              && !self.at(.poundEndifKeyword) && !self.at(.poundElseKeyword) && !self.at(.poundElseifKeyword) {
-        tokens.append(contentsOf: self.recover())
-      }
-      if let token = self.consume(if: .rightBrace) {
-        tokens.append(token)
-      }
-      return tokens
-    case .leftSquareBracket:
-      tokens.append(self.consumeAnyToken())
-      while !self.at(.eof) && !self.at(.rightSquareBracket)
-              && !self.at(.poundEndifKeyword) && !self.at(.poundElseKeyword) && !self.at(.poundElseifKeyword) {
-        tokens.append(contentsOf: self.recover())
-      }
-      if let token = self.consume(if: .rightSquareBracket) {
-        tokens.append(token)
-      }
-      return tokens
-    case .poundIfKeyword,
-        .poundElseKeyword,
-        .poundElseifKeyword:
-      tokens.append(self.consumeAnyToken())
-      // skipUntil also implicitly stops at tok::pound_endif.
-      while !self.at(.eof) && !self.at(.poundElseKeyword) && !self.at(.poundElseifKeyword)
-              && !self.at(.poundEndifKeyword) {
-        tokens.append(contentsOf: self.recover())
-      }
-      if let token = self.consume(if: .rightSquareBracket) {
-        tokens.append(token)
-      }
-
-      if self.at(.poundElseKeyword) || self.at(.poundElseifKeyword) {
-        tokens.append(contentsOf: self.recover())
-        return tokens
-      } else {
-        if let token = self.consume(if: .poundEndifKeyword) {
-          tokens.append(token)
-        }
-        return tokens
-      }
-
-    default:
-      tokens.append(self.consumeAnyToken())
-      return tokens
-    }
-  }
-}
-
 // MARK: Lookahead
 
 extension Parser.Lookahead {

Sources/SwiftParser/Statements.swift

@@ -618,18 +618,7 @@ extension Parser {
           return RawSyntax(firstCase)
         }))
       } else {
-        var tokenList = [RawTokenSyntax]()
-        while !self.at(.eof) && !self.at(.rightBrace)
-                && !self.at(.poundElseifKeyword)
-                && !self.at(.poundElseKeyword) && !self.at(.poundEndifKeyword)
-                && !self.lookahead().isStartOfConditionalSwitchCases() {
-          let tokens = self.recover()
-          guard !tokens.isEmpty else {
-            break
-          }
-          tokenList.append(contentsOf: tokens)
-        }
-        elements.append(RawSyntax(RawNonEmptyTokenListSyntax(elements: tokenList, arena: self.arena)))
+        break
       }
     }
     return RawSwitchCaseListSyntax(elements: elements, arena: self.arena)
@@ -654,10 +643,6 @@ extension Parser {
       while self.at(.atSign) {
         tokenList.append(self.eat(.atSign))
         tokenList.append(self.consumeIdentifier())
-
-        if self.at(.leftParen) {
-          tokenList.append(contentsOf: self.recover())
-        }
       }
 
       unknownAttr = RawAttributeSyntax(

Tests/SwiftParserTest/Assertions.swift

@@ -135,6 +135,12 @@ func AssertDiagnostic<T: SyntaxProtocol>(
   if let message = spec.message {
     AssertStringsEqualWithDiff(diag.message, message, file: file, line: line)
   }
+  if diag.message.contains("\n") {
+    XCTFail("""
+      Diagnostic message should only span a single line. Message was:
+      \(diag.message)
+      """)
+  }
   if let highlight = spec.highlight {
     AssertStringsEqualWithDiff(diag.highlights.map(\.description).joined(), highlight, file: file, line: line)
   }

Tests/SwiftParserTest/Declarations.swift

@@ -507,25 +507,25 @@ final class DeclarationTests: XCTestCase {
       }
       """,
       diagnostics: [
-        // FIXME: The diagnostic should not contain a newline.
         DiagnosticSpec(
           message: """
-            Unexpected text '
-              / ###line 25 "line-directive.swift"' found in struct
+            Unexpected text '/ ###line 25 "line-directive.swift"' found in struct
             """
         )
       ]
     )
   }
 
   func testBogusProtocolRequirements() {
-    // FIXME: This test case should produce a diagnostics
     AssertParse(
       """
       protocol P {
-        var prop : Int { get bogus rethrows set }
+        var prop : Int { get #^DIAG^#bogus rethrows set }
       }
-      """
+      """,
+      diagnostics: [
+        DiagnosticSpec(message: "Unexpected text 'bogus rethrows set' found in variable")
+      ]
     )
   }
 

Tests/SwiftParserTest/Expressions.swift

@@ -202,7 +202,7 @@ final class ExpressionTests: XCTestCase {
       " >> \( abc #^DIAG^#} ) << "
       """#,
       diagnostics: [
-        DiagnosticSpec(message: "Unexpected text '} ' found in string literal")
+        DiagnosticSpec(message: "Unexpected text '}' found in string literal")
       ]
     )