Skip to content

Commit 1e25cf6

Browse files
pennigpennig
authored
Migrate VerticalWhitespaceRule from SourceKit to SwiftSyntax (#6103)
* Migrate VerticalWhitespaceRule from SourceKit to SwiftSyntax * Adds tests for new horizontal whitespace behavior
1 parent 6d5af5f commit 1e25cf6

File tree

4 files changed

+121
-104
lines changed

4 files changed

+121
-104
lines changed

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222
and fewer false positives.
2323
[JP Simard](https://github.com/jpsim)
2424

25+
* Migrate `vertical_whitespace` rule from SourceKit to SwiftSyntax for improved performance.
26+
[Matt Pennig](https://github.com/pennig)
27+
2528
### Bug Fixes
2629

2730
* Improved error reporting when SwiftLint exits, because of an invalid configuration file

‎Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/VerticalWhitespaceConfiguration.swift‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,17 @@ import SwiftLintCore
44
struct VerticalWhitespaceConfiguration: SeverityBasedRuleConfiguration {
55
typealias Parent = VerticalWhitespaceRule
66

7+
static let defaultDescriptionReason = "Limit vertical whitespace to a single empty line"
8+
79
@ConfigurationElement(key: "severity")
810
private(set) var severityConfiguration = SeverityConfiguration<Parent>(.warning)
911
@ConfigurationElement(key: "max_empty_lines")
1012
private(set) var maxEmptyLines = 1
13+
14+
var configuredDescriptionReason: String {
15+
guard maxEmptyLines == 1 else {
16+
return "Limit vertical whitespace to maximum \(maxEmptyLines) empty lines"
17+
}
18+
return Self.defaultDescriptionReason
19+
}
1120
}

‎Source/SwiftLintBuiltInRules/Rules/Style/VerticalWhitespaceRule.swift‎

Lines changed: 104 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,146 +1,147 @@
11
import Foundation
2-
import SourceKittenFramework
2+
import SwiftSyntax
33

4-
private let defaultDescriptionReason = "Limit vertical whitespace to a single empty line"
5-
6-
struct VerticalWhitespaceRule: CorrectableRule {
4+
@SwiftSyntaxRule(explicitRewriter: true, correctable: true)
5+
struct VerticalWhitespaceRule: Rule {
76
var configuration = VerticalWhitespaceConfiguration()
87

98
static let description = RuleDescription(
109
identifier: "vertical_whitespace",
1110
name: "Vertical Whitespace",
12-
description: defaultDescriptionReason + ".",
11+
description: VerticalWhitespaceConfiguration.defaultDescriptionReason,
1312
kind: .style,
1413
nonTriggeringExamples: [
1514
Example("let abc = 0\n"),
1615
Example("let abc = 0\n\n"),
1716
Example("/* bcs \n\n\n\n*/"),
1817
Example("// bca \n\n"),
18+
Example("class CCCC {\n \n}"),
1919
],
2020
triggeringExamples: [
2121
Example("let aaaa = 0\n\n\n"),
2222
Example("struct AAAA {}\n\n\n\n"),
2323
Example("class BBBB {}\n\n\n"),
24+
Example("class CCCC {\n \n \n}"),
2425
],
2526
corrections: [
2627
Example("let b = 0\n\n\nclass AAA {}\n"): Example("let b = 0\n\nclass AAA {}\n"),
2728
Example("let c = 0\n\n\nlet num = 1\n"): Example("let c = 0\n\nlet num = 1\n"),
2829
Example("// bca \n\n\n"): Example("// bca \n\n"),
30+
Example("class CCCC {\n \n \n \n}"): Example("class CCCC {\n \n}"),
2931
] // End of line autocorrections are handled by Trailing Newline Rule.
3032
)
33+
}
3134

32-
private var configuredDescriptionReason: String {
33-
guard configuration.maxEmptyLines == 1 else {
34-
return "Limit vertical whitespace to maximum \(configuration.maxEmptyLines) empty lines"
35-
}
36-
return defaultDescriptionReason
37-
}
38-
39-
func validate(file: SwiftLintFile) -> [StyleViolation] {
40-
let linesSections = violatingLineSections(in: file)
41-
guard linesSections.isNotEmpty else {
42-
return []
43-
}
44-
45-
return linesSections.map { eachLastLine, eachSectionCount in
46-
StyleViolation(
47-
ruleDescription: Self.description,
48-
severity: configuration.severityConfiguration.severity,
49-
location: Location(file: file.path, line: eachLastLine.index),
50-
reason: configuredDescriptionReason + "; currently \(eachSectionCount + 1)"
51-
)
52-
}
53-
}
54-
55-
private typealias LineSection = (lastLine: Line, linesToRemove: Int)
35+
private extension VerticalWhitespaceRule {
36+
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
37+
override func visit(_ token: TokenSyntax) -> SyntaxVisitorContinueKind {
38+
// The strategy here is to keep track of the position of the _first_ violating newline
39+
// in each consecutive run, and report the violation when the run _ends_.
5640

57-
private func violatingLineSections(in file: SwiftLintFile) -> [LineSection] {
58-
let nonSpaceRegex = regex("\\S", options: [])
59-
let filteredLines = file.lines.filter {
60-
nonSpaceRegex.firstMatch(in: file.contents, options: [], range: $0.range) == nil
61-
}
62-
63-
guard filteredLines.isNotEmpty else {
64-
return []
65-
}
41+
if token.leadingTrivia.isEmpty {
42+
return .visitChildren
43+
}
6644

67-
let blankLinesSections = extractSections(from: filteredLines)
45+
var consecutiveNewlines = 0
46+
var currentPosition = token.position
47+
var violationPosition: AbsolutePosition?
48+
49+
func process(_ count: Int, _ offset: Int) {
50+
for _ in 0..<count {
51+
if consecutiveNewlines > configuration.maxEmptyLines && violationPosition == nil {
52+
violationPosition = currentPosition
53+
}
54+
consecutiveNewlines += 1
55+
currentPosition = currentPosition.advanced(by: offset)
56+
}
57+
}
6858

69-
// filtering out violations in comments and strings
70-
let stringAndComments = SyntaxKind.commentAndStringKinds
71-
let syntaxMap = file.syntaxMap
72-
let result = blankLinesSections.compactMap { eachSection -> (lastLine: Line, linesToRemove: Int)? in
73-
guard let lastLine = eachSection.last else {
74-
return nil
59+
for piece in token.leadingTrivia {
60+
switch piece {
61+
case .newlines(let count), .carriageReturns(let count), .formfeeds(let count), .verticalTabs(let count):
62+
process(count, 1)
63+
case .carriageReturnLineFeeds(let count):
64+
process(count, 2) // CRLF is 2 bytes
65+
case .spaces, .tabs:
66+
currentPosition += piece.sourceLength
67+
default:
68+
if let violationPosition {
69+
report(violationPosition, consecutiveNewlines)
70+
}
71+
violationPosition = nil
72+
consecutiveNewlines = 0
73+
currentPosition += piece.sourceLength
74+
}
7575
}
76-
let kindInSection = syntaxMap.kinds(inByteRange: lastLine.byteRange)
77-
if stringAndComments.isDisjoint(with: kindInSection) {
78-
return (lastLine, eachSection.count)
76+
if let violationPosition {
77+
report(violationPosition, consecutiveNewlines)
7978
}
8079

81-
return nil
80+
return .visitChildren
8281
}
8382

84-
return result.filter { $0.linesToRemove >= configuration.maxEmptyLines }
85-
}
86-
87-
private func extractSections(from lines: [Line]) -> [[Line]] {
88-
var blankLinesSections = [[Line]]()
89-
var lineSection = [Line]()
90-
91-
var previousIndex = 0
92-
for (index, line) in lines.enumerated() {
93-
let previousLine: Line = lines[previousIndex]
94-
if previousLine.index + 1 == line.index {
95-
lineSection.append(line)
96-
} else if lineSection.isNotEmpty {
97-
blankLinesSections.append(lineSection)
98-
lineSection.removeAll()
99-
}
100-
previousIndex = index
101-
}
102-
if lineSection.isNotEmpty {
103-
blankLinesSections.append(lineSection)
83+
private func report(_ position: AbsolutePosition, _ newlines: Int) {
84+
violations.append(ReasonedRuleViolation(
85+
position: position,
86+
reason: configuration.configuredDescriptionReason + "; currently \(newlines - 1)"
87+
))
10488
}
105-
106-
return blankLinesSections
10789
}
10890

109-
func correct(file: SwiftLintFile) -> Int {
110-
let linesSections = violatingLineSections(in: file)
111-
if linesSections.isEmpty {
112-
return 0
113-
}
114-
115-
var indexOfLinesToDelete = [Int]()
116-
117-
for section in linesSections {
118-
let linesToRemove = section.linesToRemove - configuration.maxEmptyLines + 1
119-
let start = section.lastLine.index - linesToRemove
120-
indexOfLinesToDelete.append(contentsOf: start..<section.lastLine.index)
121-
}
91+
final class Rewriter: ViolationsSyntaxRewriter<ConfigurationType> {
92+
override func visit(_ token: TokenSyntax) -> TokenSyntax {
93+
var result = [TriviaPiece]()
94+
var pendingWhitespace = [TriviaPiece]()
95+
var consecutiveNewlines = 0
96+
97+
func process(_ count: Int, _ create: (Int) -> TriviaPiece) {
98+
let linesToPreserve = min(count, max(0, configuration.maxEmptyLines + 1 - consecutiveNewlines))
99+
consecutiveNewlines += count
100+
101+
if count > linesToPreserve {
102+
self.numberOfCorrections += count - linesToPreserve
103+
}
104+
105+
if linesToPreserve > 0 {
106+
// We can still add this piece, even if we adjusted its count lower.
107+
// Pull in any pending whitespace along with it.
108+
result.append(contentsOf: pendingWhitespace)
109+
result.append(create(linesToPreserve))
110+
pendingWhitespace.removeAll()
111+
} else {
112+
// We're now in violation. Dump pending whitespace so it's excluded from the result.
113+
pendingWhitespace.removeAll()
114+
}
115+
}
122116

123-
var correctedLines = [String]()
124-
var numberOfCorrections = 0
125-
for currentLine in file.lines {
126-
// Doesn't correct lines where rule is disabled
127-
if file.ruleEnabled(violatingRanges: [currentLine.range], for: self).isEmpty {
128-
correctedLines.append(currentLine.content)
129-
continue
117+
for piece in token.leadingTrivia {
118+
switch piece {
119+
case .newlines(let count):
120+
process(count, TriviaPiece.newlines)
121+
case .carriageReturns(let count):
122+
process(count, TriviaPiece.carriageReturns)
123+
case .carriageReturnLineFeeds(let count):
124+
process(count, TriviaPiece.carriageReturnLineFeeds)
125+
case .formfeeds(let count):
126+
process(count, TriviaPiece.formfeeds)
127+
case .verticalTabs(let count):
128+
process(count, TriviaPiece.verticalTabs)
129+
case .spaces, .tabs:
130+
pendingWhitespace.append(piece)
131+
default:
132+
// Reset and pull in pending whitespace
133+
consecutiveNewlines = 0
134+
result.append(contentsOf: pendingWhitespace)
135+
result.append(piece)
136+
pendingWhitespace.removeAll()
137+
}
130138
}
131-
// removes lines by skipping them from correctedLines
132-
if Set(indexOfLinesToDelete).contains(currentLine.index) {
133-
// reports every line that is being deleted
134-
numberOfCorrections += 1
135-
continue // skips line
139+
// Pull in any remaining pending whitespace
140+
if !pendingWhitespace.isEmpty {
141+
result.append(contentsOf: pendingWhitespace)
136142
}
137-
// all lines that pass get added to final output file
138-
correctedLines.append(currentLine.content)
139-
}
140-
// converts lines back to file and adds trailing line
141-
if numberOfCorrections > 0 {
142-
file.write(correctedLines.joined(separator: "\n") + "\n")
143+
144+
return super.visit(token.with(\.leadingTrivia, Trivia(pieces: result)))
143145
}
144-
return numberOfCorrections
145146
}
146147
}

‎Tests/BuiltInRulesTests/VerticalWhitespaceRuleTests.swift‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@ final class VerticalWhitespaceRuleTests: SwiftLintTestCase {
99
// Test with custom `max_empty_lines`
1010
let maxEmptyLinesDescription = VerticalWhitespaceRule.description
1111
.with(nonTriggeringExamples: [Example("let aaaa = 0\n\n\n")])
12-
.with(triggeringExamples: [Example("struct AAAA {}\n\n\n\n")])
12+
.with(triggeringExamples: [
13+
Example("struct AAAA {}\n\n\n\n"),
14+
Example("class BBBB {\n \n \n \n}"),
15+
])
1316
.with(corrections: [:])
1417

1518
verifyRule(maxEmptyLinesDescription,
@@ -23,6 +26,7 @@ final class VerticalWhitespaceRuleTests: SwiftLintTestCase {
2326
.with(corrections: [
2427
Example("let b = 0\n\n↓\n↓\n↓\n\nclass AAA {}\n"): Example("let b = 0\n\n\nclass AAA {}\n"),
2528
Example("let b = 0\n\n\nclass AAA {}\n"): Example("let b = 0\n\n\nclass AAA {}\n"),
29+
Example("class BB {\n \n \n↓ \n let b = 0\n}\n"): Example("class BB {\n \n \n let b = 0\n}\n"),
2630
])
2731

2832
verifyRule(maxEmptyLinesDescription,

0 commit comments

Comments
 (0)