feat: improve diagnostics

Author: RayZhao1998

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/SyntaxExtensions.swift

@@ -85,7 +85,7 @@ extension Syntax {
 extension SyntaxProtocol {
   /// A name that can be used to describe this node's type in diagnostics or `nil` if there is no good name for this node.
   /// If `allowBlockNames` is `false`, ``CodeBlockSyntax`` and ``MemberDeclBlockSyntax`` are not considered to have a good name and will return `nil`.
-  func nodeTypeNameForDiagnostics(allowBlockNames: Bool) -> String? {
+  public func nodeTypeNameForDiagnostics(allowBlockNames: Bool) -> String? {
     let syntax = Syntax(self)
     if !allowBlockNames && (syntax.is(CodeBlockSyntax.self) || syntax.is(MemberBlockSyntax.self)) {
       return nil

Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift

@@ -20,7 +20,7 @@ import SwiftSyntax
 @_spi(MacroExpansion) @_spi(ExperimentalLanguageFeature) import SwiftSyntaxMacros
 #endif
 
-public enum MacroRole: Sendable {
+public enum MacroRole: String, Sendable {
   case expression
   case declaration
   case accessor
@@ -63,6 +63,7 @@ enum MacroExpansionError: Error, CustomStringConvertible {
   case noFreestandingMacroRoles(Macro.Type)
   case moreThanOneBodyMacro
   case preambleWithoutBody
+  case noAttachedMacroRoles(Macro.Type)
 
   var description: String {
     switch self {
@@ -92,6 +93,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"
     }
   }
 }
@@ -173,6 +177,19 @@ public func inferFreestandingMacroRole(definition: Macro.Type) throws -> MacroRo
   }
 }
 
+public func inferAttachedMacroRole(definition: Macro.Type) throws -> MacroRole {
+  switch definition {
+  case is AccessorMacro.Type: return .accessor
+  case is MemberAttributeMacro.Type: return .memberAttribute
+  case is MemberMacro.Type: return .member
+  case is PeerMacro.Type: return .peer
+  case is ExtensionMacro.Type: return .extension
+
+  default:
+    throw MacroExpansionError.noAttachedMacroRoles(definition)
+  }
+}
+
 @available(*, deprecated, message: "pass a macro role, please!")
 public func expandFreestandingMacro(
   definition: Macro.Type,

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,7 +635,7 @@ private enum MacroApplicationError: DiagnosticMessage, Error {
   case accessorMacroOnVariableWithMultipleBindings
   case peerMacroOnVariableWithMultipleBindings
   case malformedAccessor
-  case accessorMacroNotOnVariableOrSubscript
+  case macroAttachedToInvalidDecl(String, String)
 
   var diagnosticID: MessageID {
     return MessageID(domain: diagnosticDomain, id: "\(self)")
@@ -651,8 +653,8 @@ private enum MacroApplicationError: DiagnosticMessage, Error {
       return """
         macro returned a malformed accessor. Accessors should start with an introducer like 'get' or 'set'.
         """
-    case .accessorMacroNotOnVariableOrSubscript:
-      return "accessor macro can only be applied to a variable or subscript"
+    case let .macroAttachedToInvalidDecl(macroType, declType):
+      return "'\(macroType)' macro cannot be attached to \(declType)"
     }
   }
 }
@@ -722,18 +724,19 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
       let attributesToRemove = self.macroAttributes(attachedTo: visitedNode)
       attributesToRemove.forEach { (attribute, spec) in
         if let index = self.expandedAttributes.firstIndex(where: { expandedAttribute in
-            expandedAttribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text == attribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text
+          expandedAttribute.position == attribute.position
         }) {
           self.expandedAttributes.remove(at: index)
         } else {
-          if let _ = spec.type as? AccessorMacro.Type,
-            !declSyntax.is(VariableDeclSyntax.self) && !declSyntax.is(SubscriptDeclSyntax.self)
-          {
-            contextGenerator(node).addDiagnostics(
-              from: MacroApplicationError.accessorMacroNotOnVariableOrSubscript,
-              node: declSyntax
-            )
-          }
+            if let macroRole = try? inferAttachedMacroRole(definition: spec.type), isInvalidAttachedMacro(macroRole: macroRole, attachedTo: declSyntax) {
+              contextGenerator(node).addDiagnostics(
+                from: MacroApplicationError.macroAttachedToInvalidDecl(
+                  macroRole.rawValue,
+                  declSyntax.nodeTypeNameForDiagnostics(allowBlockNames: true) ?? ""
+                ),
+                node: declSyntax
+              )
+            }
         }
       }
       return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite(
@@ -744,6 +747,20 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
     return nil
   }
 
+  private func isInvalidAttachedMacro(macroRole: MacroRole, attachedTo: DeclSyntax) -> Bool {
+    switch macroRole {
+    case .accessor:
+      // Only var decls and subscripts have accessors
+      if (!attachedTo.is(VariableDeclSyntax.self) && !attachedTo.is(SubscriptDeclSyntax.self)) {
+        return true
+      }
+      break
+    default:
+      break
+    }
+    return false
+  }
+
   /// Visit for both the body and preamble macros.
   func visitBodyAndPreambleMacros<Node: DeclSyntaxProtocol & WithOptionalCodeBlockSyntax>(
     _ node: Node

Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift

@@ -412,7 +412,7 @@ final class AccessorMacroTests: XCTestCase {
     )
   }
 
-  func testAccessorOnStruct() {
+  func testAccessorNotOnVariableOrSubscript() {
     struct TestMacro: AccessorMacro {
       static func expansion(
         of node: AttributeSyntax,
@@ -427,9 +427,39 @@ final class AccessorMacroTests: XCTestCase {
       "@Test struct Foo {}",
       expandedSource: "struct Foo {}",
       diagnostics: [
-        DiagnosticSpec(message: "accessor macro can only be applied to a variable or subscript", line: 1, column: 1)
+        DiagnosticSpec(message: "'accessor' macro cannot be attached to struct", line: 1, column: 1)
       ],
       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)
+      ],
+      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)
+      ],
+      macros: ["Test": TestMacro.self],
+      indentationWidth: indentationWidth
+    )
   }
 }