Fix(MacroSystem): Validate MemberMacro application on non-group declarations (Issue #2206)#3257
Conversation
rickhohler
requested review from
ahoppen,
hamishknight,
hborla and
rintaro
as code owners
hamishknight
left a comment
hamishknight
left a comment
There was a problem hiding this comment.
Thanks! It's worth noting this isn't a problem specific to member macros, it seems to also apply to other attached macros e.g extension and accessor, if you're interested in fixing those too (either in this PR or a follow-up)
| !attributedNode.attributes.isEmpty | ||
| { | ||
| // Check if there are any member macros generated on a non-group decl. | ||
| if !(node.isProtocol(DeclGroupSyntax.self) || node.is(ExtensionDeclSyntax.self)) { |
There was a problem hiding this comment.
An ExtensionDeclSyntax is a DeclGroupSyntax so you should only need the former check I think
| } | ||
| } | ||
|
|
||
| final class Issue2206Tests: XCTestCase { |
There was a problem hiding this comment.
This should be folded into the existing tests we have for member macros in MemberMacroTests.swift
|
|
||
| func testMemberMacroOnVariable() { | ||
| // Issue #2206: assertMacroExpansion should emit an error if member macro is applied to declaration that can’t have members | ||
| // Currently this is expected to FAIL because the diagnostic is swallowed. |
| case .peerMacroOnVariableWithMultipleBindings: | ||
| return "peer macro can only be applied to a single variable" | ||
| case .memberMacroOnInvalidDecl(let macroName): | ||
| return "macro '\(macroName)' can only be applied to a struct, enum, class, extension, or actor" |
There was a problem hiding this comment.
I would say member macro here for clarity, also I'm not sure it's worth listing every type decl kind (note you missed protocol), maybe it would be better just to say type declaration or extension instead?
2 participants
Fix(MacroSystem): Validate MemberMacro application on non-group declarations (Issue #2206)
Description
This PR fixes an issue where
MemberMacroattributes applied to invalid declarations (such asvarproperties or functions) were silently ignored by theMacroApplicationwalker.Before this change, the following code would compile successfully with 0 diagnostics, even though
@Testis aMemberMacro:After this change, it correctly produces the expected diagnostic:
error: macro 'Test' can only be applied to a struct, enum, class, extension, or actorFixes: #2206
Motivation
The
visit(_ : MemberBlockSyntax)method handles member macros correctly for container types. However, when aMemberMacrois applied to a standalone declaration (like a VariableDecl), the walker visits it viavisitAnyor specific node visitors which previously lacked logic to check for member macro attributes. This resulted in silent failures where the user would expect either an expansion or an error.Detailed Design
MacroApplicationError.memberMacroOnInvalidDecl(macroName: String).MacroApplication.visitAny(_:)to inspectDeclSyntaxnodes.MemberMacrobut does not conform toDeclGroupSyntax(and is not anExtensionDeclSyntax), a diagnostic is now added to the context.Verification
Tests/SwiftSyntaxMacroExpansionTest/Issue2206Tests.swift.SwiftSyntaxMacroExpansion.Checklist