[WIP][Macro] Emit error if AccessorMacro is applied to node that is not a variable

CodeGeneration/Sources/generate-swift-syntax/templates/swiftparserdiagnostics/SyntaxKindNameForDiagnosticsFile.swift

@@ -27,7 +27,7 @@ let syntaxKindNameForDiagnosticFile = SourceFileSyntax(leadingTrivia: copyrightH
   )
 
   try! ExtensionDeclSyntax("extension SyntaxKind") {
-    try VariableDeclSyntax("var nameForDiagnostics: String?") {
+    try VariableDeclSyntax("public var nameForDiagnostics: String?") {
       try SwitchExprSyntax("switch self") {
         SwitchCaseSyntax("case .token:") {
           StmtSyntax(#"return "token""#)

Sources/SwiftParserDiagnostics/generated/SyntaxKindNameForDiagnostics.swift

@@ -19,7 +19,7 @@
 #endif
 
 extension SyntaxKind {
-  var nameForDiagnostics: String? {
+  public var nameForDiagnostics: String? {
     switch self {
     case .token:
       return "token"

Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift

@@ -32,6 +32,23 @@ public enum MacroRole: Sendable {
   case `extension`
   @_spi(ExperimentalLanguageFeature) case preamble
   @_spi(ExperimentalLanguageFeature) case body
+
+  init?(macroType: Macro.Type) {
+    switch macroType {
+    case is ExpressionMacro.Type: self = .expression
+    case is DeclarationMacro.Type: self = .declaration
+    case is AccessorMacro.Type: self = .accessor
+    case is MemberAttributeMacro.Type: self = .memberAttribute
+    case is MemberMacro.Type: self = .member
+    case is PeerMacro.Type: self = .peer
+    case is CodeItemMacro.Type: self = .codeItem
+    case is ExtensionMacro.Type: self = .extension
+    case is PreambleMacro.Type: self = .preamble
+    case is BodyMacro.Type: self = .body
+    default:
+      return nil
+    }
+  }
 }
 
 extension MacroRole {
@@ -50,6 +67,22 @@ extension MacroRole {
     case .body: return "BodyMacro"
     }
   }
+
+  var description: String {
+    switch self {
+    case .expression: return "expression"
+    case .declaration: return "declaration"
+    case .accessor: return "accessor"
+    case .memberAttribute: return "memberAttribute"
+    case .member: return "member"
+    case .peer: return "peer"
+    case .conformance: return "conformance"
+    case .codeItem: return "codeItem"
+    case .extension: return "extension"
+    case .preamble: return "preamble"
+    case .body: return "body"
+    }
+  }
 }
 
 /// Simple diagnostic message
@@ -63,6 +96,7 @@ enum MacroExpansionError: Error, CustomStringConvertible {
   case noFreestandingMacroRoles(Macro.Type)
   case moreThanOneBodyMacro
   case preambleWithoutBody
+  case noAttachedMacroRoles(Macro.Type)
 
   var description: String {
     switch self {
@@ -92,6 +126,9 @@ enum MacroExpansionError: Error, CustomStringConvertible {
 
     case .preambleWithoutBody:
       return "preamble macro cannot be applied to a function with no body"
+
+    case .noAttachedMacroRoles(let type):
+      return "macro implementation type '\(type)' does not conform to any attached macro protocol"
     }
   }
 }

Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift

@@ -633,6 +633,7 @@ private enum MacroApplicationError: DiagnosticMessage, Error {
   case accessorMacroOnVariableWithMultipleBindings
   case peerMacroOnVariableWithMultipleBindings
   case malformedAccessor
+  case macroAttachedToInvalidDecl(macroRole: String, declType: String)
 
   var diagnosticID: MessageID {
     return MessageID(domain: diagnosticDomain, id: "\(self)")
@@ -650,6 +651,8 @@ private enum MacroApplicationError: DiagnosticMessage, Error {
       return """
         macro returned a malformed accessor. Accessors should start with an introducer like 'get' or 'set'.
         """
+    case let .macroAttachedToInvalidDecl(macroType, declType):
+      return "'\(macroType)' macro cannot be attached to \(declType)"
     }
   }
 }
@@ -666,6 +669,8 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
   /// Store expanded extension while visiting member decls. This should be
   /// added to top-level 'CodeBlockItemList'.
   var extensions: [CodeBlockItemSyntax] = []
+  /// Attributes that we are expecting to expand from `visitAny`. As macros are expanded, they should be removed from this list. Any attributes that are still left in this array after a `visitAny` call will generate diagnostics.
+  var attributesToExpand: [(attributeNode: AttributeSyntax, spec: MacroSpec)] = []
 
   init(
     macroSystem: MacroSystem,
@@ -703,6 +708,11 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
       let attributedNode = node.asProtocol(WithAttributesSyntax.self),
       !attributedNode.attributes.isEmpty
     {
+      let previousAttributesToExpand = attributesToExpand
+      defer {
+        attributesToExpand = previousAttributesToExpand
+      }
+      attributesToExpand = self.macroAttributes(attachedTo: declSyntax)
       // Apply body and preamble macros.
       if let nodeWithBody = node.asProtocol(WithOptionalCodeBlockSyntax.self),
         let declNodeWithBody = nodeWithBody as? any DeclSyntaxProtocol & WithOptionalCodeBlockSyntax
@@ -715,14 +725,49 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
       let visitedNode = self.visit(declSyntax)
       skipVisitAnyHandling.remove(Syntax(declSyntax))
 
-      let attributesToRemove = self.macroAttributes(attachedTo: visitedNode).map(\.attributeNode)
-
-      return AttributeRemover(removingWhere: { attributesToRemove.contains($0) }).rewrite(visitedNode)
+      let attributesToRemove = self.macroAttributes(attachedTo: visitedNode)
+      for attribute in attributesToExpand {
+        self.diagnosticForUnexpandedMacro(
+          attribute: attribute.attributeNode,
+          macroType: attribute.spec.type,
+          attachedTo: declSyntax
+        )
+      }
+      return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite(
+        visitedNode
+      )
     }
 
     return nil
   }
 
+  private func diagnosticForUnexpandedMacro(attribute: AttributeSyntax, macroType: Macro.Type, attachedTo: DeclSyntax) {
+    guard let macroRole = MacroRole(macroType: macroType) else {
+      return
+    }
+    var diagnosticError: Error? = nil
+    var fixitMessage: FixItMessage? = nil
+    diagnosticError = MacroApplicationError.macroAttachedToInvalidDecl(
+      macroRole: macroRole.description,
+      declType: attachedTo.kind.nameForDiagnostics ?? ""
+    )
+    fixitMessage = SwiftSyntaxMacros.MacroExpansionFixItMessage(
+      "Remove '\(attribute.trimmedDescription)'"
+    )
+    if let diagnosticError, let fixitMessage {
+      contextGenerator(Syntax(attachedTo)).addDiagnostics(
+        from: diagnosticError,
+        node: attachedTo,
+        fixIts: [
+          FixIt(
+            message: fixitMessage,
+            changes: [.replace(oldNode: Syntax(attribute), newNode: Syntax(AttributeListSyntax()))]
+          )
+        ]
+      )
+    }
+  }
+
   /// Visit for both the body and preamble macros.
   func visitBodyAndPreambleMacros<Node: DeclSyntaxProtocol & WithOptionalCodeBlockSyntax>(
     _ node: Node
@@ -916,10 +961,10 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
     )
   }
 
-  override func visit(_ node: VariableDeclSyntax) -> DeclSyntax {
-    var node = super.visit(node).cast(VariableDeclSyntax.self)
+  override func visit(_ originalNode: VariableDeclSyntax) -> DeclSyntax {
+    var node = super.visit(originalNode).cast(VariableDeclSyntax.self)
 
-    guard !macroAttributes(attachedTo: DeclSyntax(node), ofType: AccessorMacro.Type.self).isEmpty else {
+    guard !macroAttributes(attachedTo: DeclSyntax(originalNode), ofType: AccessorMacro.Type.self).isEmpty else {
       return DeclSyntax(node)
     }
 
@@ -931,7 +976,7 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
       return DeclSyntax(node)
     }
 
-    let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
+    let expansion = expandAccessors(of: originalNode, existingAccessors: binding.accessorBlock)
     if expansion.accessors != binding.accessorBlock {
       if binding.initializer != nil, expansion.expandsGetSet {
         // The accessor block will have a leading space, but there will already be a
@@ -947,9 +992,9 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
     return DeclSyntax(node)
   }
 
-  override func visit(_ node: SubscriptDeclSyntax) -> DeclSyntax {
-    var node = super.visit(node).cast(SubscriptDeclSyntax.self)
-    node.accessorBlock = expandAccessors(of: node, existingAccessors: node.accessorBlock).accessors
+  override func visit(_ originalNode: SubscriptDeclSyntax) -> DeclSyntax {
+    var node = super.visit(originalNode).cast(SubscriptDeclSyntax.self)
+    node.accessorBlock = expandAccessors(of: originalNode, existingAccessors: node.accessorBlock).accessors
     return DeclSyntax(node)
   }
 }
@@ -1016,6 +1061,7 @@ extension MacroApplication {
     var result: [ExpandedNode] = []
 
     for macroAttribute in macroAttributes(attachedTo: decl, ofType: ofType) {
+      attributesToExpand = attributesToExpand.filter { $0.attributeNode != macroAttribute.attributeNode }
       do {
         if let expanded = try expandMacro(
           macroAttribute.attributeNode,
@@ -1170,6 +1216,7 @@ extension MacroApplication {
               from: newAccessors,
               indentationWidth: self.indentationWidth
             )
+            attributesToExpand = attributesToExpand.filter { $0.attributeNode != macro.attributeNode }
           }
         } else if let newAccessors = try expandAccessorMacroWithoutExistingAccessors(
           definition: macro.definition,
@@ -1192,6 +1239,7 @@ extension MacroApplication {
           } else {
             newAccessorsBlock = newAccessors
           }
+          attributesToExpand = attributesToExpand.filter { $0.attributeNode != macro.attributeNode }
         }
       } catch {
         contextGenerator(Syntax(storage)).addDiagnostics(from: error, node: macro.attributeNode)

Sources/SwiftSyntaxMacros/MacroExpansionContext.swift

@@ -108,21 +108,28 @@ private struct ThrownErrorDiagnostic: DiagnosticMessage {
 
 extension MacroExpansionContext {
   /// Adds diagnostics from the error thrown during a macro expansion.
-  public func addDiagnostics(from error: Error, node: some SyntaxProtocol) {
+  public func addDiagnostics(from error: Error, node: some SyntaxProtocol, fixIts: [FixIt] = []) {
     // Inspect the error to form an appropriate set of diagnostics.
     var diagnostics: [Diagnostic]
     if let diagnosticsError = error as? DiagnosticsError {
       diagnostics = diagnosticsError.diagnostics
     } else if let message = error as? DiagnosticMessage {
-      diagnostics = [Diagnostic(node: Syntax(node), message: message)]
+      diagnostics = [Diagnostic(node: Syntax(node), message: message, fixIts: fixIts)]
     } else if let error = error as? SyntaxStringInterpolationInvalidNodeTypeError {
       let diagnostic = Diagnostic(
         node: Syntax(node),
-        message: ThrownErrorDiagnostic(message: "Internal macro error: \(error.description)")
+        message: ThrownErrorDiagnostic(message: "Internal macro error: \(error.description)"),
+        fixIts: fixIts
       )
       diagnostics = [diagnostic]
     } else {
-      diagnostics = [Diagnostic(node: Syntax(node), message: ThrownErrorDiagnostic(message: String(describing: error)))]
+      diagnostics = [
+        Diagnostic(
+          node: Syntax(node),
+          message: ThrownErrorDiagnostic(message: String(describing: error)),
+          fixIts: fixIts
+        )
+      ]
     }
 
     // Emit the diagnostics.

Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift

@@ -411,4 +411,70 @@ final class AccessorMacroTests: XCTestCase {
       indentationWidth: indentationWidth
     )
   }
+
+  func testAccessorNotOnVariableOrSubscript() {
+    struct TestMacro: AccessorMacro {
+      static func expansion(
+        of node: AttributeSyntax,
+        providingAccessorsOf declaration: some DeclSyntaxProtocol,
+        in context: some MacroExpansionContext
+      ) throws -> [AccessorDeclSyntax] {
+        return []
+      }
+    }
+
+    assertMacroExpansion(
+      "@Test struct Foo {}",
+      expandedSource: "struct Foo {}",
+      diagnostics: [
+        DiagnosticSpec(
+          message: "'accessor' macro cannot be attached to struct",
+          line: 1,
+          column: 1,
+          fixIts: [FixItSpec(message: "Remove '@Test'")]
+        )
+      ],
+      macros: ["Test": TestMacro.self]
+    )
+
+    assertMacroExpansion(
+      "@Test func Foo() {}",
+      expandedSource: "func Foo() {}",
+      diagnostics: [
+        // The compiler will reject this with "'accessor' macro cannot be attached to global function"
+        DiagnosticSpec(
+          message: "'accessor' macro cannot be attached to function",
+          line: 1,
+          column: 1,
+          fixIts: [FixItSpec(message: "Remove '@Test'")]
+        )
+      ],
+      macros: ["Test": TestMacro.self]
+    )
+
+    assertMacroExpansion(
+      """
+      struct Foo {
+        @Test
+        func Bar() {}
+      }
+      """,
+      expandedSource: """
+        struct Foo {
+          func Bar() {}
+        }
+        """,
+      diagnostics: [
+        // The compiler will reject this with "'accessor' macro cannot be attached to instance method"
+        DiagnosticSpec(
+          message: "'accessor' macro cannot be attached to function",
+          line: 2,
+          column: 3,
+          fixIts: [FixItSpec(message: "Remove '@Test'")]
+        )
+      ],
+      macros: ["Test": TestMacro.self],
+      indentationWidth: indentationWidth
+    )
+  }
 }