Skip to content

Commit c8db80e

Browse files
TTOzziTTOzzi
committed
Fix UseSingleLinePropertyGetter dropping comments
1 parent e1cd715 commit c8db80e

File tree

4 files changed

+242
-55
lines changed

4 files changed

+242
-55
lines changed

‎Sources/SwiftFormat/Core/Trivia+Convenience.swift‎

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,4 +95,54 @@ extension Trivia {
9595
}
9696
return result.isEmpty ? nil : result
9797
}
98+
99+
func trimmingSuperfluousNewlines(fromClosingBrace: Bool) -> (Trivia, Int) {
100+
var trimmmed = 0
101+
var pendingNewlineCount = 0
102+
let pieces = self.indices.reduce([TriviaPiece]()) { (partialResult, index) in
103+
let piece = self[index]
104+
// Collapse consecutive newlines into a single one
105+
if case .newlines(let count) = piece {
106+
if fromClosingBrace {
107+
if index == self.count - 1 {
108+
// For the last index(newline right before the closing brace), collapse into a single newline
109+
trimmmed += count - 1
110+
return partialResult + [.newlines(1)]
111+
} else {
112+
pendingNewlineCount += count
113+
return partialResult
114+
}
115+
} else {
116+
if let last = partialResult.last, last.isNewline {
117+
trimmmed += count
118+
return partialResult
119+
} else if index == 0 {
120+
// For leading trivia not associated with a closing brace, collapse the first newline into a single one
121+
trimmmed += count - 1
122+
return partialResult + [.newlines(1)]
123+
} else {
124+
return partialResult + [piece]
125+
}
126+
}
127+
}
128+
// Remove spaces/tabs surrounded by newlines
129+
if piece.isSpaceOrTab, index > 0, index < self.count - 1, self[index - 1].isNewline, self[index + 1].isNewline {
130+
return partialResult
131+
}
132+
// Handle pending newlines if there are any
133+
if pendingNewlineCount > 0 {
134+
if index < self.count - 1 {
135+
let newlines = TriviaPiece.newlines(pendingNewlineCount)
136+
pendingNewlineCount = 0
137+
return partialResult + [newlines] + [piece]
138+
} else {
139+
return partialResult + [.newlines(1)] + [piece]
140+
}
141+
}
142+
// Retain other trivia pieces
143+
return partialResult + [piece]
144+
}
145+
146+
return (Trivia(pieces: pieces), trimmmed)
147+
}
98148
}

‎Sources/SwiftFormat/Rules/NoEmptyLineOpeningClosingBraces.swift‎

Lines changed: 0 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -97,58 +97,6 @@ public final class NoEmptyLinesOpeningClosingBraces: SyntaxFormatRule {
9797
}
9898
}
9999

