Repository: swiftlang/swift-syntax (swift)

Issue:
`assertMacroExpansion` reports duplicate diagnostic
### Description

Expression macros that throw errors report duplicate diagnostics to `assertMacroExpansion`, and these duplicate diagnostics must be asserted against.

### Steps to Reproduce

I have a slightly modified version of the default template `#stringify` macro that throws an error when the input is too long:

```diff
 public struct StringifyMacro: ExpressionMacro {
   public static func expansion<
     F: FreestandingMacroExpansionSyntax, C: MacroExpansionContext
   >(
     of node: F,
     in context: C
-  ) -> ExprSyntax {
+  ) throws -> ExprSyntax {
     guard let argument = node.argumentList.first?.expression else {
       fatalError("compiler bug: the macro does not have any arguments")
     }

+    guard argument.description.count <= 20
+    else {
+      throw SomeError()
+    }
+
     return "(\(argument), \(literal: argument.description))"
   }
 }
+
+struct SomeError: Error {}
```

If I write an expansion test for this:

```swift
assertMacroExpansion(
  """
  #stringify(a + b + c + d + e + f)
  """,
  expandedSource: """
    #stringify(a + b + c + d + e + f)
    """,
  macros: testMacros
)
```

I get the following failure:

> 🛑 failed - Expected 0 diagnostics but received 2:
> 1:1: SomeError()
> 1:1: SomeError()

Updating the assertion requires including _two_ diagnostics even though there should be just one.

Pull request:
If the macro expansion of a freestanding expression macro throws an error, `expandCodeBlockItem` returned `nil` while adding the thrown error to the macro expansion context. `visit(_:CodeBlockItemListSyntax).addResult` took the `nil` return value as an indicator that the macro wasn’t expanded because its macro definition wasn’t found and ended up calling the expansion again in

```swift
// Recurse on the child node
newItems.append(visit(node))
```

Change the return value of `expandCodeBlockItem` to an enum that indicates whether the macro was not found or if the expansion failed. If the macro was found but the expansion threw an error, we just just retain the macro as-is without calling into `visit` again.

Fixes #2111
rdar://114592410

Patch:
diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
index 6e657e064c0..4c62cc3eb3e 100644
--- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
+++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift
@@ -485,10 +485,14 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
     // Note that 'MacroExpansionExpr'/'MacroExpansionExprDecl' at code item
     // position are handled by 'visit(_:CodeBlockItemListSyntax)'.
     // Only expression expansions inside other syntax nodes is handled here.
