Skip to content

Commit b42c512

Browse files
snoflasnofla
committed
Add new rule unnest_switches_using_tuple 
1 parent e1ac6f8 commit b42c512

File tree

5 files changed

+220
-0
lines changed

5 files changed

+220
-0
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@
3838

3939
* Add new `allowed_numbers` option to the `no_magic_numbers` rule.
4040
[Martin Redington](https://github.com/mildm8nnered)
41+
* Add new `unnest_switches_using_tuple` opt-in rule that triggers when
42+
nested switches are encountered that reference the same variable. These
43+
can be replaced by a switch on a tuple.
44+
[Alfons Hoogervorst](https://github.com/snofla/SwiftLint)
4145

4246
### Bug Fixes
4347

‎Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -221,6 +221,7 @@ public let builtInRules: [any Rule.Type] = [
221221
UnneededOverrideRule.self,
222222
UnneededParenthesesInClosureArgumentRule.self,
223223
UnneededSynthesizedInitializerRule.self,
224+
UnnestSwitchesUsingTupleRule.self,
224225
UnownedVariableCaptureRule.self,
225226
UntypedErrorInCatchRule.self,
226227
UnusedClosureParameterRule.self,

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

Lines changed: 204 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,204 @@
1+
import SwiftLintCore
2+
import SwiftSyntax
3+
4+
@SwiftSyntaxRule(optIn: true)
5+
struct UnnestSwitchesUsingTupleRule: Rule {
6+
var configuration = SeverityConfiguration<Self>(.warning)
7+
8+
static let description = RuleDescription(
9+
identifier: "unnest_switches_using_tuple",
10+
name: "Unnest Switches Using Tuple",
11+
description: "Prevent nesting switches by preferring a switch on a tuple",
12+
kind: .lint,
13+
nonTriggeringExamples: [
14+
Example("""
15+
switch (a, b) {
16+
case (1, 1):
17+
break
18+
case (1, 2):
19+
break
20+
case (2, 1):
21+
break
22+
case (2, 2):
23+
break
24+
}
25+
"""),
26+
Example("""
27+
switch (a, b) {
28+
case (1, 1):
29+
break
30+
case (1, 2):
31+
break
32+
case (2, _):
33+
break
34+
}
35+
"""),
36+
Example("""
37+
switch a {
38+
case 1:
39+
let b = something
40+
switch b {
41+
case 1:
42+
break
43+
case 2:
44+
break
45+
}
46+
case 2:
47+
break
48+
}
49+
"""),
50+
],
51+
triggeringExamples: [
52+
Example("""
53+
↓switch a {
54+
case 1:
55+
switch b {
56+
case 1:
57+
break
58+
case 2:
59+
break
60+
}
61+
case 2:
62+
switch b {
63+
case 1:
64+
break
65+
case 2:
66+
break
67+
}
68+
}
69+
"""),
70+
Example("""
71+
↓switch a {
72+
case 1:
73+
switch b {
74+
case 1:
75+
break
76+
case 2:
77+
break
78+
}
79+
case 2:
80+
break
81+
}
82+
"""),
83+
Example("""
84+
switch a {
85+
case 1:
86+
↓switch b {
87+
case 1:
88+
switch c {
89+
case 1:
90+
break
91+
}
92+
case 2:
93+
break
94+
}
95+
case 2:
96+
break
97+
}
98+
"""),
99+
]
100+
)
101+
}
102+
103+
private extension UnnestSwitchesUsingTupleRule {
104+
105+
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
106+
107+
override func visitPost(_ node: SwitchExprSyntax) {
108+
guard let nestingSwitch = self.switchNestedSwitches[node] else {
109+
// does this switch have a parent switch?
110+
guard let parent = node.firstParent(ofKind: .switchExpr),
111+
let parentSwitchExpr = parent.as(SwitchExprSyntax.self) else {
112+
return
113+
}
114+
// file this node, which is a nested switch, under its parent
115+
self.switchNestedSwitches[parentSwitchExpr, default: []].append(node)
116+
return
117+
}
118+
// check the parent switch
119+
defer { self.switchNestedSwitches = [:] }
120+
guard !nestingSwitch.isEmpty else {
121+
return
122+
}
123+
// we only want the nested switched without a local reference
124+
// to the switch's decl expression, so the following is
125+
// not triggering a violation:
126+
//
127+
// let b = 1 // or a var
128+
// switch (b) {
129+
// }
130+
//
131+
let variables: Set<String> = nestingSwitch
132+
.filter { !$0.hasDeclarationInLabelScope(for: $0.referencedVariable) }
133+
.compactMap { $0.referencedVariable }
134+
.reduce(into: [], { p, e in p.insert(e) })
135+
136+
// we want all nested switches to reference the same variable
137+
// to be able to unnest a switch using tuples
138+
guard variables.count == 1 else {
139+
// different variables referenced
140+
return
141+
}
142+
self.violations.append(node.positionAfterSkippingLeadingTrivia)
143+
}
144+
145+
private var switchNestedSwitches: [SwitchExprSyntax: [SwitchExprSyntax]] = [:]
146+
}
147+
}
148+
149+
private extension SwitchExprSyntax {
150+
151+
func hasDeclarationInLabelScope(for variable: String?) -> Bool {
152+
guard let variable else {
153+
return false
154+
}
155+
guard let switchCaseSyntax = self.firstParent(ofKind: .switchCase)?.as(SwitchCaseSyntax.self) else {
156+
return false
157+
}
158+
guard let switchCodeBlockItem = self.firstParent(ofKind: .codeBlockItem)?.as(CodeBlockItemSyntax.self) else {
159+
return false
160+
}
161+
let declReferencingVariable = switchCaseSyntax.statements
162+
.prefix { $0 != switchCodeBlockItem }
163+
.first { $0.containsReference(to: variable) }
164+
return declReferencingVariable != nil
165+
}
166+
167+
var referencedVariable: String? {
168+
self.subject.as(DeclReferenceExprSyntax.self)?.baseName.text
169+
}
170+
}
171+
172+
private extension CodeBlockItemSyntax {
173+
174+
func containsReference(to variable: String) -> Bool {
175+
guard self.item.kind == .variableDecl else {
176+
return false
177+
}
178+
guard let variableDecl = self.item.as(VariableDeclSyntax.self) else {
179+
return false
180+
}
181+
guard variableDecl.references(variable) else {
182+
return false
183+
}
184+
return true
185+
}
186+
}
187+
188+
private extension VariableDeclSyntax {
189+
190+
func references(_ variable: String) -> Bool {
191+
self.bindings.first?.pattern.as(IdentifierPatternSyntax.self)?.identifier.text == variable
192+
}
193+
}
194+
195+
private extension SyntaxProtocol {
196+
197+
func firstParent(ofKind type: SyntaxKind) -> Syntax? {
198+
var someParent: Syntax? = self.parent
199+
while let current = someParent, current.kind != type {
200+
someParent = current.parent
201+
}
202+
return someParent
203+
}
204+
}

‎Tests/GeneratedTests/GeneratedTests.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1321,6 +1321,12 @@ final class UnneededSynthesizedInitializerRuleGeneratedTests: SwiftLintTestCase
13211321
}
13221322
}
13231323

1324+
final class UnnestSwitchesUsingTupleRuleGeneratedTests: SwiftLintTestCase {
1325+
func testWithDefaultConfiguration() {
1326+
verifyRule(UnnestSwitchesUsingTupleRule.description)
1327+
}
1328+
}
1329+
13241330
final class UnownedVariableCaptureRuleGeneratedTests: SwiftLintTestCase {
13251331
func testWithDefaultConfiguration() {
13261332
verifyRule(UnownedVariableCaptureRule.description)

‎Tests/IntegrationTests/default_rule_configurations.yml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1262,6 +1262,11 @@ unneeded_synthesized_initializer:
12621262
meta:
12631263
opt-in: false
12641264
correctable: true
1265+
unnest_switches_using_tuple:
1266+
severity: warning
1267+
meta:
1268+
opt-in: true
1269+
correctable: false
12651270
unowned_variable_capture:
12661271
severity: warning
12671272
meta:

0 commit comments

Comments
 (0)