Applied review comments

Author: nickolas-pohilets

Sources/SwiftParser/Declarations.swift

@@ -1167,7 +1167,9 @@ extension Parser {
 
 extension Parser {
   /// If a `throws` keyword appears right in front of the `arrow`, it is returned as `misplacedThrowsKeyword` so it can be synthesized in front of the arrow.
-  mutating func parseFunctionReturnClause(effectSpecifiers: inout (some RawEffectSpecifiersTrait)?, allowNamedOpaqueResultType: Bool) -> RawReturnClauseSyntax {
+  mutating func parseFunctionReturnClause(effectSpecifiers: inout (some RawMisplacedEffectSpecifiersTrait)?, allowNamedOpaqueResultType: Bool)
+    -> RawReturnClauseSyntax
+  {
     let (unexpectedBeforeArrow, arrow) = self.expect(.arrow)
     let unexpectedBeforeReturnType = self.parseMisplacedEffectSpecifiers(&effectSpecifiers)
     let result: RawTypeSyntax
@@ -1251,7 +1253,7 @@ extension Parser {
 
     /// Only allow recovery to the arrow with exprKeyword precedence so we only
     /// skip over misplaced identifiers and don't e.g. recover to an arrow in a 'where' clause.
-    if self.at(.arrow) || self.canRecoverTo(TokenSpec(.arrow, recoveryPrecedence: .exprKeyword)) != nil {
+    if self.canRecoverTo(TokenSpec(.arrow, recoveryPrecedence: .exprKeyword)) != nil {
       output = self.parseFunctionReturnClause(effectSpecifiers: &effectSpecifiers, allowNamedOpaqueResultType: true)
     } else {
       output = nil

Sources/SwiftParser/Recovery.swift

@@ -130,7 +130,10 @@ extension Parser.Lookahead {
     #endif
     let initialTokensConsumed = self.tokensConsumed
 
-    precondition(!specSet.allCases.isEmpty, "SpecSet must have at least one case")
+    if specSet.allCases.isEmpty {
+      return nil
+    }
+
     let recoveryPrecedence =
       overrideRecoveryPrecedence ?? specSet.allCases.map({
         return $0.spec.recoveryPrecedence

Sources/SwiftParser/Specifiers.swift

@@ -134,7 +134,7 @@ public enum EffectSpecifier: TokenSpecSet {
 
 /// Raw syntax nodes don't have traits (because usually we don't need them).
 /// Specify the effect specifiers trait manually as a one off.
-protocol RawEffectSpecifiersTrait {
+protocol RawMisplacedEffectSpecifiersTrait {
   /// The token kinds that should be consumed as misspelled `asyncSpecifier`.
   /// Should be a subset of ``AsyncEffectSpecifier``.
   associatedtype MisspelledAsyncTokenKinds: TokenSpecSet
@@ -151,10 +151,21 @@ protocol RawEffectSpecifiersTrait {
   /// Should be a subset of ``ThrowsEffectSpecifier``.
   associatedtype CorrectThrowsTokenKinds: TokenSpecSet
 
-  var unexpectedBeforeAsyncSpecifier: RawUnexpectedNodesSyntax? { get }
   var asyncSpecifier: RawTokenSyntax? { get }
-  var unexpectedBetweenAsyncSpecifierAndThrowsSpecifier: RawUnexpectedNodesSyntax? { get }
   var throwsSpecifier: RawTokenSyntax? { get }
+
+  init(
+    asyncSpecifier: RawTokenSyntax?,
+    throwsSpecifier: RawTokenSyntax?,
+    arena: __shared SyntaxArena
+  )
+
+  func withMisplaced(async misplacedAsyncKeyword: RawTokenSyntax?, throws misplacedThrowsKeyword: RawTokenSyntax?, arena: __shared SyntaxArena) -> Self
+}
+
+protocol RawEffectSpecifiersTrait: RawMisplacedEffectSpecifiersTrait {
+  var unexpectedBeforeAsyncSpecifier: RawUnexpectedNodesSyntax? { get }
+  var unexpectedBetweenAsyncSpecifierAndThrowsSpecifier: RawUnexpectedNodesSyntax? { get }
   var unexpectedAfterThrowsSpecifier: RawUnexpectedNodesSyntax? { get }
 
   init(
@@ -168,6 +179,21 @@ protocol RawEffectSpecifiersTrait {
 }
 
 extension RawEffectSpecifiersTrait {
+  init(
+    asyncSpecifier: RawTokenSyntax?,
+    throwsSpecifier: RawTokenSyntax?,
+    arena: __shared SyntaxArena
+  ) {
+    self.init(
+      nil,
+      asyncSpecifier: asyncSpecifier,
+      nil,
+      throwsSpecifier: throwsSpecifier,
+      nil,
+      arena: arena
+    )
+  }
+
   func withMisplaced(async misplacedAsyncKeyword: RawTokenSyntax?, throws misplacedThrowsKeyword: RawTokenSyntax?, arena: __shared SyntaxArena) -> Self {
     return Self.init(
       self.unexpectedBeforeAsyncSpecifier,
@@ -417,8 +443,7 @@ extension RawAccessorEffectSpecifiersSyntax: RawEffectSpecifiersTrait {
   }
 }
 
-// This conformance exists solely to parse misplaced specifiers in parseFunctionReturnClause()
-extension RawDeinitEffectSpecifiersSyntax: RawEffectSpecifiersTrait {
+extension RawDeinitEffectSpecifiersSyntax: RawMisplacedEffectSpecifiersTrait {
   enum MisspelledAsyncTokenKinds: TokenSpecSet {
     case await
     case reasync
@@ -495,25 +520,32 @@ extension RawDeinitEffectSpecifiersSyntax: RawEffectSpecifiersTrait {
     }
   }
 
-  var unexpectedBetweenAsyncSpecifierAndThrowsSpecifier: RawUnexpectedNodesSyntax? { unexpectedAfterAsyncSpecifier }
   var throwsSpecifier: RawTokenSyntax? { nil }
-  var unexpectedAfterThrowsSpecifier: RawUnexpectedNodesSyntax? { nil }
 
   init(
-    _ unexpectedBeforeAsyncSpecifier: RawUnexpectedNodesSyntax?,
     asyncSpecifier: RawTokenSyntax?,
-    _ unexpectedBetweenAsyncSpecifierAndThrowsSpecifier: RawUnexpectedNodesSyntax?,
     throwsSpecifier: RawTokenSyntax?,
-    _ unexpectedAfterThrowsSpecifier: RawUnexpectedNodesSyntax?,
     arena: __shared SwiftSyntax.SyntaxArena
   ) {
     // Inserted missing throws is discarded
-    assert(throwsSpecifier?.isMissing ?? true)
-    assert(unexpectedAfterThrowsSpecifier == nil)
+    precondition(throwsSpecifier?.isMissing ?? true)
     self.init(
-      unexpectedBeforeAsyncSpecifier,
+      nil,
       asyncSpecifier: asyncSpecifier,
-      unexpectedBetweenAsyncSpecifierAndThrowsSpecifier,
+      nil,
+      arena: arena
+    )
+  }
+
+  func withMisplaced(async misplacedAsyncKeyword: RawTokenSyntax?, throws misplacedThrowsKeyword: RawTokenSyntax?, arena: SyntaxArena)
+    -> RawDeinitEffectSpecifiersSyntax
+  {
+    // Inserted missing throws is discarded
+    precondition(throwsSpecifier?.isMissing ?? true)
+    return Self.init(
+      self.unexpectedBeforeAsyncSpecifier,
+      asyncSpecifier: self.asyncSpecifier ?? misplacedAsyncKeyword,
+      self.unexpectedAfterAsyncSpecifier,
       arena: arena
     )
   }
@@ -672,7 +704,7 @@ extension Parser {
   /// When a misplaced effect specifier is consumed and `effectSpecifiers`
   /// doesn't have  an effect specifier of that kind, modify `effectSpecifiers`
   /// to have a missing specifier of that kind.
-  mutating func parseMisplacedEffectSpecifiers<S: RawEffectSpecifiersTrait>(_ effectSpecifiers: inout S?) -> RawUnexpectedNodesSyntax? {
+  mutating func parseMisplacedEffectSpecifiers<S: RawMisplacedEffectSpecifiersTrait>(_ effectSpecifiers: inout S?) -> RawUnexpectedNodesSyntax? {
     var synthesizedAsync: RawTokenSyntax? = nil
     var synthesizedThrows: RawTokenSyntax? = nil
     var unexpected: [RawTokenSyntax] = []
@@ -707,11 +739,8 @@ extension Parser {
         effectSpecifiers = specifiers.withMisplaced(async: synthesizedAsync, throws: synthesizedThrows, arena: self.arena)
       } else {
         effectSpecifiers = S(
-          nil,
           asyncSpecifier: synthesizedAsync,
-          nil,
           throwsSpecifier: synthesizedThrows,
-          nil,
           arena: self.arena
         )
       }

Tests/SwiftParserTest/translated/InitDeinitTests.swift

@@ -972,6 +972,44 @@ final class InitDeinitTests: XCTestCase {
     )
   }
 
+  func testDeinitOutputAsyncThrows() {
+    assertParse(
+      """
+      class FooClassDeinitializerA {
+        deinit 1️⃣-> async throws Void {}
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'async' in effect specifiers", fixIts: ["insert 'async'"]),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "deinitializers cannot have return type", fixIts: ["remove '->', 'async throws', and return type"]),
+      ],
+      fixedSource: """
+        class FooClassDeinitializerA {
+          deinit async {}
+        }
+        """
+    )
+  }
+
+  func testDeinitOutputThrowsAsync() {
+    assertParse(
+      """
+      class FooClassDeinitializerA {
+        deinit 1️⃣-> throws async Void {}
+      }
+      """,
+      diagnostics: [
+        DiagnosticSpec(locationMarker: "1️⃣", message: "expected 'async' in effect specifiers", fixIts: ["insert 'async'"]),
+        DiagnosticSpec(locationMarker: "1️⃣", message: "deinitializers cannot have return type", fixIts: ["remove '->', 'throws async', and return type"]),
+      ],
+      fixedSource: """
+        class FooClassDeinitializerA {
+          deinit async {}
+        }
+        """
+    )
+  }
+
   func testAsyncDeinit() {
     // This is expected for now.
     // `async` is parsed as a modifier like `public` because you can have an `async var x: Int`.