Skip to content

Commit f55da3f

Browse files
GandaLF2006GandaLF2006
committed
Improve pattern_matching_keywords SwiftSyntax rule
1 parent eddb35d commit f55da3f

2 files changed

Lines changed: 331 additions & 47 deletions

File tree

‎Source/SwiftLintBuiltInRules/Rules/Idiomatic/PatternMatchingKeywordsRule.swift‎

Lines changed: 200 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,12 @@ struct PatternMatchingKeywordsRule: Rule {
77
static let description = RuleDescription(
88
identifier: "pattern_matching_keywords",
99
name: "Pattern Matching Keywords",
10-
description: "Combine multiple pattern matching bindings by moving keywords out of tuples",
10+
description: """
11+
Combine multiple pattern matching bindings by moving keywords out of tuples
12+
and enum associated values
13+
""",
1114
kind: .idiomatic,
12-
nonTriggeringExamples: [
15+
nonTriggeringExamples: switchWrapped(examples: [
1316
Example("default"),
1417
Example("case 1"),
1518
Example("case bar"),
@@ -22,88 +25,238 @@ struct PatternMatchingKeywordsRule: Rule {
2225
Example("case .foo(var x)"),
2326
Example("case var .foo(x, y)"),
2427
Example("case (y, let x, z)"),
25-
].map(wrapInSwitch),
26-
triggeringExamples: [
27-
Example("case (↓let x, ↓let y)"),
28-
Example("case (↓let x, ↓let y, .foo)"),
29-
Example("case (↓let x, ↓let y, _)"),
30-
Example("case (↓let x, ↓let y, f())"),
31-
Example("case (↓let x, ↓let y, s.f())"),
32-
Example("case (↓let x, ↓let y, s.t)"),
28+
Example("case (foo, let x)"),
29+
Example("case (foo, let x, let y)"),
30+
Example("case .foo(bar, let x)"),
31+
Example("case (let x, y)"),
32+
Example("case .foo(let x, y)"),
33+
Example("case (.foo(let x), y)"),
34+
Example("case let .foo(x, y), let .bar(x, y)"),
35+
Example("case var .foo(x, y), var .bar(x, y)"),
36+
Example("case let .foo(x, y), let .bar(x, y), let .baz(x, y)"),
37+
Example("case .foo(bar: let x, baz: var y)"),
38+
Example("case (.yamlParsing(var x), (.yamlParsing(var y), z))"),
39+
Example("case (.foo(let x), (y, let z))"),
40+
]) + [
41+
Example("if case let (x, y) = foo {}"),
42+
Example("guard case let (x, y) = foo else { return }"),
43+
Example("while case let (x, y) = foo {}"),
44+
Example("for case let (x, y) in foos {}"),
45+
Example("if case (foo, let x) = value {}"),
46+
Example("guard case .foo(bar, let x) = value else { return }"),
47+
Example("do {} catch let Pattern.error(x, y) {}"),
48+
Example("do {} catch (foo, let x) {}"),
49+
],
50+
triggeringExamples: switchWrapped(examples: [
51+
Example("case (↓let x, ↓let y)"),
52+
Example("case (↓let x, ↓let y, .foo)"),
53+
Example("case (↓let x, ↓let y, _)"),
54+
Example("case (↓let x, ↓let y, 1)"),
55+
Example("case (↓let x, ↓let y, f())"),
56+
Example("case (↓let x, ↓let y, s.f())"),
57+
Example("case (↓let x, ↓let y, s.t)"),
3358
Example("case .foo(↓let x, ↓let y)"),
34-
Example("case (.yamlParsing(↓let x), .yamlParsing(↓let y))"),
35-
Example("case (↓var x, ↓var y)"),
59+
Example("case .foo(bar: ↓let x, baz: ↓let y)"),
3660
Example("case .foo(↓var x, ↓var y)"),
37-
Example("case (.yamlParsing(↓var x), .yamlParsing(↓var y))"),
38-
].map(wrapInSwitch)
61+
Example("case .foo(bar: ↓var x, baz: ↓var y)"),
62+
Example("case (.yamlParsing(↓let x), .yamlParsing(↓let y))"),
63+
Example("case (.yamlParsing(↓var x), (.yamlParsing(↓var y), _))"),
64+
Example("case ((↓let x, ↓let y), z)"),
65+
Example("case .foo((↓let x, ↓let y), z)"),
66+
Example("case (.foo(↓let x, ↓let y), z)"),
67+
Example("case .foo(.bar(↓let x), .bar(↓let y))"),
68+
Example("case .foo(.bar(↓let x), .bar(↓let y), .baz)"),
69+
Example("case .foo(↓let x, ↓let y), .bar(↓let x, ↓let y)"),
70+
Example("case .foo(↓var x, ↓var y), .bar(↓var x, ↓var y)"),
71+
]) + [
72+
Example("if case (↓let x, ↓let y) = foo {}"),
73+
Example("guard case (↓let x, ↓let y) = foo else { return }"),
74+
Example("while case (↓let x, ↓let y) = foo {}"),
75+
Example("for case (↓let x, ↓let y) in foos {}"),
76+
Example("if case .foo(bar: ↓let x, baz: ↓let y) = value {}"),
77+
Example("do {} catch Pattern.error(↓let x, ↓let y) {}"),
78+
Example("do {} catch (↓let x, ↓let y) {}"),
79+
Example("do {} catch Foo.outer(.inner(↓let x), .inner(↓let y)) {}"),
80+
]
3981
)
4082

41-
private static func wrapInSwitch(_ example: Example) -> Example {
42-
example.with(code: """
43-
switch foo {
44-
\(example.code): break
45-
}
46-
""")
83+
private static func switchWrapped(examples: [Example]) -> [Example] {
84+
examples.map { example in
85+
example.with(
86+
code: """
87+
switch foo {
88+
\(example.code): break
89+
}
90+
"""
91+
)
92+
}
4793
}
4894
}
4995

5096
private extension PatternMatchingKeywordsRule {
5197
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
5298
override func visitPost(_ node: SwitchCaseItemSyntax) {
53-
let localViolations = TupleVisitor(configuration: configuration, file: file)
54-
.walk(tree: node.pattern, handler: \.violations)
55-
violations.append(contentsOf: localViolations)
99+
appendViolations(from: node.pattern)
100+
}
101+
102+
override func visitPost(_ node: MatchingPatternConditionSyntax) {
103+
appendViolations(from: node.pattern)
104+
}
105+
106+
override func visitPost(_ node: ForStmtSyntax) {
107+
guard node.caseKeyword != nil else {
108+
return
109+
}
110+
111+
appendViolations(from: node.pattern)
112+
}
113+
114+
override func visitPost(_ node: CatchItemSyntax) {
115+
guard let pattern = node.pattern else {
116+
return
117+
}
118+
119+
appendViolations(from: pattern)
120+
}
121+
122+
private func appendViolations(from pattern: PatternSyntax) {
123+
violations.append(contentsOf: PatternViolationCollector.collect(in: pattern))
56124
}
57125
}
58126
}
59127

60-
private final class TupleVisitor<Configuration: RuleConfiguration>: ViolationsSyntaxVisitor<Configuration> {
61-
override func visitPost(_ node: LabeledExprListSyntax) {
62-
let list = node.flatteningEnumPatterns().map(\.expression.categorized)
63-
if list.contains(where: \.isReference) {
128+
private enum PatternViolationCollector {
129+
static func collect(in pattern: PatternSyntax) -> [AbsolutePosition] {
130+
var violations: [AbsolutePosition] = []
131+
collect(from: pattern, into: &violations)
132+
return violations
133+
}
134+
135+
private static func collect(from pattern: PatternSyntax, into violations: inout [AbsolutePosition]) {
136+
if let binding = pattern.as(ValueBindingPatternSyntax.self) {
137+
collect(from: binding.pattern, into: &violations)
64138
return
65139
}
66-
let specifiers = list.compactMap { if case let .binding(specifier) = $0 { specifier } else { nil } }
67-
if specifiers.count > 1, specifiers.allSatisfy({ $0.tokenKind == specifiers.first?.tokenKind }) {
68-
violations.append(contentsOf: specifiers.map(\.positionAfterSkippingLeadingTrivia))
140+
141+
guard let expression = pattern.as(ExpressionPatternSyntax.self)?.expression else {
142+
return
69143
}
144+
145+
collect(from: expression, into: &violations)
70146
}
71-
}
72147

73-
private extension LabeledExprListSyntax {
74-
func flatteningEnumPatterns() -> [LabeledExprSyntax] {
75-
flatMap { elem in
76-
guard let pattern = elem.expression.as(FunctionCallExprSyntax.self),
77-
pattern.calledExpression.is(MemberAccessExprSyntax.self) else {
78-
return [elem]
148+
private static func collect(from expression: ExprSyntax, into violations: inout [AbsolutePosition]) {
149+
if let childExpressions = expression.immediatePatternGroupChildren {
150+
appendViolationsIfBindingsCanBeLifted(from: childExpressions, into: &violations)
151+
152+
for childExpression in childExpressions {
153+
collect(from: childExpression, into: &violations)
79154
}
80155

81-
return Array(pattern.arguments)
156+
return
157+
}
158+
159+
if let pattern = expression.as(PatternExprSyntax.self)?.pattern {
160+
collect(from: pattern, into: &violations)
161+
}
162+
}
163+
164+
private static func appendViolationsIfBindingsCanBeLifted(
165+
from expressions: [ExprSyntax],
166+
into violations: inout [AbsolutePosition]
167+
) {
168+
let categories = expressions.map(GroupCategoryResolver.category(for:))
169+
170+
if categories.contains(where: \.isReference) {
171+
return
172+
}
173+
174+
let specifiers = categories.compactMap(\.bindingSpecifier)
175+
guard specifiers.count > 1, let first = specifiers.first else {
176+
return
177+
}
178+
179+
if specifiers.allSatisfy({ $0.tokenKind == first.tokenKind }) {
180+
violations.append(contentsOf: specifiers.map(\.positionAfterSkippingLeadingTrivia))
82181
}
83182
}
84183
}
85184

86-
private enum ArgumentType {
185+
private enum GroupCategory {
87186
case binding(specifier: TokenSyntax)
88187
case reference
89-
case constant
188+
case neutral
90189

91190
var isReference: Bool {
92191
switch self {
93-
case .reference: true
94-
default: false
192+
case .reference:
193+
return true
194+
default:
195+
return false
196+
}
197+
}
198+
199+
var bindingSpecifier: TokenSyntax? {
200+
switch self {
201+
case let .binding(specifier):
202+
return specifier
203+
default:
204+
return nil
95205
}
96206
}
97207
}
98208

99-
private extension ExprSyntax {
100-
var categorized: ArgumentType {
101-
if let binding = `as`(PatternExprSyntax.self)?.pattern.as(ValueBindingPatternSyntax.self) {
209+
private enum GroupCategoryResolver {
210+
static func category(for expression: ExprSyntax) -> GroupCategory {
211+
if let binding = expression.as(PatternExprSyntax.self)?.pattern.as(ValueBindingPatternSyntax.self) {
102212
return .binding(specifier: binding.bindingSpecifier)
103213
}
104-
if `is`(DeclReferenceExprSyntax.self) {
214+
215+
// `case (foo, let x)` must not become `case let (foo, x)`,
216+
// because `foo` would stop matching an existing value and
217+
// would instead become a newly introduced binding.
218+
if expression.is(DeclReferenceExprSyntax.self) {
105219
return .reference
106220
}
107-
return .constant
221+
222+
if let childExpressions = expression.immediatePatternGroupChildren {
223+
return liftedCategory(for: childExpressions)
224+
}
225+
226+
return .neutral
227+
}
228+
229+
private static func liftedCategory(for expressions: [ExprSyntax]) -> GroupCategory {
230+
let categories = expressions.map(category(for:))
231+
232+
if categories.contains(where: \.isReference) {
233+
return .neutral
234+
}
235+
236+
let specifiers = categories.compactMap(\.bindingSpecifier)
237+
guard let first = specifiers.first else {
238+
return .neutral
239+
}
240+
241+
if specifiers.allSatisfy({ $0.tokenKind == first.tokenKind }) {
242+
return .binding(specifier: first)
243+
}
244+
245+
return .neutral
246+
}
247+
}
248+
249+
private extension ExprSyntax {
250+
var immediatePatternGroupChildren: [ExprSyntax]? {
251+
if let tuple = `as`(TupleExprSyntax.self) {
252+
return tuple.elements.map(\.expression)
253+
}
254+
255+
if let call = `as`(FunctionCallExprSyntax.self),
256+
call.calledExpression.is(MemberAccessExprSyntax.self) {
257+
return call.arguments.map(\.expression)
258+
}
259+
260+
return nil
108261
}
109262
}

0 commit comments

Comments
 (0)