Skip to content

Commit af38476

Browse files
GandaLF2006GandaLF2006
committed
Fix multiline_call_arguments pattern-matching false positives
1 parent eddb35d commit af38476

4 files changed

Lines changed: 768 additions & 81 deletions

File tree

‎Source/SwiftLintBuiltInRules/Rules/Lint/MultilineCallArgumentsRule.swift‎

Lines changed: 150 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -5,105 +5,179 @@ import SwiftSyntax
55
struct MultilineCallArgumentsRule: Rule {
66
var configuration = MultilineCallArgumentsConfiguration()
77

8+
enum Reason {
9+
static let singleLineMultipleArgumentsNotAllowed =
10+
"Single-line calls with multiple arguments are not allowed"
11+
12+
static func tooManyArgumentsOnSingleLine(max: Int) -> String {
13+
"Too many arguments on a single line (max: \(max))"
14+
}
15+
16+
static let eachArgumentMustStartOnOwnLine =
17+
"In multi-line calls, each argument must start on its own line"
18+
}
19+
820
static let description = RuleDescription(
921
identifier: "multiline_call_arguments",
1022
name: "Multiline Call Arguments",
11-
description: "Call should have each parameter on a separate line",
23+
description: """
24+
Enforces one-argument-per-line for multi-line calls; \
25+
optionally limits or forbids multi-argument single-line calls via configuration
26+
""",
1227
kind: .style,
13-
nonTriggeringExamples: [
14-
Example("""
15-
foo(
16-
param1: "param1",
17-
param2: false,
18-
param3: []
19-
)
20-
""",
21-
configuration: ["max_number_of_single_line_parameters": 2]),
22-
Example("""
23-
foo(param1: 1,
24-
param2: false,
25-
param3: [])
26-
""",
27-
configuration: ["max_number_of_single_line_parameters": 1]),
28-
Example(
29-
"foo(param1: 1, param2: false)",
30-
configuration: ["max_number_of_single_line_parameters": 2]),
31-
Example(
32-
"Enum.foo(param1: 1, param2: false)",
33-
configuration: ["max_number_of_single_line_parameters": 2]),
34-
Example("foo(param1: 1)", configuration: ["allows_single_line": false]),
35-
Example("Enum.foo(param1: 1)", configuration: ["allows_single_line": false]),
36-
Example(
37-
"Enum.foo(param1: 1, param2: 2, param3: 3)",
38-
configuration: ["allows_single_line": true]),
39-
Example("""
40-
foo(
41-
param1: 1,
42-
param2: 2,
43-
param3: 3
44-
)
45-
""",
46-
configuration: ["allows_single_line": false]),
47-
],
48-
triggeringExamples: [
49-
Example(
50-
"↓foo(param1: 1, param2: false, param3: [])",
51-
configuration: ["max_number_of_single_line_parameters": 2]),
52-
Example(
53-
"↓Enum.foo(param1: 1, param2: false, param3: [])",
54-
configuration: ["max_number_of_single_line_parameters": 2]),
55-
Example("""
56-
↓foo(param1: 1, param2: false,
57-
param3: [])
58-
""",
59-
configuration: ["max_number_of_single_line_parameters": 3]),
60-
Example("""
61-
↓Enum.foo(param1: 1, param2: false,
62-
param3: [])
63-
""",
64-
configuration: ["max_number_of_single_line_parameters": 3]),
65-
Example("↓foo(param1: 1, param2: false)", configuration: ["allows_single_line": false]),
66-
Example("↓Enum.foo(param1: 1, param2: false)", configuration: ["allows_single_line": false]),
67-
]
28+
nonTriggeringExamples: MultilineCallArgumentsRuleExamples.nonTriggeringExamples,
29+
triggeringExamples: MultilineCallArgumentsRuleExamples.triggeringExamples
6830
)
6931
}
7032