-    if let expanded = expandExpr(node: node) {
+    switch expandExpr(node: node) {
+    case .success(let expanded):
       return Syntax(visit(expanded))
+    case .failure:
+      return Syntax(node)
+    case .notAMacro:
+      break
     }
-
     if let declSyntax = node.as(DeclSyntax.self),
       let attributedNode = node.asProtocol(WithAttributesSyntax.self),
       !attributedNode.attributes.isEmpty
@@ -510,16 +514,21 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
     var newItems: [CodeBlockItemSyntax] = []
     func addResult(_ node: CodeBlockItemSyntax) {
       // Expand freestanding macro.
-      if let expanded = expandCodeBlockItem(node: node) {
+      switch expandCodeBlockItem(node: node) {
+      case .success(let expanded):
         for item in expanded {
           addResult(item)
         }
         return
+      case .failure:
+        // Expanding the macro threw an error. We don't have an expanded source.
+        // Retain the macro node as-is.
+        newItems.append(node)
+      case .notAMacro:
+        // Recurse on the child node
+        newItems.append(visit(node))
       }
 
-      // Recurse on the child node
-      newItems.append(visit(node))
-
       // Expand any peer macro on this item.
       if case .decl(let decl) = node.item {
         for peer in expandCodeBlockPeers(of: decl) {
@@ -552,16 +561,19 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
 
     func addResult(_ node: MemberBlockItemSyntax) {
       // Expand freestanding macro.
-      if let expanded = expandMemberDecl(node: node) {
+      switch expandMemberDecl(node: node) {
+      case .success(let expanded):
         for item in expanded {
           addResult(item)
         }
         return
+      case .failure:
+        newItems.append(node)
+      case .notAMacro:
+        // Recurse on the child node.
+        newItems.append(visit(node))
       }
 
-      // Recurse on the child node.
-      newItems.append(visit(node))
-
       // Expand any peer macro on this member.
       for peer in expandMemberDeclPeers(of: node.decl) {
         addResult(peer)
@@ -842,20 +854,36 @@ extension MacroApplication {
 // MARK: Freestanding macro expansion
 
 extension MacroApplication {
+  enum MacroExpansionResult<ResultType> {
+    /// Expansion of the macro succeeded.
+    case success(ResultType)
+
+    /// Macro system found the macro to expand but running the expansion threw
+    /// an error and thus no expansion result exists.
+    case failure
+
+    /// The node that should be expanded was not a macro known to the macro system.
+    case notAMacro
+  }
+
   private func expandFreestandingMacro<ExpandedMacroType: SyntaxProtocol>(
     _ node: (any FreestandingMacroExpansionSyntax)?,
     expandMacro: (_ macro: Macro.Type, _ node: any FreestandingMacroExpansionSyntax) throws -> ExpandedMacroType?
-  ) -> ExpandedMacroType? {
+  ) -> MacroExpansionResult<ExpandedMacroType> {
     guard let node,
       let macro = macroSystem.lookup(node.macro.text)
     else {
-      return nil
+      return .notAMacro
     }
     do {
-      return try expandMacro(macro, node)
+      if let expanded = try expandMacro(macro, node) {
+        return .success(expanded)
+      } else {
+        return .failure
+      }
     } catch {
       context.addDiagnostics(from: error, node: node)
-      return nil
+      return .failure
     }
   }
 
@@ -867,7 +895,7 @@ extension MacroApplication {
   ///   #foo
   /// }
   /// ```
-  func expandCodeBlockItem(node: CodeBlockItemSyntax) -> CodeBlockItemListSyntax? {
+  func expandCodeBlockItem(node: CodeBlockItemSyntax) -> MacroExpansionResult<CodeBlockItemListSyntax> {
     return expandFreestandingMacro(node.item.asProtocol(FreestandingMacroExpansionSyntax.self)) { macro, node in
       return try expandFreestandingCodeItemList(
         definition: macro,
@@ -886,7 +914,7 @@ extension MacroApplication {
   ///   #foo
   /// }
   /// ```
-  func expandMemberDecl(node: MemberBlockItemSyntax) -> MemberBlockItemListSyntax? {
+  func expandMemberDecl(node: MemberBlockItemSyntax) -> MacroExpansionResult<MemberBlockItemListSyntax> {
     return expandFreestandingMacro(node.decl.as(MacroExpansionDeclSyntax.self)) { macro, node in
       return try expandFreestandingMemberDeclList(
         definition: macro,
@@ -904,7 +932,7 @@ extension MacroApplication {
   /// ```swift
   /// let a = #foo
   /// ```
-  func expandExpr(node: Syntax) -> ExprSyntax? {
+  func expandExpr(node: Syntax) -> MacroExpansionResult<ExprSyntax> {
     return expandFreestandingMacro(node.as(MacroExpansionExprSyntax.self)) { macro, node in
       return try expandFreestandingExpr(
         definition: macro,


Test patch:
diff --git a/Tests/SwiftSyntaxMacroExpansionTest/DeclarationMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/DeclarationMacroTests.swift
index 3832cff8f87..1ccd9e5ac07 100644
--- a/Tests/SwiftSyntaxMacroExpansionTest/DeclarationMacroTests.swift
+++ b/Tests/SwiftSyntaxMacroExpansionTest/DeclarationMacroTests.swift
@@ -347,4 +347,46 @@ final class DeclarationMacroTests: XCTestCase {
       indentationWidth: indentationWidth
     )
   }
+
+  func testThrowErrorFromDeclMacro() {
+    struct MyError: Error, CustomStringConvertible {
+      let description: String = "my error"
+    }
+
+    struct TestMacro: DeclarationMacro {
+      static func expansion(
+        of node: some FreestandingMacroExpansionSyntax,
+        in context: some MacroExpansionContext
+      ) throws -> [DeclSyntax] {
+        throw MyError()
+      }
+    }
+
+    assertMacroExpansion(
+      "#test",
+      expandedSource: "#test",
+      diagnostics: [
+        DiagnosticSpec(message: "my error", line: 1, column: 1)
+      ],
+      macros: ["test": TestMacro.self]
+    )
+
+    assertMacroExpansion(
+      """
+      struct Foo {
+        #test
+      }
+      """,
+      expandedSource: """
+        struct Foo {
+          #test
+        }
+        """,
+      diagnostics: [
+        DiagnosticSpec(message: "my error", line: 2, column: 3)
+      ],
+      macros: ["test": TestMacro.self]
+    )
+  }
+
 }
diff --git a/Tests/SwiftSyntaxMacroExpansionTest/ExpressionMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/ExpressionMacroTests.swift
index 3bffc01ffd6..30cba4a2240 100644
--- a/Tests/SwiftSyntaxMacroExpansionTest/ExpressionMacroTests.swift
+++ b/Tests/SwiftSyntaxMacroExpansionTest/ExpressionMacroTests.swift
@@ -200,4 +200,38 @@ final class ExpressionMacroTests: XCTestCase {
       indentationWidth: indentationWidth
     )
   }
+
+  func testThrowErrorFromExpressionMacro() {
+    struct MyError: Error, CustomStringConvertible {
+      let description: String = "my error"
+    }
+
+    struct TestMacro: ExpressionMacro {
+      public static func expansion(
+        of node: some FreestandingMacroExpansionSyntax,
+        in context: some MacroExpansionContext
+      ) throws -> ExprSyntax {
+        throw MyError()
+      }
+    }
+
+    assertMacroExpansion(
+      "#test",
+      expandedSource: "#test",
+      diagnostics: [
+        DiagnosticSpec(message: "my error", line: 1, column: 1)
+      ],
+      macros: ["test": TestMacro.self]
+    )
+
+    assertMacroExpansion(
+      "1 + #test",
+      expandedSource: "1 + #test",
+      diagnostics: [
+        DiagnosticSpec(message: "my error", line: 1, column: 5)
+      ],
+      macros: ["test": TestMacro.self]
+    )
+  }
+
 }
diff --git a/Tests/SwiftSyntaxMacroExpansionTest/MultiRoleMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/MultiRoleMacroTests.swift
index 7166bcfec9d..903c58b2dae 100644
--- a/Tests/SwiftSyntaxMacroExpansionTest/MultiRoleMacroTests.swift
+++ b/Tests/SwiftSyntaxMacroExpansionTest/MultiRoleMacroTests.swift
@@ -204,6 +204,51 @@ final class MultiRoleMacroTests: XCTestCase {
       macros: ["customTypeWrapper": CustomTypeWrapperMacro.self],
       indentationWidth: indentationWidth
     )
+  }
+
+  func testAttachedMacroOnFreestandingMacro() {
+    struct DeclMacro: DeclarationMacro {
+      static func expansion(of node: some FreestandingMacroExpansionSyntax, in context: some MacroExpansionContext) throws -> [DeclSyntax] {
+        return ["var x: Int"]
+      }
+    }
+
+    struct MyPeerMacro: PeerMacro {
+      static func expansion(of node: AttributeSyntax, providingPeersOf declaration: some DeclSyntaxProtocol, in context: some MacroExpansionContext) throws
+        -> [DeclSyntax]
+      {
+        return ["var peer: Int"]
+      }
+    }
+
+    assertMacroExpansion(
+      """
+      struct Foo {
+        @Peer
+        #decl
+      }
+      """,
+      expandedSource: """
+        struct Foo {
+          var x: Int
 
+          var peer: Int
+        }
+        """,
+      macros: ["decl": DeclMacro.self, "Peer": MyPeerMacro.self]
+    )
+
+    assertMacroExpansion(
+      """
+      @Peer
+      #decl
+      """,
+      expandedSource: """
+        var x: Int
+
+        var peer: Int
+        """,
+      macros: ["decl": DeclMacro.self, "Peer": MyPeerMacro.self]
+    )
   }
 }