From 55a0237337b46ed11bf5e0c61434b7f970850dae 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 ++++++--- .../MacroSystemTests.swift | 119 ++++++++++++++++++ 2 files changed, 164 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/MacroSystemTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/MacroSystemTests.swift index 3de1c9e6e78..a3c9b24e475 100644 --- a/Tests/SwiftSyntaxMacroExpansionTest/MacroSystemTests.swift +++ b/Tests/SwiftSyntaxMacroExpansionTest/MacroSystemTests.swift @@ -1775,4 +1775,123 @@ final class MacroSystemTests: XCTestCase { ) } + 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] + ) + } + + 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] + ) + } + + 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] + ) + } }