Merge pull request #655 from ahoppen/ahoppen/no-legacy-recovery

Remove `recover` methods from the parser

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,
@@ -1530,8 +1500,8 @@ extension Parser {
           }
 
           var body = [RawCodeBlockItemSyntax]()
-          while !self.at(.eof) && !self.at(.rightBrace) {
-            body.append(self.parseCodeBlockItem())
+          while !self.at(.rightBrace), let newItem = self.parseCodeBlockItem() {
+            body.append(newItem)
           }
           let (unexpectedBeforeRBrace, rbrace) = self.expect(.rightBrace)
           return RawSyntax(RawCodeBlockSyntax(
@@ -1719,7 +1689,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()
@@ -1772,15 +1742,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/ParseDiagnosticsGenerator.swift

@@ -11,7 +11,7 @@
 //===----------------------------------------------------------------------===//
 
 import SwiftDiagnostics
-import SwiftSyntax
+@_spi(RawSyntax) import SwiftSyntax
 
 extension UnexpectedNodesSyntax {
   func tokens(satisfying isIncluded: (TokenSyntax) -> Bool) -> [TokenSyntax] {
@@ -58,6 +58,9 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
   ) -> [Diagnostic] {
     let diagProducer = ParseDiagnosticsGenerator()
     diagProducer.walk(tree)
+    diagProducer.diagnostics.sort {
+      return $0.node.id.indexInTree < $1.node.id.indexInTree
+    }
     return diagProducer.diagnostics
   }
 
@@ -172,6 +175,17 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: SourceFileSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    if let extraneous = node.unexpectedBetweenStatementsAndEOFToken, !extraneous.isEmpty {
+      addDiagnostic(extraneous, ExtaneousCodeAtTopLevel(extraneousCode: extraneous))
+      markNodesAsHandled(extraneous.id)
+    }
+    return .visitChildren
+  }
+
   public override func visit(_ node: UnresolvedTernaryExprSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren

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.
@@ -90,6 +102,18 @@ public enum StaticParserFixIt: String, FixItMessage {
 
 // MARK: - Diagnostics (please sort alphabetically)
 
+public struct ExtaneousCodeAtTopLevel: ParserError {
+  public let extraneousCode: UnexpectedNodesSyntax
+
+  public var message: String {
+    if let shortContent = extraneousCode.contentForDiagnosticsIfShortSingleLine {
+      return "Extraneous '\(shortContent)' at top level"
+    } else {
+      return "Extraneous code at top level"
+    }
+  }
+}
+
 public struct MissingTokenError: ParserError {
   public let missingToken: TokenSyntax
 
@@ -117,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

@@ -1547,8 +1547,8 @@ extension Parser {
     // Parse the body.
     var elements = [RawCodeBlockItemSyntax]()
     do {
-      while !self.at(.eof) && !self.at(.rightBrace) {
-        elements.append(self.parseCodeBlockItem())
+      while !self.at(.rightBrace), let newItem = self.parseCodeBlockItem() {
+        elements.append(newItem)
       }
     }
 
@@ -1714,7 +1714,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
@@ -1734,26 +1734,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.