Skip to content

Commit 8bb0a0a

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 d181e99 commit 8bb0a0a

File tree

2 files changed

+138
-5
lines changed

2 files changed

+138
-5
lines changed

Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -923,25 +923,37 @@ 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,
927+
var binding = node.bindings.first
928+
else {
927929
contextGenerator(Syntax(node)).addDiagnostics(
928930
from: MacroApplicationError.accessorMacroOnVariableWithMultipleBindings,
929931
node: node
930932
)
931933
return DeclSyntax(node)
932934
}
933935

934-
let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
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+
expansion.accessors?.leftBrace.trailingTrivia = binding.trailingTrivia
943+
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+
binding.accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
951+
binding.initializer = nil
942952
} else {
943-
node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors
953+
binding.accessorBlock = expansion.accessors
944954
}
955+
956+
node.bindings = [binding]
945957
}
946958

947959
return DeclSyntax(node)

Tests/SwiftSyntaxMacroExpansionTest/AccessorMacroTests.swift

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,127 @@ 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+
136+
func testAccessorOnVariableDeclWithTrailingCommentsAndSingleLineGetterExpansion() {
137+
struct ConstantOneSingleLineGetter: AccessorMacro {
138+
static let formatMode: FormatMode = .disabled
139+
140+
static func expansion(
141+
of node: AttributeSyntax,
142+
providingAccessorsOf declaration: some DeclSyntaxProtocol,
143+
in context: some MacroExpansionContext
144+
) throws -> [AccessorDeclSyntax] {
145+
return [
146+
"""
147+
get { 1 }
148+
"""
149+
]
150+
}
151+
}
152+
153+
assertMacroExpansion(
154+
"""
155+
@constantOne
156+
var x: Int // hello
157+
""",
158+
expandedSource: """
159+
var x: Int { // hello
160+
get { 1 }
161+
}
162+
""",
163+
macros: ["constantOne": ConstantOneSingleLineGetter.self],
164+
indentationWidth: indentationWidth
165+
)
166+
}
167+
47168
func testAccessorOnVariableDeclWithExistingGetter() {
48169
assertMacroExpansion(
49170
"""

0 commit comments

Comments
 (0)