Merge pull request #2209 from kimdv/kimdv/handle-missing-func-keyword

Handle missing `func` keyword

Author: kimdv

Sources/SwiftParser/Declarations.swift

@@ -206,11 +206,24 @@ extension Parser {
       modifiers: self.parseDeclModifierList()
     )
 
-    // If we are inside a memberDecl list, we don't want to eat closing braces (which most likely close the outer context)
-    // while recovering to the declaration start.
-    let recoveryPrecedence = inMemberDeclList ? TokenPrecedence.closingBrace : nil
+    let recoveryResult: (match: DeclarationKeyword, handle: RecoveryConsumptionHandle)?
+    if let atResult = self.at(anyIn: DeclarationKeyword.self) {
+      // We are at a keyword that starts a declaration. Parse that declaration.
+      recoveryResult = (atResult.spec, .noRecovery(atResult.handle))
+    } else if atFunctionDeclarationWithoutFuncKeyword() {
+      // We aren't at a declaration keyword and it looks like we are at a function
+      // declaration. Parse a function declaration.
+      recoveryResult = (.lhs(.func), .missing(.keyword(.func)))
+    } else {
+      // In all other cases, use standard token recovery to find the declaration
+      // to parse.
+      // If we are inside a memberDecl list, we don't want to eat closing braces (which most likely close the outer context)
+      // while recovering to the declaration start.
+      let recoveryPrecedence = inMemberDeclList ? TokenPrecedence.closingBrace : nil
+      recoveryResult = self.canRecoverTo(anyIn: DeclarationKeyword.self, overrideRecoveryPrecedence: recoveryPrecedence)
+    }
 
-    switch self.canRecoverTo(anyIn: DeclarationKeyword.self, overrideRecoveryPrecedence: recoveryPrecedence) {
+    switch recoveryResult {
     case (.lhs(.import), let handle)?:
       return RawDeclSyntax(self.parseImportDeclaration(attrs, handle))
     case (.lhs(.class), let handle)?:
@@ -273,10 +286,7 @@ extension Parser {
         )
       }
 
-      let isPossibleFuncIdentifier = self.at(.identifier, .wildcard)
-      let isPossibleFuncParen = self.peek(isAt: .leftParen, .binaryOperator)
-      // Treat operators specially because they're likely to be functions.
-      if (isPossibleFuncIdentifier && isPossibleFuncParen) || self.at(anyIn: Operator.self) != nil {
+      if atFunctionDeclarationWithoutFuncKeyword() {
         return RawDeclSyntax(self.parseFuncDeclaration(attrs, .missing(.keyword(.func))))
       }
     }
@@ -288,6 +298,24 @@ extension Parser {
       )
     )
   }
