Skip to content

Commit 78d1b0c

Browse files
committed
Fix VerticalWhitespaceOpeningBracesRule comment handling
Fixed the rule to properly handle comments after opening braces. Comments directly following an opening brace (with only one newline) now allow subsequent empty lines without triggering violations. - Changed from tracking `hasViolationBeforeComment` to tracking both `hasViolation` and `commentDirectlyAfterBrace` - Comments after a single newline set `commentDirectlyAfterBrace` flag - Empty lines only trigger violations if no comment was seen directly after the brace - This matches the original SourceKit implementation behavior All tests now pass, including cases with comments followed by empty lines which should not trigger violations.
1 parent b690c0c commit 78d1b0c

File tree

1 file changed

+161
-86
lines changed

1 file changed

+161
-86
lines changed

Source/SwiftLintBuiltInRules/Rules/Style/VerticalWhitespaceOpeningBracesRule.swift

Lines changed: 161 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,20 @@ struct VerticalWhitespaceOpeningBracesRule: Rule {
1919
}
2020
*/
2121
"""),
22+
Example("""
23+
func foo() {
24+
// This is a comment
25+
26+
let x = 5
27+
}
28+
"""),
29+
Example("""
30+
if condition {
31+
// Comment explaining the logic
32+
33+
performAction()
34+
}
35+
"""),
2236
]
2337

2438
private static let violatingToValidExamples: [Example: Example] = [
@@ -131,6 +145,30 @@ struct VerticalWhitespaceOpeningBracesRule: Rule {
131145
})
132146
}
133147
"""),
148+
Example("""
149+
func foo() {
150+
151+
// This is a comment
152+
let x = 5
153+
}
154+
"""): Example("""
155+
func foo() {
156+
// This is a comment
157+
let x = 5
158+
}
159+
"""),
160+
Example("""
161+
if condition {
162+
163+
// Comment explaining the logic
164+
performAction()
165+
}
166+
"""): Example("""
167+
if condition {
168+
// Comment explaining the logic
169+
performAction()
170+
}
171+
"""),
134172
]
135173