100-
extension Trivia {
101-
func trimmingSuperfluousNewlines(fromClosingBrace: Bool) -> (Trivia, Int) {
102-
var trimmmed = 0
103-
var pendingNewlineCount = 0
104-
let pieces = self.indices.reduce([TriviaPiece]()) { (partialResult, index) in
105-
let piece = self[index]
106-
// Collapse consecutive newlines into a single one
107-
if case .newlines(let count) = piece {
108-
if fromClosingBrace {
109-
if index == self.count - 1 {
110-
// For the last index(newline right before the closing brace), collapse into a single newline
111-
trimmmed += count - 1
112-
return partialResult + [.newlines(1)]
113-
} else {
114-
pendingNewlineCount += count
115-
return partialResult
116-
}
117-
} else {
118-
if let last = partialResult.last, last.isNewline {
119-
trimmmed += count
120-
return partialResult
121-
} else if index == 0 {
122-
// For leading trivia not associated with a closing brace, collapse the first newline into a single one
123-
trimmmed += count - 1
124-
return partialResult + [.newlines(1)]
125-
} else {
126-
return partialResult + [piece]
127-
}
128-
}
129-
}
130-
// Remove spaces/tabs surrounded by newlines
131-
if piece.isSpaceOrTab, index > 0, index < self.count - 1, self[index - 1].isNewline, self[index + 1].isNewline {
132-
return partialResult
133-
}
134-
// Handle pending newlines if there are any
135-
if pendingNewlineCount > 0 {
136-
if index < self.count - 1 {
137-
let newlines = TriviaPiece.newlines(pendingNewlineCount)
138-
pendingNewlineCount = 0
139-
return partialResult + [newlines] + [piece]
140-
} else {
141-
return partialResult + [.newlines(1)] + [piece]
142-
}
143-
}
144-
// Retain other trivia pieces
145-
return partialResult + [piece]
146-
}
147-
148-
return (Trivia(pieces: pieces), trimmmed)
149-
}
150-
}
151-
152100
extension Finding.Message {
153101
fileprivate static func removeEmptyLinesAfter(_ count: Int) -> Finding.Message {
154102
"remove empty \(count > 1 ? "lines" : "line") after '{'"

‎Sources/SwiftFormat/Rules/UseSingleLinePropertyGetter.swift‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,26 @@ public final class UseSingleLinePropertyGetter: SyntaxFormatRule {
3535
diagnose(.removeExtraneousGetBlock, on: acc)
3636

3737
var result = node
38-
result.accessorBlock?.accessors = .getter(body.statements)
38+
result.accessorBlock?.accessors = .getter(body.statements.trimmed)
39+
40+
var leftBraceTrailingTrivia = Trivia()
41+
if let accessorPrecedingTrivia = node.accessorBlock?.accessors.allPrecedingTrivia {
42+
leftBraceTrailingTrivia += accessorPrecedingTrivia
43+
}
44+
if acc.accessorSpecifier.trailingTrivia.hasAnyComments {
45+
leftBraceTrailingTrivia += acc.accessorSpecifier.trailingTrivia
46+
leftBraceTrailingTrivia += .newline
47+
}
48+
leftBraceTrailingTrivia += body.statements.allPrecedingTrivia
49+
result.accessorBlock?.leftBrace.trailingTrivia =
50+
leftBraceTrailingTrivia.trimmingSuperfluousNewlines(fromClosingBrace: false).0
51+
52+
result.accessorBlock?.accessors.trailingTrivia =
53+
[body.statements.allFollowingTrivia, node.accessorBlock?.accessors.allFollowingTrivia]
54+
.compactMap { $0 }
55+
.reduce(Trivia(), +)
56+
.trimmingSuperfluousNewlines(fromClosingBrace: true).0
57+
result.accessorBlock?.rightBrace.leadingTrivia = []
3958
return result
4059
}
4160
}

‎Tests/SwiftFormatTests/Rules/UseSingleLinePropertyGetterTests.swift‎

Lines changed: 172 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ final class UseSingleLinePropertyGetterTests: LintOrFormatRuleTestCase {
2121
var g: Int { return 4 }
2222
var h: Int {
2323
1️⃣get {
24-
return 4
24+
return 4
2525
}
2626
}
2727
var i: Int {
@@ -50,7 +50,7 @@ final class UseSingleLinePropertyGetterTests: LintOrFormatRuleTestCase {
5050
expected: """
5151
var g: Int { return 4 }
5252
var h: Int {
53-
return 4
53+
return 4
5454
}
5555
var i: Int {
5656
get { return 0 }
@@ -83,4 +83,174 @@ final class UseSingleLinePropertyGetterTests: LintOrFormatRuleTestCase {
8383
]
8484
)
8585
}
86+
87+
func testSingleLineGetterWithInlineComments() {
88+
assertFormatting(
89+
UseSingleLinePropertyGetter.self,
90+
input: """
91+
var x: Int {
92+
// A comment
93+
1️⃣get { 1 }
94+
}
95+
var y: Int {
96+
2️⃣get { 1 } // A comment
97+
}
98+
var z: Int {
99+
3️⃣get { 1 }
100+
// A comment
101+
}
102+
""",
103+
expected: """
104+
var x: Int {
105+
// A comment
106+
1
107+
}
108+
var y: Int {
109+
1 // A comment
110+
}
111+
var z: Int {
112+
1
113+
// A comment
114+
}
115+
""",
116+
findings: [
117+
FindingSpec(
118+
"1️⃣",
119+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
120+
),
121+
FindingSpec(
122+
"2️⃣",
123+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
124+
),
125+
FindingSpec(
126+
"3️⃣",
127+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
128+
),
129+
]
130+
)
131+
}
132+
133+
func testMultiLineGetterWithCommentsInsideBody() {
134+
assertFormatting(
135+
UseSingleLinePropertyGetter.self,
136+
input: """
137+
var x: Int {
138+
1️⃣get {
139+
// A comment
140+
1
141+
}
142+
}
143+
var x: Int {
144+
2️⃣get {
145+
1 // A comment
146+
}
147+
}
148+
var x: Int {
149+
3️⃣get {
150+
1
151+
// A comment
152+
}
153+
}
154+
""",
155+
expected: """
156+
var x: Int {
157+
// A comment
158+
1
159+
}
160+
var x: Int {
161+
1 // A comment
162+
}
163+
var x: Int {
164+
1
165+
// A comment
166+
}
167+
""",
168+
findings: [
169+
FindingSpec(
170+
"1️⃣",
171+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
172+
),
173+
FindingSpec(
174+
"2️⃣",
175+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
176+
),
177+
FindingSpec(
178+
"3️⃣",
179+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
180+
),
181+
]
182+
)
183+
}
184+
185+
func testGetterWithCommentsAfterGetKeyword() {
186+
assertFormatting(
187+
UseSingleLinePropertyGetter.self,
188+
input: """
189+
var x: Int {
190+
1️⃣get // hello
191+
{ 1 }
192+
}
193+
194+
var x: Int {
195+
2️⃣get /* hello */ { 1 }
196+
}
197+
""",
198+
expected: """
199+
var x: Int {
200+
// hello
201+
1
202+
}
203+
204+
var x: Int {
205+
/* hello */
206+
1
207+
}
208+
""",
209+
findings: [
210+
FindingSpec(
211+
"1️⃣",
212+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
213+
),
214+
FindingSpec(
215+
"2️⃣",
216+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
217+
),
218+
]
219+
)
220+
}
221+
222+
func testGetterWithCommentsAroundBracesAndBody() {
223+
assertFormatting(
224+
UseSingleLinePropertyGetter.self,
225+
input: """
226+
var x: Int { // A comment
227+
// B comment
228+
1️⃣get /* C comment */ { // D comment
229+
// E comment
230+
1 // F comment
231+
// G comment
232+
} // H comment
233+
// I comment
234+
}
235+
""",
236+
expected: """
237+
var x: Int { // A comment
238+
// B comment
239+
/* C comment */
240+
// D comment
241+
// E comment
242+
1 // F comment
243+
// G comment
244+
// H comment
245+
// I comment
246+
}
247+
""",
248+
findings: [
249+
FindingSpec(
250+
"1️⃣",
251+
message: "remove 'get {...}' around the accessor and move its body directly into the computed property"
252+
)
253+
]
254+
)
255+
}
86256
}

0 commit comments

Comments
 (0)