From c32dca9340a2d9353d28da8da5280caa68743db3 Mon Sep 17 00:00:00 2001 From: Alex Hoppen Date: Tue, 29 Aug 2023 11:16:40 -0700 Subject: [PATCH] Fix issue that caused errors thrown from macro expansion to show up twice in MacroSystem MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../MacroSystem.swift | 62 ++++++++++++++----- .../DeclarationMacroTests.swift | 42 +++++++++++++ .../ExpressionMacroTests.swift | 34 ++++++++++ .../MultiRoleMacroTests.swift | 45 ++++++++++++++ 4 files changed, 166 insertions(+), 17 deletions(-) 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: 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: 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: 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 { + /// 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( _ node: (any FreestandingMacroExpansionSyntax)?, expandMacro: (_ macro: Macro.Type, _ node: any FreestandingMacroExpansionSyntax) throws -> ExpandedMacroType? - ) -> ExpandedMacroType? { + ) -> MacroExpansionResult { 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 { 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 { 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 { return expandFreestandingMacro(node.as(MacroExpansionExprSyntax.self)) { macro, node in return try expandFreestandingExpr( definition: macro, 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] + ) } }