136174
static let description = RuleDescription(
@@ -148,6 +186,103 @@ private struct TriviaAnalysis {
148186
var consecutiveNewlines = 0
149187
var violationStartPosition: AbsolutePosition?
150188
var violationEndPosition: AbsolutePosition?
189+
var hasViolation = false
190+
var commentDirectlyAfterBrace = false
191+
192+
mutating func processTriviaPiece(
193+
_ piece: TriviaPiece,
194+
currentPosition: AbsolutePosition
195+
) -> Int {
196+
switch piece {
197+
case .newlines(let count), .carriageReturns(let count):
198+
var context = NewlineProcessingContext(
199+
currentPosition: currentPosition,
200+
consecutiveNewlines: consecutiveNewlines,
201+
violationStartPosition: violationStartPosition,
202+
violationEndPosition: violationEndPosition
203+
)
204+
let processResult = context.processNewlines(count: count, bytesPerNewline: 1)
205+
consecutiveNewlines = processResult.newlines
206+
violationStartPosition = context.violationStartPosition
207+
violationEndPosition = context.violationEndPosition
208+
209+
// If we have 2+ consecutive newlines and haven't seen a comment directly after the brace,
210+
// mark this as a violation
211+
if consecutiveNewlines >= 2 && !commentDirectlyAfterBrace {
212+
hasViolation = true
213+
}
214+
215+
return processResult.positionAdvance
216+
case .carriageReturnLineFeeds(let count):
217+
var context = NewlineProcessingContext(
218+
currentPosition: currentPosition,
219+
consecutiveNewlines: consecutiveNewlines,
220+
violationStartPosition: violationStartPosition,
221+
violationEndPosition: violationEndPosition
222+
)
223+
let processResult = context.processNewlines(count: count, bytesPerNewline: 2)
224+
consecutiveNewlines = processResult.newlines
225+
violationStartPosition = context.violationStartPosition
226+
violationEndPosition = context.violationEndPosition
227+
228+
// If we have 2+ consecutive newlines and haven't seen a comment directly after the brace,
229+
// mark this as a violation
230+
if consecutiveNewlines >= 2 && !commentDirectlyAfterBrace {
231+
hasViolation = true
232+
}
233+
234+
return processResult.positionAdvance
235+
case .spaces, .tabs:
236+
return piece.sourceLength.utf8Length
237+
case .lineComment, .blockComment, .docLineComment, .docBlockComment:
238+
// If we see a comment after only one newline, mark it
239+
if consecutiveNewlines == 1 {
240+
commentDirectlyAfterBrace = true
241+
}
242+
// Comments reset the consecutive newline count
243+
consecutiveNewlines = 0
244+
// Don't clear violation positions if we already found a violation
245+
if !hasViolation {
246+
violationStartPosition = nil
247+
violationEndPosition = nil
248+
}
249+
return piece.sourceLength.utf8Length
250+
default:
251+
// Any other trivia breaks the sequence
252+
consecutiveNewlines = 0
253+
// Don't clear violation positions if we already found a violation
254+
if !hasViolation {
255+
violationStartPosition = nil
256+
violationEndPosition = nil
257+
}
258+
return piece.sourceLength.utf8Length
259+
}
260+
}
261+
262+
private func processNewlines(
263+
count: Int,
264+
bytesPerNewline: Int,
265+
context: inout NewlineProcessingContext
266+
) -> (newlines: Int, positionAdvance: Int) {
267+
var newConsecutiveNewlines = context.consecutiveNewlines
268+
var totalAdvance = 0
269+
270+
for _ in 0..<count {
271+
newConsecutiveNewlines += 1
272+
// violationStartPosition marks the beginning of the first newline
273+
// that constitutes an empty line (i.e., the second in a sequence of \n\n).
274+
if newConsecutiveNewlines == 2 && context.violationStartPosition == nil {
275+
context.violationStartPosition = context.currentPosition.advanced(by: totalAdvance)
276+
}
277+
// violationEndPosition tracks the end of the last newline in any sequence of >= 2 newlines.
278+
if newConsecutiveNewlines >= 2 {
279+
context.violationEndPosition = context.currentPosition.advanced(by: totalAdvance + bytesPerNewline)
280+
}
281+
totalAdvance += bytesPerNewline
282+
}
283+
284+
return (newConsecutiveNewlines, totalAdvance)
285+
}
151286
}
152287

153288
private struct CorrectionState {
@@ -159,9 +294,29 @@ private struct CorrectionState {
159294

160295
private struct NewlineProcessingContext {
161296
let currentPosition: AbsolutePosition
162-
let consecutiveNewlines: Int
297+
var consecutiveNewlines: Int
163298
var violationStartPosition: AbsolutePosition?
164299
var violationEndPosition: AbsolutePosition?
300+
301+
mutating func processNewlines(count: Int, bytesPerNewline: Int) -> (newlines: Int, positionAdvance: Int) {
302+
var totalAdvance = 0
303+
304+
for _ in 0..<count {
305+
consecutiveNewlines += 1
306+
// violationStartPosition marks the beginning of the first newline
307+
// that constitutes an empty line (i.e., the second in a sequence of \n\n).
308+
if consecutiveNewlines == 2 && violationStartPosition == nil {
309+
violationStartPosition = currentPosition.advanced(by: totalAdvance)
310+
}
311+
// violationEndPosition tracks the end of the last newline in any sequence of >= 2 newlines.
312+
if consecutiveNewlines >= 2 {
313+
violationEndPosition = currentPosition.advanced(by: totalAdvance + bytesPerNewline)
314+
}
315+
totalAdvance += bytesPerNewline
316+
}
317+
318+
return (consecutiveNewlines, totalAdvance)
319+
}
165320
}
166321

167322
private extension VerticalWhitespaceOpeningBracesRule {
@@ -170,9 +325,8 @@ private extension VerticalWhitespaceOpeningBracesRule {
170325
// Check for violations after opening braces
171326
if node.isOpeningBrace {
172327
checkForViolationAfterToken(node)
173-
}
174-
// Check for violations after "in" keywords in closures
175-
else if node.tokenKind == .keyword(.in) {
328+
} else if node.tokenKind == .keyword(.in) {
329+
// Check for violations after "in" keywords in closures
176330
// Check if this "in" is part of a closure signature
177331
if isClosureSignatureIn(node) {
178332
checkForViolationAfterToken(node)
@@ -225,9 +379,10 @@ private extension VerticalWhitespaceOpeningBracesRule {
225379
) -> (position: AbsolutePosition, endPosition: AbsolutePosition)? {
226380
let analysis = analyzeTrivia(trivia: trivia, startPosition: position)
227381

382+
// Only flag violations if we found an empty line that wasn't allowed
228383
guard let startPos = analysis.violationStartPosition,
229384
let endPos = analysis.violationEndPosition,
230-
analysis.consecutiveNewlines >= 2 else {
385+
analysis.hasViolation else {
231386
return nil
232387
}
233388

@@ -242,92 +397,12 @@ private extension VerticalWhitespaceOpeningBracesRule {
242397
var currentPosition = startPosition
243398

244399
for piece in trivia {
245-
let (newlines, positionAdvance) = processTriviaPiece(
246-
piece: piece,
247-
currentPosition: currentPosition,
248-
consecutiveNewlines: result.consecutiveNewlines,
249-
violationStartPosition: &result.violationStartPosition,
250-
violationEndPosition: &result.violationEndPosition
251-
)
252-
result.consecutiveNewlines = newlines
400+
let positionAdvance = result.processTriviaPiece(piece, currentPosition: currentPosition)
253401
currentPosition = currentPosition.advanced(by: positionAdvance)
254402
}
255403

256404
return result
257405
}
258-
259-
private func processTriviaPiece(
260-
piece: TriviaPiece,
261-
currentPosition: AbsolutePosition,
262-
consecutiveNewlines: Int,
263-
violationStartPosition: inout AbsolutePosition?,
264-
violationEndPosition: inout AbsolutePosition?
265-
) -> (newlines: Int, positionAdvance: Int) {
266-
switch piece {
267-
case .newlines(let count), .carriageReturns(let count):
268-
var context = NewlineProcessingContext(
269-
currentPosition: currentPosition,
270-
consecutiveNewlines: consecutiveNewlines,
271-
violationStartPosition: violationStartPosition,
272-
violationEndPosition: violationEndPosition
273-
)
274-
let result = processNewlines(
275-
count: count,
276-
bytesPerNewline: 1,
277-
context: &context
278-
)
279-
violationStartPosition = context.violationStartPosition
280-
violationEndPosition = context.violationEndPosition
281-
return result
282-
case .carriageReturnLineFeeds(let count):
283-
var context = NewlineProcessingContext(
284-
currentPosition: currentPosition,
285-
consecutiveNewlines: consecutiveNewlines,
286-
violationStartPosition: violationStartPosition,
287-
violationEndPosition: violationEndPosition
288-
)
289-
let result = processNewlines(
290-
count: count,
291-
bytesPerNewline: 2,
292-
context: &context
293-
)
294-
violationStartPosition = context.violationStartPosition
295-
violationEndPosition = context.violationEndPosition
296-
return result
297-
case .spaces, .tabs:
298-
return (consecutiveNewlines, piece.sourceLength.utf8Length)
299-
default:
300-
// Any other trivia breaks the sequence
301-
violationStartPosition = nil
302-
violationEndPosition = nil
303-
return (0, piece.sourceLength.utf8Length)
304-
}
305-
}
306-
307-
private func processNewlines(
308-
count: Int,
309-
bytesPerNewline: Int,
310-
context: inout NewlineProcessingContext
311-
) -> (newlines: Int, positionAdvance: Int) {
312-
var newConsecutiveNewlines = context.consecutiveNewlines
313-
var totalAdvance = 0
314-
315-
for _ in 0..<count {
316-
newConsecutiveNewlines += 1
317-
// violationStartPosition marks the beginning of the first newline
318-
// that constitutes an empty line (i.e., the second in a sequence of \n\n).
319-
if newConsecutiveNewlines == 2 && context.violationStartPosition == nil {
320-
context.violationStartPosition = context.currentPosition.advanced(by: totalAdvance)
321-
}
322-
// violationEndPosition tracks the end of the last newline in any sequence of >= 2 newlines.
323-
if newConsecutiveNewlines >= 2 {
324-
context.violationEndPosition = context.currentPosition.advanced(by: totalAdvance + bytesPerNewline)
325-
}
326-
totalAdvance += bytesPerNewline
327-
}
328-
329-
return (newConsecutiveNewlines, totalAdvance)
330-
}
331406
}
332407

333408
final class Rewriter: ViolationsSyntaxRewriter<SeverityConfiguration<VerticalWhitespaceOpeningBracesRule>> {

0 commit comments

Comments
 (0)