Skip to content

Commit 8a5f2d1

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 transfering line comments from the trailing trivia of the variable binding to the left brace of the accessor block added tests on properties with trailing comments and with/without an accessor block
1 parent 050f057 commit 8a5f2d1

File tree

2 files changed

+115
-4
lines changed

2 files changed

+115
-4
lines changed

Sources/SwiftSyntaxMacroExpansion/MacroSystem.swift

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -931,17 +931,39 @@ private class MacroApplication<Context: MacroExpansionContext>: SyntaxRewriter {
931931
return DeclSyntax(node)
932932
}
933933

934-
let expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
934+
var expansion = expandAccessors(of: node, existingAccessors: binding.accessorBlock)
935935
if expansion.accessors != binding.accessorBlock {
936+
var newBinding = binding
937+
if binding.accessorBlock == nil, let accessors = expansion.accessors {
938+
// transfer all line comments from the trailing trivia of the binding pattern
939+
// to the trailing trivia of the left brace of the newly created accessor block
940+
var truncatedPieces = [TriviaPiece]()
941+
var leftBraceTriviaPieces = accessors.leftBrace.trailingTrivia.pieces
942+
943+
for piece in binding.trailingTrivia {
944+
switch piece {
945+
case .lineComment, .docLineComment:
946+
leftBraceTriviaPieces.append(piece)
947+
default:
948+
truncatedPieces.append(piece)
949+
}
950+
}
951+
952+
newBinding.trailingTrivia = Trivia(pieces: truncatedPieces)
953+
expansion.accessors = accessors.with(\.leftBrace.trailingTrivia, Trivia(pieces: leftBraceTriviaPieces))
954+
}
955+
936956
if binding.initializer != nil, expansion.expandsGetSet {
937957
// The accessor block will have a leading space, but there will already be a
938958
// space between the variable and the to-be-removed initializer. Remove the
939959
// 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
960+
newBinding.accessorBlock = expansion.accessors?.with(\.leadingTrivia, [])
961+
newBinding.initializer = nil
942962
} else {
943-
node.bindings[node.bindings.startIndex].accessorBlock = expansion.accessors
963+
newBinding.accessorBlock = expansion.accessors
944964
}
965+
966+
node.bindings[node.bindings.startIndex] = newBinding
945967
}
946968

947969
return DeclSyntax(node)

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)