Skip to content

Commit 552e539

Browse files
committed
Fix Accessor Macros Attached to Properties With Trailing Comments
fixed the bug that trailing comments of a property would stay in place after macro expansion, commenting out the left brace of the newly created accessor block - the bug affected only properties without an accessor block while having trailing line comments - fixed by moving the trailing trivia of the variable declaration to the trailing trivia of the left brace of the accessor block added tests on properties with trailing comments and with/without an accessor block
1 parent 050f057 commit 552e539

File tree

2 files changed

+105
-6
lines changed

2 files changed

+105
-6
lines changed

Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -923,24 +923,34 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
923923
return DeclSyntax(node)
924924
}
925925

926-
guard node.bindings.count == 1, let binding = node.bindings.first else {
926+
guard node.bindings.count == 1 else {
927927
contextGenerator(Syntax(node)).addDiagnostics(
928928
from: MacroApplicationError.accessorMacroOnVariableWithMultipleBindings,
929929
node: node
930930
)
931931
return DeclSyntax(node)
932932
}
933-
934-
let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
933+
934+
let bindingKeyPath = \VariableDeclSyntax.bindings[node.bindings.startIndex]
935+
let binding = node[keyPath: bindingKeyPath]
936+
var expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
937+
935938
if expansion.accessors != binding.accessorBlock {
939+
if binding.accessorBlock == nil {
940+
// remove the trailing trivia of the variable declaration and move it
941+
// to the trailing trivia of the left brace of the newly created accessor block
942+
node[keyPath: bindingKeyPath].trailingTrivia = []
943+
expansion.accessors?.leftBrace.trailingTrivia = binding.trailingTrivia
944+
}
945+
936946
if binding.initializer != nil, expansion.expandsGetSet {
937947
// The accessor block will have a leading space, but there will already be a
938948
// space between the variable and the to-be-removed initializer. Remove the
939949
// leading trivia on the accessor block so we don't double up.
940-
node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
941-
node.bindings[node.bindings.startIndex].initializer = nil
950+
node[keyPath: bindingKeyPath].accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
951+
node[keyPath: bindingKeyPath].initializer = nil
942952
} else {
943-
node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors
953+
node[keyPath: bindingKeyPath].accessorBlock = expansion.accessors
944954
}
945955
}
946956

Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift

Lines changed: 89 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,95 @@ fileprivate struct ConstantOneGetter: AccessorMacro {
4444
final class AccessorMacroTests: XCTestCase {
4545
private let indentationWidth: Trivia = .spaces(2)
4646

47+
func testAccessorOnVariableDeclWithTrailingLineCommentAndNoAccessorBlock() {
48+
assertMacroExpansion(
49+
"""
50+
@constantOne
51+
var x: Int /*1*/ // hello
52+
""",
53+
expandedSource: """
54+
var x: Int { /*1*/ // hello
55+
get {
56+
return 1
57+
}
58+
}
59+
""",
60+
macros: ["constantOne": ConstantOneGetter.self],
61+
indentationWidth: indentationWidth
62+
)
63+
64+
assertMacroExpansion(
65+
"""
66+
@constantOne
67+
var x: Int /// hello
68+
""",
69+
expandedSource: """
70+
var x: Int { /// hello
71+
get {
72+
return 1
73+
}
74+
}
75+
""",
76+
macros: ["constantOne": ConstantOneGetter.self],
77+
indentationWidth: indentationWidth
78+
)
79+
80+
assertMacroExpansion(
81+
"""
82+
@constantOne
83+
var x: Int = 1 /// hello
84+
""",
85+
expandedSource: """
86+
var x: Int { /// hello
87+
get {
88+
return 1
89+
}
90+
}
91+
""",
92+
macros: ["constantOne": ConstantOneGetter.self],
93+
indentationWidth: indentationWidth
94+
)
95+
96+
assertMacroExpansion(
97+
"""
98+
@constantOne
99+
var x /// hello
100+
""",
101+
expandedSource: """
102+
var x { /// hello
103+
get {
104+
return 1
105+
}
106+
}
107+
""",
108+
macros: ["constantOne": ConstantOneGetter.self],
109+
indentationWidth: indentationWidth
110+
)
111+
}
112+
113+
func testAccessorOnVariableDeclWithTrailingLineCommentAndAccessorBlock() {
114+
assertMacroExpansion(
115+
"""
116+
@constantOne
117+
var x: Int /*h*/ { // hello
118+
1
119+
}
120+
""",
121+
expandedSource: """
122+
var x: Int /*h*/ { // hello
123+
get {
124+
1
125+
}
126+
get {
127+
return 1
128+
}
129+
}
130+
""",
131+
macros: ["constantOne": ConstantOneGetter.self],
132+
indentationWidth: indentationWidth
133+
)
134+
}
135+
47136
func testAccessorOnVariableDeclWithExistingGetter() {
48137
assertMacroExpansion(
49138
"""

0 commit comments

Comments
 (0)