From fbc962fff16cb41dd3bf377e9a7f46e45ebeca34 Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Fri, 26 Apr 2024 00:37:44 +0800 Subject: [PATCH 01/10] [Macro] Emit error if AccessorMacro is applied to node that is not a variable --- .../MacroSystem.swift | 11 ++++++++++ .../AccessorMacroTests.swift | 21 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 1c539aeb296..1e53ace62e0 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -633,6 +633,7 @@ private enum MacroApplicationError: DiagnosticMessage, Error { case accessorMacroOnVariableWithMultipleBindings case peerMacroOnVariableWithMultipleBindings case malformedAccessor + case accessorMacroNotOnVariableOrSubscript var diagnosticID: MessageID { return MessageID(domain: diagnosticDomain, id: "\(self)") @@ -650,6 +651,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" } } } @@ -703,6 +706,14 @@ private class MacroApplication: SyntaxRewriter { let attributedNode = node.asProtocol(WithAttributesSyntax.self), !attributedNode.attributes.isEmpty { + if (!macroAttributes(attachedTo: declSyntax, ofType: AccessorMacro.Type.self).isEmpty + && !declSyntax.is(VariableDeclSyntax.self) && !declSyntax.is(SubscriptDeclSyntax.self)) + { + contextGenerator(node).addDiagnostics( + from: MacroApplicationError.accessorMacroNotOnVariableOrSubscript, + node: declSyntax + ) + } // Apply body and preamble macros. if let nodeWithBody = node.asProtocol(WithOptionalCodeBlockSyntax.self), let declNodeWithBody = nodeWithBody as? any DeclSyntaxProtocol & WithOptionalCodeBlockSyntax diff --git a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift index 471a34c7684..cbae4eb8c49 100644 --- a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift +++ b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift @@ -411,4 +411,25 @@ final class AccessorMacroTests: XCTestCase { indentationWidth: indentationWidth ) } + + func testAccessorOnStruct() { + 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 can only be applied to a variable or subscript", line: 1, column: 1) + ], + macros: ["Test": TestMacro.self] + ) + } } From 09252c7754e947ff8da6213998aef32f67615163 Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Sat, 27 Apr 2024 02:34:28 +0800 Subject: [PATCH 02/10] fix: use attributesToRemove instead of getting attributes --- .../MacroSystem.swift | 24 +++++++++++-------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 1e53ace62e0..33ac26a6f30 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -706,14 +706,6 @@ private class MacroApplication: SyntaxRewriter { let attributedNode = node.asProtocol(WithAttributesSyntax.self), !attributedNode.attributes.isEmpty { - if (!macroAttributes(attachedTo: declSyntax, ofType: AccessorMacro.Type.self).isEmpty - && !declSyntax.is(VariableDeclSyntax.self) && !declSyntax.is(SubscriptDeclSyntax.self)) - { - contextGenerator(node).addDiagnostics( - from: MacroApplicationError.accessorMacroNotOnVariableOrSubscript, - node: declSyntax - ) - } // Apply body and preamble macros. if let nodeWithBody = node.asProtocol(WithOptionalCodeBlockSyntax.self), let declNodeWithBody = nodeWithBody as? any DeclSyntaxProtocol & WithOptionalCodeBlockSyntax @@ -726,9 +718,21 @@ private class MacroApplication: SyntaxRewriter { let visitedNode = self.visit(declSyntax) skipVisitAnyHandling.remove(Syntax(declSyntax)) - let attributesToRemove = self.macroAttributes(attachedTo: visitedNode).map(\.attributeNode) + let attributesToRemove = self.macroAttributes(attachedTo: visitedNode) + attributesToRemove.forEach { (attribute, spec) in + if let _ = spec.type as? AccessorMacro.Type, + !declSyntax.is(VariableDeclSyntax.self) && !declSyntax.is(SubscriptDeclSyntax.self) + { + contextGenerator(node).addDiagnostics( + from: MacroApplicationError.accessorMacroNotOnVariableOrSubscript, + node: declSyntax + ) + } + } - return AttributeRemover(removingWhere: { attributesToRemove.contains($0) }).rewrite(visitedNode) + return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite( + visitedNode + ) } return nil From d870f0659604733c5639dc08bdc136399b14a28b Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Sun, 28 Apr 2024 00:56:29 +0800 Subject: [PATCH 03/10] fix: temp for cr --- .../MacroSystem.swift | 26 +++++++++++++------ 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 33ac26a6f30..cb8019e3532 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -669,6 +669,7 @@ private class MacroApplication: 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, @@ -720,16 +721,21 @@ private class MacroApplication: SyntaxRewriter { let attributesToRemove = self.macroAttributes(attachedTo: visitedNode) attributesToRemove.forEach { (attribute, spec) in - 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 index = self.expandedAttributes.firstIndex(where: { expandedAttribute in + expandedAttribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text == attribute.attributeName.as(IdentifierTypeSyntax.self)?.name.text + }) { + 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 + ) + } } } - return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite( visitedNode ) @@ -1038,6 +1044,7 @@ extension MacroApplication { macroAttribute.conformanceList ) { result += expanded + self.expandedAttributes.append(macroAttribute.attributeNode) } } catch { contextGenerator(Syntax(decl)).addDiagnostics(from: error, node: macroAttribute.attributeNode) @@ -1185,6 +1192,7 @@ extension MacroApplication { from: newAccessors, indentationWidth: self.indentationWidth ) + self.expandedAttributes.append(macro.attributeNode) } } else if let newAccessors = try expandAccessorMacroWithoutExistingAccessors( definition: macro.definition, @@ -1207,11 +1215,13 @@ extension MacroApplication { } else { newAccessorsBlock = newAccessors } + self.expandedAttributes.append(macro.attributeNode) } } catch { contextGenerator(Syntax(storage)).addDiagnostics(from: error, node: macro.attributeNode) } } + return (newAccessorsBlock, expandsGetSet) } } From 75725587c5b492f97177f700a96d4d5f44a995b9 Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Tue, 30 Apr 2024 02:51:44 +0800 Subject: [PATCH 04/10] feat: improve diagnostics --- Package.swift | 5 ++- .../SyntaxExtensions.swift | 2 +- .../MacroExpansion.swift | 19 ++++++++- .../MacroSystem.swift | 41 +++++++++++++------ .../AccessorMacroTests.swift | 34 ++++++++++++++- 5 files changed, 84 insertions(+), 17 deletions(-) diff --git a/Package.swift b/Package.swift index 122590d6982..d2fc2801c3c 100644 --- a/Package.swift +++ b/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"] ), diff --git a/Sources/SwiftParserDiagnostics/SyntaxExtensions.swift b/Sources/SwiftParserDiagnostics/SyntaxExtensions.swift index 38ee98bbe30..4d25493a189 100644 --- a/Sources/SwiftParserDiagnostics/SyntaxExtensions.swift +++ b/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 diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift index 82500b42703..45bde6d3701 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift +++ b/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, diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index cb8019e3532..d4067abd4ee 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/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: 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: 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: Node diff --git a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift index cbae4eb8c49..dba54bf66ec 100644 --- a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift +++ b/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 + ) } } From d6b9cd289d1df45fe7634a7c124347ee7c3b8a5c Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Wed, 1 May 2024 22:56:40 +0800 Subject: [PATCH 05/10] feat: support fixit --- .../MacroSystem.swift | 28 +++++++++++++------ .../MacroExpansionContext.swift | 15 +++++++--- .../AccessorMacroTests.swift | 21 ++++++++++++-- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index d4067abd4ee..90a4d0a3c54 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -728,15 +728,25 @@ private class MacroApplication: SyntaxRewriter { }) { self.expandedAttributes.remove(at: index) } else { - 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 - ) - } + 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, + fixIts: [ + FixIt( + message: SwiftSyntaxMacros.MacroExpansionFixItMessage( + "Remove '\(attribute.trimmedDescription)'" + ), + changes: [.replace(oldNode: Syntax(attribute), newNode: Syntax(AttributeListSyntax()))] + ) + ] + ) + } } } return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite( diff --git a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift b/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift index 586ae269aaf..731f8cb2cc3 100644 --- a/Sources/SwiftSyntaxMacros/MacroExpansionContext.swift +++ b/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. diff --git a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift index dba54bf66ec..2a6961e6cb0 100644 --- a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift +++ b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift @@ -427,7 +427,12 @@ final class AccessorMacroTests: XCTestCase { "@Test struct Foo {}", expandedSource: "struct Foo {}", diagnostics: [ - DiagnosticSpec(message: "'accessor' macro cannot be attached to struct", line: 1, column: 1) + DiagnosticSpec( + message: "'accessor' macro cannot be attached to struct", + line: 1, + column: 1, + fixIts: [FixItSpec(message: "Remove '@Test'")] + ) ], macros: ["Test": TestMacro.self] ) @@ -437,7 +442,12 @@ final class AccessorMacroTests: XCTestCase { 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) + DiagnosticSpec( + message: "'accessor' macro cannot be attached to function", + line: 1, + column: 1, + fixIts: [FixItSpec(message: "Remove '@Test'")] + ) ], macros: ["Test": TestMacro.self] ) @@ -456,7 +466,12 @@ final class AccessorMacroTests: XCTestCase { """, 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) + DiagnosticSpec( + message: "'accessor' macro cannot be attached to function", + line: 2, + column: 3, + fixIts: [FixItSpec(message: "Remove '@Test'")] + ) ], macros: ["Test": TestMacro.self], indentationWidth: indentationWidth From ad9e5ae03c64e54e8e49a737cd4ae6c54babec73 Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Thu, 2 May 2024 11:58:29 +0800 Subject: [PATCH 06/10] fix: cr --- .../SyntaxExtensions.swift | 2 +- .../SyntaxKindNameForDiagnostics.swift | 2 +- .../MacroExpansion.swift | 18 +++++++++++++++++- .../MacroSystem.swift | 7 +++---- 4 files changed, 22 insertions(+), 7 deletions(-) diff --git a/Sources/SwiftParserDiagnostics/SyntaxExtensions.swift b/Sources/SwiftParserDiagnostics/SyntaxExtensions.swift index 4d25493a189..38ee98bbe30 100644 --- a/Sources/SwiftParserDiagnostics/SyntaxExtensions.swift +++ b/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`. - public func nodeTypeNameForDiagnostics(allowBlockNames: Bool) -> String? { + func nodeTypeNameForDiagnostics(allowBlockNames: Bool) -> String? { let syntax = Syntax(self) if !allowBlockNames && (syntax.is(CodeBlockSyntax.self) || syntax.is(MemberBlockSyntax.self)) { return nil diff --git a/Sources/SwiftParserDiagnostics/generated/SyntaxKindNameForDiagnostics.swift b/Sources/SwiftParserDiagnostics/generated/SyntaxKindNameForDiagnostics.swift index 4006f445c54..011cc9bdbba 100644 --- a/Sources/SwiftParserDiagnostics/generated/SyntaxKindNameForDiagnostics.swift +++ b/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" diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift index 45bde6d3701..2c63b0a7b75 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift @@ -20,7 +20,7 @@ import SwiftSyntax @_spi(MacroExpansion) @_spi(ExperimentalLanguageFeature) import SwiftSyntaxMacros #endif -public enum MacroRole: String, Sendable { +public enum MacroRole: Sendable { case expression case declaration case accessor @@ -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 diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 90a4d0a3c54..e7072ffa194 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -722,7 +722,7 @@ private class MacroApplication: SyntaxRewriter { skipVisitAnyHandling.remove(Syntax(declSyntax)) let attributesToRemove = self.macroAttributes(attachedTo: visitedNode) - attributesToRemove.forEach { (attribute, spec) in + for (attribute, spec) in attributesToRemove { if let index = self.expandedAttributes.firstIndex(where: { expandedAttribute in expandedAttribute.position == attribute.position }) { @@ -733,8 +733,8 @@ private class MacroApplication: SyntaxRewriter { { contextGenerator(node).addDiagnostics( from: MacroApplicationError.macroAttachedToInvalidDecl( - macroRole.rawValue, - declSyntax.nodeTypeNameForDiagnostics(allowBlockNames: true) ?? "" + macroRole.description, + declSyntax.kind.nameForDiagnostics ?? "" ), node: declSyntax, fixIts: [ @@ -1248,7 +1248,6 @@ extension MacroApplication { contextGenerator(Syntax(storage)).addDiagnostics(from: error, node: macro.attributeNode) } } - return (newAccessorsBlock, expandsGetSet) } } From 83b3407e3666ac862bf3313b40dd7e21036c57f0 Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Sat, 4 May 2024 20:23:08 +0800 Subject: [PATCH 07/10] fix: diagnosticForUnexpandedMacro --- .../MacroExpansion.swift | 13 ----- .../MacroSystem.swift | 51 +++++++++---------- 2 files changed, 25 insertions(+), 39 deletions(-) diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift index 2c63b0a7b75..b5253da1910 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift @@ -193,19 +193,6 @@ 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, diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index e7072ffa194..7e192fd9c49 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -728,25 +728,7 @@ private class MacroApplication: SyntaxRewriter { }) { self.expandedAttributes.remove(at: index) } else { - if let macroRole = try? inferAttachedMacroRole(definition: spec.type), - isInvalidAttachedMacro(macroRole: macroRole, attachedTo: declSyntax) - { - contextGenerator(node).addDiagnostics( - from: MacroApplicationError.macroAttachedToInvalidDecl( - macroRole.description, - declSyntax.kind.nameForDiagnostics ?? "" - ), - node: declSyntax, - fixIts: [ - FixIt( - message: SwiftSyntaxMacros.MacroExpansionFixItMessage( - "Remove '\(attribute.trimmedDescription)'" - ), - changes: [.replace(oldNode: Syntax(attribute), newNode: Syntax(AttributeListSyntax()))] - ) - ] - ) - } + self.diagnosticForUnexpandedMacro(attribute: attribute, macroType: spec.type, attachedTo: declSyntax) } } return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite( @@ -757,18 +739,35 @@ private class MacroApplication: SyntaxRewriter { return nil } - private func isInvalidAttachedMacro(macroRole: MacroRole, attachedTo: DeclSyntax) -> Bool { - switch macroRole { - case .accessor: - // Only var decls and subscripts have accessors + 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)) { - return true + diagnosticError = MacroApplicationError.macroAttachedToInvalidDecl( + MacroRole.accessor.description, + attachedTo.kind.nameForDiagnostics ?? "" + ) + fixitMessage = SwiftSyntaxMacros.MacroExpansionFixItMessage( + "Remove '\(attribute.trimmedDescription)'" + ) } - break default: break } - return false + 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. From 3fe3f7721f36e47f8362d3e0a47d1ad6c8d1699d Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Sun, 5 May 2024 01:00:50 +0800 Subject: [PATCH 08/10] fix: update generated files --- .../SyntaxKindNameForDiagnosticsFile.swift | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparserdiagnostics/SyntaxKindNameForDiagnosticsFile.swift b/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparserdiagnostics/SyntaxKindNameForDiagnosticsFile.swift index 34957cf11ab..c78cd3647ce 100644 --- a/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparserdiagnostics/SyntaxKindNameForDiagnosticsFile.swift +++ b/CodeGeneration/Sources/generate-swift-syntax/templates/swiftparserdiagnostics/SyntaxKindNameForDiagnosticsFile.swift @@ -27,7 +27,7 @@ let syntaxKindNameForDiagnosticFile = SourceFileSyntax(leadingTrivia: copyrightH ) try! ExtensionDeclSyntax("extension SyntaxKind") { - try VariableDeclSyntax("var nameForDiagnostics: String?") { + try VariableDeclSyntax("public var nameForDiagnostics: String?") { try SwitchExprSyntax("switch self") { SwitchCaseSyntax("case .token:") { StmtSyntax(#"return "token""#) From e029a456458e00b0fd4306f7b0489680f978cf3c Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Sun, 5 May 2024 01:14:47 +0800 Subject: [PATCH 09/10] fix: remove dependency of SwiftParserDiagnostics --- Package.swift | 5 +---- Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift | 2 -- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/Package.swift b/Package.swift index d2fc2801c3c..122590d6982 100644 --- a/Package.swift +++ b/Package.swift @@ -186,10 +186,7 @@ let package = Package( .target( name: "SwiftSyntaxMacroExpansion", - dependencies: [ - "SwiftSyntax", "SwiftSyntaxBuilder", "SwiftSyntaxMacros", "SwiftDiagnostics", "SwiftOperators", - "SwiftParserDiagnostics", - ], + dependencies: ["SwiftSyntax", "SwiftSyntaxBuilder", "SwiftSyntaxMacros", "SwiftDiagnostics", "SwiftOperators"], exclude: ["CMakeLists.txt"] ), diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 7e192fd9c49..231270703a8 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -16,7 +16,6 @@ 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 @@ -25,7 +24,6 @@ import SwiftOperators import SwiftSyntax import SwiftSyntaxBuilder @_spi(MacroExpansion) @_spi(ExperimentalLanguageFeature) import SwiftSyntaxMacros -import SwiftParserDiagnostics #endif // MARK: - Public entry function From a9acf65d6fd340fcbddf23a10dfb13b8f7cd8676 Mon Sep 17 00:00:00 2001 From: ninjiacoder Date: Thu, 20 Jun 2024 01:09:09 +0800 Subject: [PATCH 10/10] fix: common diagnostics for unexpanded macros --- .../MacroExpansion.swift | 17 +++++ .../MacroSystem.swift | 68 +++++++++---------- 2 files changed, 51 insertions(+), 34 deletions(-) diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift index b5253da1910..3c1e7f6d483 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroExpansion.swift @@ -32,6 +32,23 @@ public enum MacroRole: Sendable { case `extension` @_spi(ExperimentalLanguageFeature) case preamble @_spi(ExperimentalLanguageFeature) case body + + init?(macroType: Macro.Type) { + switch macroType { + case is ExpressionMacro.Type: self = .expression + case is DeclarationMacro.Type: self = .declaration + case is AccessorMacro.Type: self = .accessor + case is MemberAttributeMacro.Type: self = .memberAttribute + case is MemberMacro.Type: self = .member + case is PeerMacro.Type: self = .peer + case is CodeItemMacro.Type: self = .codeItem + case is ExtensionMacro.Type: self = .extension + case is PreambleMacro.Type: self = .preamble + case is BodyMacro.Type: self = .body + default: + return nil + } + } } extension MacroRole { diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 231270703a8..8d7b57a0349 100644 --- a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift +++ b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift @@ -633,7 +633,7 @@ private enum MacroApplicationError: DiagnosticMessage, Error { case accessorMacroOnVariableWithMultipleBindings case peerMacroOnVariableWithMultipleBindings case malformedAccessor - case macroAttachedToInvalidDecl(String, String) + case macroAttachedToInvalidDecl(macroRole: String, declType: String) var diagnosticID: MessageID { return MessageID(domain: diagnosticDomain, id: "\(self)") @@ -669,7 +669,8 @@ private class MacroApplication: SyntaxRewriter { /// Store expanded extension while visiting member decls. This should be /// added to top-level 'CodeBlockItemList'. var extensions: [CodeBlockItemSyntax] = [] - var expandedAttributes: [AttributeSyntax] = [] + /// Attributes that we are expecting to expand from `visitAny`. As macros are expanded, they should be removed from this list. Any attributes that are still left in this array after a `visitAny` call will generate diagnostics. + var attributesToExpand: [(attributeNode: AttributeSyntax, spec: MacroSpec)] = [] init( macroSystem: MacroSystem, @@ -707,6 +708,11 @@ private class MacroApplication: SyntaxRewriter { let attributedNode = node.asProtocol(WithAttributesSyntax.self), !attributedNode.attributes.isEmpty { + let previousAttributesToExpand = attributesToExpand + defer { + attributesToExpand = previousAttributesToExpand + } + attributesToExpand = self.macroAttributes(attachedTo: declSyntax) // Apply body and preamble macros. if let nodeWithBody = node.asProtocol(WithOptionalCodeBlockSyntax.self), let declNodeWithBody = nodeWithBody as? any DeclSyntaxProtocol & WithOptionalCodeBlockSyntax @@ -720,14 +726,12 @@ private class MacroApplication: SyntaxRewriter { skipVisitAnyHandling.remove(Syntax(declSyntax)) 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) - } + for attribute in attributesToExpand { + self.diagnosticForUnexpandedMacro( + attribute: attribute.attributeNode, + macroType: attribute.spec.type, + attachedTo: declSyntax + ) } return AttributeRemover(removingWhere: { attributesToRemove.map(\.attributeNode).contains($0) }).rewrite( visitedNode @@ -738,22 +742,18 @@ private class MacroApplication: SyntaxRewriter { } private func diagnosticForUnexpandedMacro(attribute: AttributeSyntax, macroType: Macro.Type, attachedTo: DeclSyntax) { + guard let macroRole = MacroRole(macroType: macroType) else { + return + } 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 - } + diagnosticError = MacroApplicationError.macroAttachedToInvalidDecl( + macroRole: macroRole.description, + declType: attachedTo.kind.nameForDiagnostics ?? "" + ) + fixitMessage = SwiftSyntaxMacros.MacroExpansionFixItMessage( + "Remove '\(attribute.trimmedDescription)'" + ) if let diagnosticError, let fixitMessage { contextGenerator(Syntax(attachedTo)).addDiagnostics( from: diagnosticError, @@ -961,10 +961,10 @@ private class MacroApplication: SyntaxRewriter { ) } - override func visit(_ node: VariableDeclSyntax) -> DeclSyntax { - var node = super.visit(node).cast(VariableDeclSyntax.self) + override func visit(_ originalNode: VariableDeclSyntax) -> DeclSyntax { + var node = super.visit(originalNode).cast(VariableDeclSyntax.self) - guard !macroAttributes(attachedTo: DeclSyntax(node), ofType: AccessorMacro.Type.self).isEmpty else { + guard !macroAttributes(attachedTo: DeclSyntax(originalNode), ofType: AccessorMacro.Type.self).isEmpty else { return DeclSyntax(node) } @@ -976,7 +976,7 @@ private class MacroApplication: SyntaxRewriter { return DeclSyntax(node) } - let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock) + let expansion = expandAccessors(of: originalNode, existingAccessors: binding.accessorBlock) if expansion.accessors != binding.accessorBlock { if binding.initializer != nil, expansion.expandsGetSet { // The accessor block will have a leading space, but there will already be a @@ -992,9 +992,9 @@ private class MacroApplication: SyntaxRewriter { return DeclSyntax(node) } - override func visit(_ node: SubscriptDeclSyntax) -> DeclSyntax { - var node = super.visit(node).cast(SubscriptDeclSyntax.self) - node.accessorBlock = expandAccessors(of: node, existingAccessors: node.accessorBlock).accessors + override func visit(_ originalNode: SubscriptDeclSyntax) -> DeclSyntax { + var node = super.visit(originalNode).cast(SubscriptDeclSyntax.self) + node.accessorBlock = expandAccessors(of: originalNode, existingAccessors: node.accessorBlock).accessors return DeclSyntax(node) } } @@ -1061,6 +1061,7 @@ extension MacroApplication { var result: [ExpandedNode] = [] for macroAttribute in macroAttributes(attachedTo: decl, ofType: ofType) { + attributesToExpand = attributesToExpand.filter { $0.attributeNode != macroAttribute.attributeNode } do { if let expanded = try expandMacro( macroAttribute.attributeNode, @@ -1068,7 +1069,6 @@ extension MacroApplication { macroAttribute.conformanceList ) { result += expanded - self.expandedAttributes.append(macroAttribute.attributeNode) } } catch { contextGenerator(Syntax(decl)).addDiagnostics(from: error, node: macroAttribute.attributeNode) @@ -1216,7 +1216,7 @@ extension MacroApplication { from: newAccessors, indentationWidth: self.indentationWidth ) - self.expandedAttributes.append(macro.attributeNode) + attributesToExpand = attributesToExpand.filter { $0.attributeNode != macro.attributeNode } } } else if let newAccessors = try expandAccessorMacroWithoutExistingAccessors( definition: macro.definition, @@ -1239,7 +1239,7 @@ extension MacroApplication { } else { newAccessorsBlock = newAccessors } - self.expandedAttributes.append(macro.attributeNode) + attributesToExpand = attributesToExpand.filter { $0.attributeNode != macro.attributeNode } } } catch { contextGenerator(Syntax(storage)).addDiagnostics(from: error, node: macro.attributeNode)