Skip to content

Commit 3a1a472

Browse files
JP Simardjpsim
JP Simard
authored and
jpsim
committed
Migrate MultilineParametersBracketsRule from SourceKit to SwiftSyntax
## Summary Convert MultilineParametersBracketsRule to use SwiftSyntax instead of SourceKit for improved performance and better detection of multiline parameter formatting violations. ## Key Technical Improvements - **Enhanced multiline detection** distinguishing between structurally multiline parameters and parameters with multiline default values - **Accurate position reporting** using SwiftSyntax's precise token locations - **Better handling of default values** by focusing on parameter structure rather than content - **Improved performance** using visitor pattern over regex-based SourceKit analysis - **Reduced false positives** for single parameters with multiline default values ## Migration Details - Replaced `ASTRule` with `@SwiftSyntaxRule(optIn: true)` annotation - Implemented `ViolationsSyntaxVisitor` pattern for function and initializer parameter analysis - Added helper methods to extract significant tokens (name/type) excluding default values - Converted regex-based bracket detection to SwiftSyntax position comparisons - Maintained full compatibility with existing rule behavior and test cases
1 parent f7f3caa commit 3a1a472

File tree

2 files changed

+135
-77
lines changed

2 files changed

+135
-77
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
* `file_header`
3636
* `file_length`
3737
* `line_length`
38+
* `multiline_parameters_brackets`
3839
* `trailing_whitespace`
3940
* `vertical_whitespace`
4041
<!-- Keep empty line to have the contributors on a separate line. -->

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

Lines changed: 134 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
1-
import Foundation
2-
import SourceKittenFramework
1+
import SwiftLintCore
2+
import SwiftSyntax
33