7133
private extension MultilineCallArgumentsRule {
7234
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
35+
/// Cache line lookups by utf8Offset (stable, cheap key)
36+
private var lineCache: [Int: Int] = [:]
37+
38+
override init(configuration: ConfigurationType, file: SwiftLintFile) {
39+
super.init(configuration: configuration, file: file)
40+
41+
// Most files trigger O(10–100) unique line lookups for this rule.
42+
// Reserving a small initial capacity reduces rehashing; it is NOT a hard limit.
43+
lineCache.reserveCapacity(64)
44+
}
45+
7346
override func visitPost(_ node: FunctionCallExprSyntax) {
74-
if containsViolation(parameterPositions: node.arguments.map(\.positionAfterSkippingLeadingTrivia)) {
75-
violations.append(node.calledExpression.positionAfterSkippingLeadingTrivia)
76-
}
47+
// Ignore calls that are part of pattern-matching syntax (patterns only, not bodies).
48+
guard !node.isInPatternMatchingPatternPosition else { return }
49+
50+
let args = node.arguments
51+
guard args.count > 1 else { return }
52+
53+
let argumentPositions = args.map(\.positionAfterSkippingLeadingTrivia)
54+
guard let violation = reasonedViolation(argumentPositions: argumentPositions) else { return }
55+
violations.append(violation)
7756
}
7857

79-
private func containsViolation(parameterPositions: [AbsolutePosition]) -> Bool {
80-
var numberOfParameters = 0
81-
var linesWithParameters: Set<Int> = []
82-
var hasMultipleParametersOnSameLine = false
58+
private func reasonedViolation(argumentPositions: [AbsolutePosition]) -> ReasonedRuleViolation? {
59+
guard let firstPos = argumentPositions.first else { return nil }
60+
61+
let firstLine = line(for: firstPos)
62+
var allOnSameLine = true
63+
64+
for pos in argumentPositions.dropFirst() where line(for: pos) != firstLine {
65+
allOnSameLine = false
66+
break
67+
}
8368

84-
for position in parameterPositions {
85-
let line = locationConverter.location(for: position).line
69+
if allOnSameLine {
70+
if !configuration.allowsSingleLine {
71+
return ReasonedRuleViolation(
72+
position: argumentPositions[1],
73+
reason: Reason.singleLineMultipleArgumentsNotAllowed
74+
)
75+
}
8676

87-
if !linesWithParameters.insert(line).inserted {
88-
hasMultipleParametersOnSameLine = true
77+
if let max = configuration.maxNumberOfSingleLineParameters,
78+
argumentPositions.count > max {
79+
return ReasonedRuleViolation(
80+
position: argumentPositions[max],
81+
reason: Reason.tooManyArgumentsOnSingleLine(max: max)
82+
)
8983
}
9084

91-
numberOfParameters += 1
85+
return nil
9286
}
9387

94-
if linesWithParameters.count == 1 {
95-
guard configuration.allowsSingleLine else {
96-
return numberOfParameters > 1
88+
var seen: Set<Int> = []
89+
for pos in argumentPositions {
90+
let line = line(for: pos)
91+
if !seen.insert(line).inserted {
92+
return ReasonedRuleViolation(
93+
position: pos,
94+
reason: Reason.eachArgumentMustStartOnOwnLine
95+
)
9796
}
97+
}
9898

99-
if let maxNumberOfSingleLineParameters = configuration.maxNumberOfSingleLineParameters {
100-
return numberOfParameters > maxNumberOfSingleLineParameters
101-
}
99+
return nil
100+
}
101+
102+
private func line(for position: AbsolutePosition) -> Int {
103+
let key = position.utf8Offset
104+
if let cached = lineCache[key] { return cached }
105+
let line = locationConverter.location(for: position).line
106+
lineCache[key] = line
107+
return line
108+
}
109+
}
110+
}
111+
112+
// MARK: - Pattern filtering (precise, pattern-part only)
113+
114+
private extension FunctionCallExprSyntax {
115+
/// `true` only when this FunctionCall is used inside a *pattern* (e.g. `.caseOne(...)`),
116+
/// not just somewhere inside `if case` / `switch case` bodies.
117+
var isInPatternMatchingPatternPosition: Bool {
118+
let selfSyntax = Syntax(self)
119+
var current: Syntax? = parent
120+
121+
// Low-level pattern nodes can appear inside multiple contexts; we only need to check each once.
122+
var checkedExpressionPattern = false
123+
var checkedValueBindingPattern = false
124+
125+
while let node = current {
126+
if !checkedExpressionPattern, let expressionPattern = node.as(ExpressionPatternSyntax.self) {
127+
checkedExpressionPattern = true
128+
if selfSyntax.isInside(Syntax(expressionPattern.expression)) { return true }
129+
}
130+
131+
if !checkedValueBindingPattern, let valueBindingPattern = node.as(ValueBindingPatternSyntax.self) {
132+
checkedValueBindingPattern = true
133+
if selfSyntax.isInside(Syntax(valueBindingPattern.pattern)) { return true }
134+
}
135+
136+
// Once we reach a *top-level* pattern container (if/switch/for/catch),
137+
// we can safely stop walking up the parent chain after checking its pattern subtree.
138+
139+
if let condition = node.as(MatchingPatternConditionSyntax.self) {
140+
if selfSyntax.isInside(Syntax(condition.pattern)) { return true }
141+
break
142+
}
102143

103-
return false
144+
if let caseItem = node.as(SwitchCaseItemSyntax.self) {
145+
if selfSyntax.isInside(Syntax(caseItem.pattern)) { return true }
146+
break
104147
}
105148

106-
return hasMultipleParametersOnSameLine
149+
if let forStmt = node.as(ForStmtSyntax.self) {
150+
if selfSyntax.isInside(Syntax(forStmt.pattern)) { return true }
151+
break
152+
}
153+
154+
if let catchClause = node.as(CatchClauseSyntax.self) {
155+
for item in catchClause.catchItems {
156+
if let pattern = item.pattern,
157+
selfSyntax.isInside(Syntax(pattern)) {
158+
return true
159+
}
160+
}
161+
break
162+
}
163+
164+
current = node.parent
165+
}
166+
167+
return false
168+
}
169+
}
170+
171+
// MARK: - Generic helpers
172+
173+
private extension Syntax {
174+
/// Returns `true` if `self` is the `ancestor` node itself or is located inside its subtree.
175+
func isInside(_ ancestor: Syntax) -> Bool {
176+
var current: Syntax? = self
177+
while let node = current {
178+
if node.id == ancestor.id { return true }
179+
current = node.parent
107180
}
181+
return false
108182
}
109183
}

0 commit comments

Comments
 (0)