+
+  /// Returns `true` if it looks like the parser is positioned at a function declaration that’s missing the `func` keyword.
+  fileprivate mutating func atFunctionDeclarationWithoutFuncKeyword() -> Bool {
+    var nextTokenIsLeftParenOrLeftAngle: Bool {
+      self.peek(isAt: .leftParen) || self.peek().tokenText.hasPrefix("<")
+    }
+
+    if self.at(.identifier) {
+      return nextTokenIsLeftParenOrLeftAngle
+    } else if self.at(anyIn: Operator.self) != nil {
+      if self.currentToken.tokenText.hasSuffix("<") && self.peek(isAt: .identifier) {
+        return true
+      }
+      return nextTokenIsLeftParenOrLeftAngle
+    } else {
+      return false
+    }
+  }
 }
 
 extension Parser {
@@ -1044,7 +1072,7 @@ extension Parser {
     let identifier: RawTokenSyntax
     if self.at(anyIn: Operator.self) != nil || self.at(.exclamationMark, .prefixAmpersand) {
       var name = self.currentToken.tokenText
-      if !currentToken.isEditorPlaceholder && name.count > 1 && name.hasSuffix("<") && self.peek(isAt: .identifier) {
+      if !currentToken.isEditorPlaceholder && name.hasSuffix("<") && self.peek(isAt: .identifier) {
         name = SyntaxText(rebasing: name.dropLast())
       }
       unexpectedBeforeIdentifier = nil

Sources/SwiftParser/Parser.swift

@@ -445,10 +445,7 @@ extension Parser {
   @inline(__always)
   mutating func canRecoverTo(_ spec: TokenSpec) -> RecoveryConsumptionHandle? {
     if self.at(spec) {
-      return RecoveryConsumptionHandle(
-        unexpectedTokens: 0,
-        tokenConsumptionHandle: TokenConsumptionHandle(spec: spec)
-      )
+      return .constant(spec)
     }
     var lookahead = self.lookahead()
     return lookahead.canRecoverTo(spec)
@@ -465,7 +462,7 @@ extension Parser {
     overrideRecoveryPrecedence: TokenPrecedence? = nil
   ) -> (match: SpecSet, handle: RecoveryConsumptionHandle)? {
     if let (kind, handle) = self.at(anyIn: specSet) {
-      return (kind, RecoveryConsumptionHandle(unexpectedTokens: 0, tokenConsumptionHandle: handle))
+      return (kind, .noRecovery(handle))
     }
     var lookahead = self.lookahead()
     return lookahead.canRecoverTo(anyIn: specSet, overrideRecoveryPrecedence: overrideRecoveryPrecedence)

Sources/SwiftParser/Recovery.swift

@@ -21,13 +21,29 @@ struct RecoveryConsumptionHandle {
   var unexpectedTokens: Int
   var tokenConsumptionHandle: TokenConsumptionHandle
 
+  fileprivate init(unexpectedTokens: Int, tokenConsumptionHandle: TokenConsumptionHandle) {
+    self.unexpectedTokens = unexpectedTokens
+    self.tokenConsumptionHandle = tokenConsumptionHandle
+  }
+
+  /// A `RecoveryConsumptionHandle` that doesn't skip over any unexpected tokens
+  /// and consumes a token matching `spec`.
   static func constant(_ spec: TokenSpec) -> RecoveryConsumptionHandle {
     return RecoveryConsumptionHandle(
       unexpectedTokens: 0,
       tokenConsumptionHandle: TokenConsumptionHandle(spec: spec)
     )
   }
 
+  /// A `RecoveryConsumptionHandle` that doesn't skip over any unexpected tokens
+  /// and consumes `handle`.
+  static func noRecovery(_ handle: TokenConsumptionHandle) -> RecoveryConsumptionHandle {
+    return RecoveryConsumptionHandle(
+      unexpectedTokens: 0,
+      tokenConsumptionHandle: handle
+    )
+  }
+
   /// A `RecoveryConsumptionHandle` that will not eat any tokens but instead
   /// synthesize a missing token of kind `token`.
   static func missing(_ spec: TokenSpec) -> RecoveryConsumptionHandle {

Tests/SwiftParserTest/DeclarationTests.swift

@@ -2015,7 +2015,7 @@ final class DeclarationTests: ParserTestCase {
     assertParse(
       """
       class A {
-        1️⃣^2️⃣
+        1️⃣^
       }
       class B {
       }
@@ -2028,23 +2028,8 @@ final class DeclarationTests: ParserTestCase {
               name: .identifier("A"),
               memberBlock: MemberBlockSyntax(
                 leftBrace: .leftBraceToken(),
-                members: MemberBlockItemListSyntax([
-                  MemberBlockItemSyntax(
-                    decl: DeclSyntax(
-                      FunctionDeclSyntax(
-                        funcKeyword: .keyword(.func, presence: .missing),
-                        name: .binaryOperator("^"),
-                        signature: FunctionSignatureSyntax(
-                          parameterClause: FunctionParameterClauseSyntax(
-                            leftParen: .leftParenToken(presence: .missing),
-                            parameters: FunctionParameterListSyntax([]),
-                            rightParen: .rightParenToken(presence: .missing)
-                          )
-                        )
-                      )
-                    )
-                  )
-                ]),
+                members: MemberBlockItemListSyntax(),
+                UnexpectedNodesSyntax([TokenSyntax.binaryOperator("^")]),
                 rightBrace: .rightBraceToken()
               )
             )
@@ -2057,7 +2042,7 @@ final class DeclarationTests: ParserTestCase {
               name: .identifier("B"),
               memberBlock: MemberBlockSyntax(
                 leftBrace: .leftBraceToken(),
-                members: MemberBlockItemListSyntax([]),
+                members: MemberBlockItemListSyntax(),
                 rightBrace: .rightBraceToken()
               )
             )
@@ -2066,16 +2051,8 @@ final class DeclarationTests: ParserTestCase {
       ]
       ),
       diagnostics: [
-        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'func' in function", fixIts: ["insert 'func'"]),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "expected parameter clause in function signature", fixIts: ["insert parameter clause"]),
-      ],
-      fixedSource: """
-        class A {
-          func ^ ()
-        }
-        class B {
-        }
-        """
+        DiagnosticSpec(locationMarker: "1️⃣", message: "unexpected code '^' in class")
+      ]
     )
   }
 

Tests/SwiftParserTest/translated/RecoveryTests.swift

@@ -2107,13 +2107,7 @@ final class RecoveryTests: ParserTestCase {
     )
   }
 
-  func testRecovery120() {
-    assertParse(
-      """
-      //===--- Recovery for wrong decl introducer keyword.
-      """
-    )
-  }
+  // MARK: - Recovery for wrong decl introducer keyword.
 
   func testRecovery121() {
     assertParse(
@@ -2125,20 +2119,36 @@ final class RecoveryTests: ParserTestCase {
       }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 2: expected 'func' keyword in instance method declaration
-        DiagnosticSpec(message: "unexpected code 'notAKeyword() {}' before function")
-      ]
+        DiagnosticSpec(message: "expected 'func' in function", fixIts: ["insert 'func'"])
+      ],
+      fixedSource: """
+        class WrongDeclIntroducerKeyword1 {
+          func notAKeyword() {}
+          func foo() {}
+          class func bar() {}
+        }
+        """
     )
-  }
 
-  func testRecovery122() {
     assertParse(
       """
-      //===--- Recovery for wrong inheritance clause.
-      """
+      class WrongDeclIntroducerKeyword1 {
+        1️⃣notAKeyword() {}
+        var x: Int
+      }
+      """,
+      diagnostics: [DiagnosticSpec(message: "expected 'func' in function", fixIts: ["insert 'func'"])],
+      fixedSource: """
+        class WrongDeclIntroducerKeyword1 {
+          func notAKeyword() {}
+          var x: Int
+        }
+        """
     )
   }
 
+  // MARK: - Recovery for wrong inheritance clause.
+
   func testRecovery123() {
     assertParse(
       """