4-
struct MultilineParametersBracketsRule: OptInRule {
4+
@SwiftSyntaxRule(optIn: true)
5+
struct MultilineParametersBracketsRule: Rule {
56
var configuration = SeverityConfiguration<Self>(.warning)
67

78
static let description = RuleDescription(
@@ -89,97 +90,153 @@ struct MultilineParametersBracketsRule: OptInRule {
8990
"""),
9091
]
9192
)
93+
}
9294

93-
func validate(file: SwiftLintFile) -> [StyleViolation] {
94-
violations(in: file.structureDictionary, file: file)
95-
}
95+
private extension MultilineParametersBracketsRule {
96+
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
97+
override func visitPost(_ node: FunctionDeclSyntax) {
98+
checkViolations(for: node.signature.parameterClause)
99+
}
96100

97-
private func violations(in substructure: SourceKittenDictionary, file: SwiftLintFile) -> [StyleViolation] {
98-
var violations = [StyleViolation]()
99-
100-
// find violations at current level
101-
if let kind = substructure.declarationKind,
102-
SwiftDeclarationKind.functionKinds.contains(kind) {
103-
guard
104-
let nameOffset = substructure.nameOffset,
105-
let nameLength = substructure.nameLength,
106-
case let nameByteRange = ByteRange(location: nameOffset, length: nameLength),
107-
let functionName = file.stringView.substringWithByteRange(nameByteRange)
108-
else {
109-
return []
110-
}
111-
112-
let parameters = substructure.substructure.filter { $0.declarationKind == .varParameter }
113-
let parameterBodies = parameters.compactMap { $0.content(in: file) }
114-
let parametersNewlineCount = parameterBodies.map { body in
115-
body.countOccurrences(of: "\n")
116-
}.reduce(0, +)
117-
let declarationNewlineCount = functionName.countOccurrences(of: "\n")
118-
let isMultiline = declarationNewlineCount > parametersNewlineCount
119-
120-
if isMultiline && parameters.isNotEmpty {
121-
if let openingBracketViolation = openingBracketViolation(parameters: parameters, file: file) {
122-
violations.append(openingBracketViolation)
123-
}
101+
override func visitPost(_ node: InitializerDeclSyntax) {
102+
checkViolations(for: node.signature.parameterClause)
103+
}
124104

125-
if let closingBracketViolation = closingBracketViolation(parameters: parameters, file: file) {
126-
violations.append(closingBracketViolation)
127-
}
105+
private func significantStartToken(for parameter: FunctionParameterSyntax) -> TokenSyntax? {
106+
// The first name token (external or internal)
107+
if parameter.firstName.tokenKind == .wildcard, let secondName = parameter.secondName {
108+
return secondName
128109
}
110+
return parameter.firstName
129111
}
130112

131-
// find violations at deeper levels
132-
for substructure in substructure.substructure {
133-
violations += self.violations(in: substructure, file: file)
113+
private func significantEndToken(for parameter: FunctionParameterSyntax) -> TokenSyntax? {
114+
// End of ellipsis, or type, or name (in that order of preference)
115+
if let ellipsis = parameter.ellipsis {
116+
return ellipsis
117+
}
118+
// Type is not optional, so directly use it
119+
return parameter.type.lastToken(viewMode: .sourceAccurate) // Gets the very last token of the type
134120
}
135121

136-
return violations
137-
}
122+
private func checkViolations(
123+
for parameterClause: FunctionParameterClauseSyntax
124+
) {
125+
guard parameterClause.parameters.isNotEmpty else {
126+
return
127+
}
138128

139-
private func openingBracketViolation(parameters: [SourceKittenDictionary],
140-
file: SwiftLintFile) -> StyleViolation? {
141-
guard
142-
let firstParamByteRange = parameters.first?.byteRange,
143-
let firstParamRange = file.stringView.byteRangeToNSRange(firstParamByteRange)
144-
else {
145-
return nil
146-
}
129+
let parameters = parameterClause.parameters
130+
let leftParen = parameterClause.leftParen
131+
let rightParen = parameterClause.rightParen
147132

148-
let prefix = file.stringView.nsString.substring(to: firstParamRange.lowerBound)
149-
let invalidRegex = regex("\\([ \\t]*\\z")
133+
let leftParenLine = locationConverter.location(for: leftParen.positionAfterSkippingLeadingTrivia).line
134+
let rightParenLine = locationConverter.location(for: rightParen.positionAfterSkippingLeadingTrivia).line
150135

151-
guard let invalidMatch = invalidRegex.firstMatch(in: prefix, options: [], range: prefix.fullNSRange) else {
152-
return nil
153-
}
136+
guard let firstParam = parameters.first, let lastParam = parameters.last else { return }
154137

155-
return StyleViolation(
156-
ruleDescription: Self.description,
157-
severity: configuration.severity,
158-
location: Location(file: file, characterOffset: invalidMatch.range.location + 1)
159-
)
160-
}
138+
guard let firstParamStartToken = significantStartToken(for: firstParam),
139+
let lastParamEndToken = significantEndToken(for: lastParam) else {
140+
return // Should not happen with valid parameters
141+
}
142+
143+
let firstParamSignificantStartLine = locationConverter.location(
144+
for: firstParamStartToken.positionAfterSkippingLeadingTrivia
145+
).line
146+
let lastParamSignificantEndLine = locationConverter.location(
147+
for: lastParamEndToken.endPositionBeforeTrailingTrivia
148+
).line
149+
150+
guard isStructurallyMultiline(
151+
parameters: parameters,
152+
firstParam: firstParam,
153+
firstParamStartLine: firstParamSignificantStartLine,
154+
lastParamEndLine: lastParamSignificantEndLine,
155+
leftParenLine: leftParenLine
156+
) else {
157+
return // Not structurally multiline, so this rule doesn't apply.
158+
}
159+
160+
// Check if opening paren has first parameter on same line
161+
if leftParenLine == firstParamSignificantStartLine {
162+
violations.append(
163+
ReasonedRuleViolation(
164+
position: firstParam.positionAfterSkippingLeadingTrivia,
165+
reason: "Opening parenthesis should be on a separate line when using multiline parameters"
166+
)
167+
)
168+
}
161169

162-
private func closingBracketViolation(parameters: [SourceKittenDictionary],
163-
file: SwiftLintFile) -> StyleViolation? {
164-
guard
165-
let lastParamByteRange = parameters.last?.byteRange,
166-
let lastParamRange = file.stringView.byteRangeToNSRange(lastParamByteRange)
167-
else {
168-
return nil
170+
// Check if closing paren is on same line as last parameter's significant part
171+
if rightParenLine == lastParamSignificantEndLine {
172+
violations.append(
173+
ReasonedRuleViolation(
174+
position: rightParen.positionAfterSkippingLeadingTrivia,
175+
reason: "Closing parenthesis should be on a separate line when using multiline parameters"
176+
)
177+
)
178+
}
169179
}
170180

171-
let suffix = file.stringView.nsString.substring(from: lastParamRange.upperBound)
172-
let invalidRegex = regex("\\A[ \\t]*\\)")
181+
private func isStructurallyMultiline(
182+
parameters: FunctionParameterListSyntax,
183+
firstParam: FunctionParameterSyntax,
184+
firstParamStartLine: Int,
185+
lastParamEndLine: Int,
186+
leftParenLine: Int
187+
) -> Bool {
188+
// First check if parameters themselves span multiple lines
189+
if parameters.count > 1 && areParametersOnDifferentLines(parameters: parameters, firstParam: firstParam) {
190+
return true
191+
}
173192

174-
guard let invalidMatch = invalidRegex.firstMatch(in: suffix, options: [], range: suffix.fullNSRange) else {
175-
return nil
193+
// Also check if first parameter starts on a different line than opening paren
194+
if firstParamStartLine > leftParenLine {
195+
return true
196+
}
197+
198+
// Also check if parameters span from opening to closing paren across lines
199+
if firstParamStartLine != lastParamEndLine {
200+
return true
201+
}
202+
203+
return false
176204
}
177205

178-
let characterOffset = lastParamRange.upperBound + invalidMatch.range.upperBound - 1
179-
return StyleViolation(
180-
ruleDescription: Self.description,
181-
severity: configuration.severity,
182-
location: Location(file: file, characterOffset: characterOffset)
183-
)
206+
private func areParametersOnDifferentLines(
207+
parameters: FunctionParameterListSyntax,
208+
firstParam: FunctionParameterSyntax
209+
) -> Bool {
210+
var previousParamSignificantEndLine = -1
211+
if let firstParamEndToken = significantEndToken(for: firstParam) {
212+
previousParamSignificantEndLine = locationConverter.location(
213+
for: firstParamEndToken.endPositionBeforeTrailingTrivia
214+
).line
215+
}
216+
217+
for (index, parameter) in parameters.enumerated() {
218+
if index == 0 { continue } // Already used firstParam for initialization
219+
220+
guard let currentParamStartToken = significantStartToken(for: parameter) else { continue }
221+
let currentParamSignificantStartLine = locationConverter.location(
222+
for: currentParamStartToken.positionAfterSkippingLeadingTrivia
223+
).line
224+
225+
if currentParamSignificantStartLine > previousParamSignificantEndLine {
226+
return true
227+
}
228+
229+
if let currentParamEndToken = significantEndToken(for: parameter) {
230+
previousParamSignificantEndLine = locationConverter.location(
231+
for: currentParamEndToken.endPositionBeforeTrailingTrivia
232+
).line
233+
} else {
234+
// If a parameter somehow doesn't have a significant end,
235+
// fallback to its start line to avoid issues in comparison.
236+
previousParamSignificantEndLine = currentParamSignificantStartLine
237+
}
238+
}
239+
return false
240+
}
184241
}
185242
}

0 commit comments

Comments
 (0)