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""#) 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 82500b42703..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 { @@ -50,6 +67,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 @@ -63,6 +96,7 @@ enum MacroExpansionError: Error, CustomStringConvertible { case noFreestandingMacroRoles(Macro.Type) case moreThanOneBodyMacro case preambleWithoutBody + case noAttachedMacroRoles(Macro.Type) var description: String { switch self { @@ -92,6 +126,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" } } } diff --git a/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift b/Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift index 1c539aeb296..8d7b57a0349 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 macroAttachedToInvalidDecl(macroRole: String, declType: String) 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 let .macroAttachedToInvalidDecl(macroType, declType): + return "'\(macroType)' macro cannot be attached to \(declType)" } } } @@ -666,6 +669,8 @@ private class MacroApplication: SyntaxRewriter { /// Store expanded extension while visiting member decls. This should be /// added to top-level 'CodeBlockItemList'. var extensions: [CodeBlockItemSyntax] = [] + /// 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, @@ -703,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 @@ -715,14 +725,49 @@ private class MacroApplication: SyntaxRewriter { let visitedNode = self.visit(declSyntax) skipVisitAnyHandling.remove(Syntax(declSyntax)) - let attributesToRemove = self.macroAttributes(attachedTo: visitedNode).map(\.attributeNode) - - return AttributeRemover(removingWhere: { attributesToRemove.contains($0) }).rewrite(visitedNode) + let attributesToRemove = self.macroAttributes(attachedTo: visitedNode) + 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 + ) } return nil } + 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 + 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, + node: attachedTo, + fixIts: [ + FixIt( + message: fixitMessage, + changes: [.replace(oldNode: Syntax(attribute), newNode: Syntax(AttributeListSyntax()))] + ) + ] + ) + } + } + /// Visit for both the body and preamble macros. func visitBodyAndPreambleMacros( _ node: Node @@ -916,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) } @@ -931,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 @@ -947,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) } } @@ -1016,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, @@ -1170,6 +1216,7 @@ extension MacroApplication { from: newAccessors, indentationWidth: self.indentationWidth ) + attributesToExpand = attributesToExpand.filter { $0.attributeNode != macro.attributeNode } } } else if let newAccessors = try expandAccessorMacroWithoutExistingAccessors( definition: macro.definition, @@ -1192,6 +1239,7 @@ extension MacroApplication { } else { newAccessorsBlock = newAccessors } + attributesToExpand = attributesToExpand.filter { $0.attributeNode != macro.attributeNode } } } catch { contextGenerator(Syntax(storage)).addDiagnostics(from: error, node: macro.attributeNode) 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 471a34c7684..2a6961e6cb0 100644 --- a/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift +++ b/Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift @@ -411,4 +411,70 @@ final class AccessorMacroTests: XCTestCase { indentationWidth: indentationWidth ) } + + func testAccessorNotOnVariableOrSubscript() { + 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 cannot be attached to struct", + line: 1, + column: 1, + fixIts: [FixItSpec(message: "Remove '@Test'")] + ) + ], + 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, + fixIts: [FixItSpec(message: "Remove '@Test'")] + ) + ], + 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, + fixIts: [FixItSpec(message: "Remove '@Test'")] + ) + ], + macros: ["Test": TestMacro.self], + indentationWidth: indentationWidth + ) + } }