Merge pull request #959 from ahoppen/ahoppen/deinits-with-name

Add diagnostic if deinits have names and parameters

Author: ahoppen

Sources/SwiftParser/Declarations.swift

@@ -1310,12 +1310,18 @@ extension Parser {
     _ handle: RecoveryConsumptionHandle
   ) -> RawDeinitializerDeclSyntax {
     let (unexpectedBeforeDeinitKeyword, deinitKeyword) = self.eat(handle)
+    var unexpectedNameAndSignature: [RawSyntax?] = []
+    unexpectedNameAndSignature.append(self.consume(if: .identifier, where: { !$0.isAtStartOfLine }).map(RawSyntax.init))
+    if self.at(.leftParen) && !self.currentToken.isAtStartOfLine {
+      unexpectedNameAndSignature.append(RawSyntax(parseFunctionSignature()))
+    }
     let items = self.parseOptionalCodeBlock()
     return RawDeinitializerDeclSyntax(
       attributes: attrs.attributes,
       modifiers: attrs.modifiers,
       unexpectedBeforeDeinitKeyword,
       deinitKeyword: deinitKeyword,
+      RawUnexpectedNodesSyntax(unexpectedNameAndSignature, arena: self.arena),
       body: items,
       arena: self.arena
     )

Sources/SwiftParser/Diagnostics/DiagnosticExtensions.swift

@@ -52,6 +52,12 @@ extension FixIt.Changes {
     return FixIt.Changes(changes: changes)
   }
 
+  static func makeMissing<SyntaxType: SyntaxProtocol>(node: SyntaxType) -> Self {
+    return FixIt.Changes(changes: [
+      .replace(oldNode: Syntax(node), newNode: MissingMaker().visit(Syntax(node)))
+    ])
+  }
+
   /// Remove the nodes in `unexpected`.
   static func remove(unexpected: UnexpectedNodesSyntax) -> Self {
     var changes: [FixIt.Change] = [

Sources/SwiftParser/Diagnostics/ParseDiagnosticsGenerator.swift

@@ -182,6 +182,9 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     if shouldSkip(node) {
       return .skipChildren
     }
+    if node.allSatisfy({ handledNodes.contains($0.id) }) {
+      return .skipChildren
+    }
     if let tryKeyword = node.onlyToken(where: { $0.tokenKind == .tryKeyword }),
        let nextToken = tryKeyword.nextToken(viewMode: .sourceAccurate),
        nextToken.tokenKind.isKeyword {
@@ -447,6 +450,26 @@ public class ParseDiagnosticsGenerator: SyntaxAnyVisitor {
     return .visitChildren
   }
 
+  public override func visit(_ node: DeinitializerDeclSyntax) -> SyntaxVisitorContinueKind {
+    if shouldSkip(node) {
+      return .skipChildren
+    }
+    if let unexpected = node.unexpectedBetweenDeinitKeywordAndBody,
+       let name = unexpected.filter({ $0.as(TokenSyntax.self)?.tokenKind.isIdentifier == true }).only?.as(TokenSyntax.self) {
+      addDiagnostic(name, .deinitCannotHaveName, fixIts: [
+        FixIt(message: RemoveNodesFixIt(name), changes: .makeMissing(token: name))
+      ], handledNodes: [name.id])
+    }
+    if let unexpected = node.unexpectedBetweenDeinitKeywordAndBody,
+       let signature = unexpected.compactMap({ $0.as(FunctionSignatureSyntax.self) }).only {
+      addDiagnostic(signature, .deinitCannotHaveParameters, fixIts: [
+        FixIt(message: RemoveNodesFixIt(signature), changes: .makeMissing(node: signature))
+      ], handledNodes: [signature.id])
+    }
+
+    return .visitChildren
+  }
+
   public override func visit(_ node: ForInStmtSyntax) -> SyntaxVisitorContinueKind {
     if shouldSkip(node) {
       return .skipChildren

Sources/SwiftParser/Diagnostics/ParserDiagnosticMessages.swift

@@ -74,6 +74,8 @@ public enum StaticParserError: String, DiagnosticMessage {
   case cStyleForLoop = "C-style for statement has been removed in Swift 3"
   case defaultCannotBeUsedWithWhere = "'default' cannot be used with a 'where' guard expression"
   case defaultOutsideOfSwitch = "'default' label can only appear inside a 'switch' statement"
+  case deinitCannotHaveName = "deinitializers cannot have a name"
+  case deinitCannotHaveParameters = "deinitializers cannot have parameters"
   case editorPlaceholderInSourceFile = "editor placeholder in source file"
   case expectedExpressionAfterTry = "expected expression after 'try'"
   case invalidFlagAfterPrecedenceGroupAssignment = "expected 'true' or 'false' after 'assignment'"

Sources/SwiftParser/Diagnostics/PresenceUtils.swift

@@ -124,3 +124,12 @@ class PresentMaker: SyntaxRewriter {
     ))
   }
 }
+
+class MissingMaker: SyntaxRewriter {
+  override func visit(_ node: TokenSyntax) -> Syntax {
+    guard node.presence == .present else {
+      return Syntax(node)
+    }
+    return Syntax(TokenSyntax(node.tokenKind, presence: .missing))
+  }
+}

Tests/SwiftParserTest/translated/AsyncTests.swift

@@ -112,9 +112,9 @@ final class AsyncTests: XCTestCase {
       """
       class X {
         init() async { }
-        deinit1️⃣ async 2️⃣{ }
+        deinit 1️⃣async { }
         func f() async { }
-        subscript(x: Int) 4️⃣async -> Int {
+        subscript(x: Int) 2️⃣async -> Int {
           get {
             return 0
           }
@@ -124,15 +124,8 @@ final class AsyncTests: XCTestCase {
       }
       """,
       diagnostics: [
-        // TODO: Old parser expected error on line 3: deinitializers cannot have a name
-        DiagnosticSpec(locationMarker: "1️⃣", message: "consecutive declarations on a line must be separated by ';'"),
-        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code '{ }' in function"),
-        // TODO: Old parser expected error on line 5: expected '->' for subscript element type
-        // TODO: Old parser expected error on line 5: single argument function types require parentheses
-        // TODO: Old parser expected error on line 5: cannot find type 'async' in scope
-        // TODO: Old parser expected note on line 5: cannot use module 'async' as a type
-        DiagnosticSpec(locationMarker: "4️⃣", message: "unexpected code 'async' in subscript"),
-        // TODO: Old parser expected error on line 9: 'set' accessor cannot have specifier 'async'
+        DiagnosticSpec(locationMarker: "1️⃣", message: "deinitializers cannot have a name"),
+        DiagnosticSpec(locationMarker: "2️⃣", message: "unexpected code 'async' in subscript"),
       ]
     )
   }