Skip to content

Commit f7c1ffc

Browse files
SimplyDannySimplyDanny
committed
Add new opaque_over_existential rule
1 parent be25c1f commit f7c1ffc

File tree

8 files changed

+128
-16
lines changed

8 files changed

+128
-16
lines changed

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,10 @@
3636
* Support replacing identity expressions with `\.self` in `prefer_key_path` rule from Swift 6 on.
3737
[SimplyDanny](https://github.com/SimplyDanny)
3838

39+
* Add new `opaque_over_existential` opt-in rule that triggers when the existential `any` type of a
40+
function parameter can be replaced with an opaque `some` type.
41+
[SimplyDanny](https://github.com/SimplyDanny)
42+
3943
* Support linting only provided file paths with command plugins.
4044
[DanSkeel](https://github.com/DanSkeel)
4145

‎Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ public let builtInRules: [any Rule.Type] = [
139139
NumberSeparatorRule.self,
140140
ObjectLiteralRule.self,
141141
OneDeclarationPerFileRule.self,
142+
OpaqueOverExistentialParameterRule.self,
142143
OpeningBraceRule.self,
143144
OperatorFunctionWhitespaceRule.self,
144145
OperatorUsageWhitespaceRule.self,

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

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import SwiftSyntax
2+
3+
@SwiftSyntaxRule(correctable: true, optIn: true)
4+
struct OpaqueOverExistentialParameterRule: Rule {
5+
var configuration = SeverityConfiguration<Self>(.warning)
6+
7+
static let description = RuleDescription(
8+
identifier: "opaque_over_existential",
9+
name: "Opaque Over Existential Parameter",
10+
description: "Prefer opaque type over existential type in function parameter",
11+
kind: .lint,
12+
nonTriggeringExamples: [
13+
Example("func f(_: some P) {}"),
14+
Example("func f(_: (some P)?) {}"),
15+
Example("func f(_: some P & Q) {}"),
16+
Example("func f(_: any P.Type) {}"),
17+
Example("func f(_: (any P.Type)?) {}"),
18+
Example("func f(_: borrowing any ~Copyable.Type) {}"),
19+
Example("func f(_: () -> Int = { let i: any P = p(); return i.get() }) {}"),
20+
Example("func f(_: [any P]) {}"),
21+
Example("func f(_: [any P: any Q]) {}"),
22+
Example("func f(_: () -> any P) {}"),
23+
],
24+
triggeringExamples: [
25+
Example("func f(_: ↓any P) {}"),
26+
Example("func f(_: (↓any P)?) {}"),
27+
Example("func f(_: ↓any P & Q) {}"),
28+
Example("func f(_: borrowing ↓any ~Copyable) {}"),
29+
Example("func f(_: borrowing (↓any ~Copyable)?) {}"),
30+
Example("func f(_: (↓any P, ↓any Q)) {}"),
31+
],
32+
corrections: [
33+
Example("func f(_: any P) {}"):
34+
Example("func f(_: some P) {}"),
35+
Example("func f(_: (any P)?) {}"):
36+
Example("func f(_: (some P)?) {}"),
37+
Example("func f(_: any P & Q) {}"):
38+
Example("func f(_: some P & Q) {}"),
39+
Example("func f(_: /* comment */ any P/* comment*/) {}"):
40+
Example("func f(_: /* comment */ some P/* comment*/) {}"),
41+
Example("func f(_: borrowing (any ~Copyable)?) {}"):
42+
Example("func f(_: borrowing (some ~Copyable)?) {}"),
43+
]
44+
)
45+
}
46+
47+
private extension OpaqueOverExistentialParameterRule {
48+
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
49+
override func visitPost(_ node: FunctionParameterSyntax) {
50+
AnyTypeVisitor(viewMode: .sourceAccurate).walk(tree: node.type, handler: \.anyRanges).forEach { range in
51+
violations.append(.init(
52+
position: range.start,
53+
correction: .init(
54+
start: range.start,
55+
end: range.end,
56+
replacement: "some"
57+
)
58+
))
59+
}
60+
}
61+
}
62+
}
63+
64+
private class AnyTypeVisitor: SyntaxVisitor {
65+
var anyRanges = [(start: AbsolutePosition, end: AbsolutePosition)]()
66+
67+
override func visitPost(_ node: SomeOrAnyTypeSyntax) {
68+
let specifier = node.someOrAnySpecifier
69+
if specifier.tokenKind == .keyword(.any), !node.constraint.isMetaType {
70+
anyRanges.append((specifier.positionAfterSkippingLeadingTrivia, specifier.endPositionBeforeTrailingTrivia))
71+
}
72+
}
73+
74+
override func visit(_: ArrayTypeSyntax) -> SyntaxVisitorContinueKind {
75+
.skipChildren
76+
}
77+
78+
override func visit(_: DictionaryTypeSyntax) -> SyntaxVisitorContinueKind {
79+
.skipChildren
80+
}
81+
82+
override func visit(_: FunctionTypeSyntax) -> SyntaxVisitorContinueKind {
83+
.skipChildren
84+
}
85+
}
86+
87+
private extension TypeSyntax {
88+
var isMetaType: Bool {
89+
if `is`(MetatypeTypeSyntax.self) {
90+
return true
91+
}
92+
if let suppressedType = `as`(SuppressedTypeSyntax.self) {
93+
return suppressedType.type.isMetaType
94+
}
95+
return false
96+
}
97+
}

‎Source/SwiftLintCore/Protocols/CollectingRule.swift‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,12 @@ package extension CollectingCorrectableRule where Self: AnalyzerRule {
157157
/// :nodoc:
158158
public extension Array where Element == any Rule {
159159
static func == (lhs: Array, rhs: Array) -> Bool {
160-
if lhs.count != rhs.count { return false }
161-
return !zip(lhs, rhs).contains { !$0.0.isEqualTo($0.1) }
160+
guard lhs.count == rhs.count else {
161+
return false
162+
}
163+
return !zip(lhs, rhs).contains { pair in
164+
let first = pair.0, second = pair.1
165+
return !first.isEqualTo(second)
166+
}
162167
}
163168
}

‎Source/SwiftLintCore/Protocols/Rule.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ public protocol Rule {
4848
/// - parameter rule: The `rule` value to compare against.
4949
///
5050
/// - returns: Whether or not the specified rule is equivalent to the current rule.
51-
func isEqualTo(_ rule: any Rule) -> Bool
51+
func isEqualTo(_ rule: some Rule) -> Bool
5252

5353
/// Collects information for the specified file in a storage object, to be analyzed by a `CollectedLinter`.
5454
///
@@ -110,7 +110,7 @@ public extension Rule {
110110
validate(file: file)
111111
}
112112

113-
func isEqualTo(_ rule: any Rule) -> Bool {
113+
func isEqualTo(_ rule: some Rule) -> Bool {
114114
if let rule = rule as? Self {
115115
return configuration == rule.configuration
116116
}

‎Source/swiftlint/Commands/Rules.swift‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,7 @@ extension SwiftLint {
3636
.list
3737
.sorted { $0.0 < $1.0 }
3838
if configOnly {
39-
rules
40-
.map(\.value)
41-
.map { createInstance(of: $0, using: configuration, configure: !defaultConfig) }
42-
.forEach { printConfig(for: $0) }
39+
rules.forEach { printConfig(for: createInstance(of: $0.value, using: configuration)) }
4340
} else {
4441
let table = TextTable(
4542
ruleList: rules,
@@ -54,7 +51,7 @@ extension SwiftLint {
5451
private func printDescription(for ruleType: any Rule.Type, with configuration: Configuration) {
5552
let description = ruleType.description
5653

57-
let rule = createInstance(of: ruleType, using: configuration, configure: !defaultConfig)
54+
let rule = createInstance(of: ruleType, using: configuration)
5855
if configOnly {
5956
printConfig(for: rule)
6057
return
@@ -76,20 +73,18 @@ extension SwiftLint {
7673
}
7774
}
7875

79-
private func printConfig(for rule: any Rule) {
76+
private func printConfig(for rule: some Rule) {
8077
let configDescription = rule.createConfigurationDescription()
8178
if configDescription.hasContent {
8279
print("\(type(of: rule).identifier):")
8380
print(configDescription.yaml().indent(by: 2))
8481
}
8582
}
8683

87-
private func createInstance(of ruleType: any Rule.Type,
88-
using config: Configuration,
89-
configure: Bool) -> any Rule {
90-
configure
91-
? config.configuredRule(forID: ruleType.identifier) ?? ruleType.init()
92-
: ruleType.init()
84+
private func createInstance(of ruleType: any Rule.Type, using config: Configuration) -> any Rule {
85+
defaultConfig
86+
? ruleType.init()
87+
: config.configuredRule(forID: ruleType.identifier) ?? ruleType.init()
9388
}
9489
}
9590
}

‎Tests/GeneratedTests/GeneratedTests.swift‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -823,6 +823,12 @@ final class OneDeclarationPerFileRuleGeneratedTests: SwiftLintTestCase {
823823
}
824824
}
825825

826+
final class OpaqueOverExistentialParameterRuleGeneratedTests: SwiftLintTestCase {
827+
func testWithDefaultConfiguration() {
828+
verifyRule(OpaqueOverExistentialParameterRule.description)
829+
}
830+
}
831+
826832
final class OpeningBraceRuleGeneratedTests: SwiftLintTestCase {
827833
func testWithDefaultConfiguration() {
828834
verifyRule(OpeningBraceRule.description)

‎Tests/IntegrationTests/default_rule_configurations.yml‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -649,6 +649,10 @@ one_declaration_per_file:
649649
severity: warning
650650
meta:
651651
opt-in: true
652+
opaque_over_existential:
653+
severity: warning
654+
meta:
655+
opt-in: true
652656
opening_brace:
653657
severity: warning
654658
ignore_multiline_type_headers: false

0 commit comments

Comments
 (0)