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

Package.swift

@@ -186,7 +186,10 @@ let package = Package(
 
     .target(
       name: "SwiftSyntaxMacroExpansion",
-      dependencies: ["SwiftSyntax", "SwiftSyntaxBuilder", "SwiftSyntaxMacros", "SwiftDiagnostics", "SwiftOperators"],
+      dependencies: [
+        "SwiftSyntax", "SwiftSyntaxBuilder", "SwiftSyntaxMacros", "SwiftDiagnostics", "SwiftOperators",
+        "SwiftParserDiagnostics",
+      ],
       exclude: ["CMakeLists.txt"]
     ),
 

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

@@ -50,6 +50,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 +79,7 @@ enum MacroExpansionError: Error, CustomStringConvertible {
   case noFreestandingMacroRoles(Macro.Type)
   case moreThanOneBodyMacro
   case preambleWithoutBody
+  case noAttachedMacroRoles(Macro.Type)
 
   var description: String {
     switch self {
@@ -92,6 +109,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

@@ -16,6 +16,7 @@ internal import SwiftOperators
 @_spi(MacroExpansion) internal import SwiftParser
 public import SwiftSyntax
 internal import SwiftSyntaxBuilder
+internal import SwiftParserDiagnostics
 @_spi(MacroExpansion) @_spi(ExperimentalLanguageFeature) public import SwiftSyntaxMacros
 #else
 import SwiftDiagnostics
@@ -24,6 +25,7 @@ import SwiftOperators
 import SwiftSyntax
 import SwiftSyntaxBuilder
 @_spi(MacroExpansion) @_spi(ExperimentalLanguageFeature) import SwiftSyntaxMacros
+import SwiftParserDiagnostics
 #endif
 
 // MARK: - Public entry function
@@ -633,6 +635,7 @@ private enum MacroApplicationError: DiagnosticMessage, Error {
   case accessorMacroOnVariableWithMultipleBindings
   case peerMacroOnVariableWithMultipleBindings
   case malformedAccessor
+  case macroAttachedToInvalidDecl(String, String)
 
   var diagnosticID: MessageID {
     return MessageID(domain: diagnosticDomain, id: "\(self)")
@@ -650,6 +653,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 +671,7 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
   /// Store expanded extension while visiting member decls. This should be
   /// added to top-level 'CodeBlockItemList'.
   var extensions: [CodeBlockItemSyntax] = []
+  var expandedAttributes: [AttributeSyntax] = []
 
   init(
     macroSystem: MacroSystem,
@@ -715,14 +721,55 @@ 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, spec) in attributesToRemove {
+        if let index = self.expandedAttributes.firstIndex(where: { expandedAttribute in
+          expandedAttribute.position == attribute.position
+        }) {
+          self.expandedAttributes.remove(at: index)
+        } else {
+          self.diagnosticForUnexpandedMacro(attribute: attribute, macroType: 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) {
+    var diagnosticError: Error? = nil
+    var fixitMessage: FixItMessage? = nil
+    switch macroType {
+    case is AccessorMacro.Type:
+      if (!attachedTo.is(VariableDeclSyntax.self) && !attachedTo.is(SubscriptDeclSyntax.self)) {
+        diagnosticError = MacroApplicationError.macroAttachedToInvalidDecl(
+          MacroRole.accessor.description,
+          attachedTo.kind.nameForDiagnostics ?? ""
+        )
+        fixitMessage = SwiftSyntaxMacros.MacroExpansionFixItMessage(
+          "Remove '\(attribute.trimmedDescription)'"
+        )
+      }
+    default:
+      break
+    }
+    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
@@ -1023,6 +1070,7 @@ extension MacroApplication {
           macroAttribute.conformanceList
         ) {
           result += expanded
+          self.expandedAttributes.append(macroAttribute.attributeNode)
         }
       } catch {
         contextGenerator(Syntax(decl)).addDiagnostics(from: error, node: macroAttribute.attributeNode)
@@ -1170,6 +1218,7 @@ extension MacroApplication {
               from: newAccessors,
               indentationWidth: self.indentationWidth
             )
+            self.expandedAttributes.append(macro.attributeNode)
           }
         } else if let newAccessors = try expandAccessorMacroWithoutExistingAccessors(
           definition: macro.definition,
@@ -1192,6 +1241,7 @@ extension MacroApplication {
           } else {
             newAccessorsBlock = newAccessors
           }
+          self.expandedAttributes.append(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
+    )
+  }
 }