Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new opaque_over_existential rule #5915

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
* Support replacing identity expressions with `\.self` in `prefer_key_path` rule from Swift 6 on.
[SimplyDanny](https://github.com/SimplyDanny)

* Add new `opaque_over_existential` opt-in rule that triggers when the existential `any` type of a
function parameter can be replaced with an opaque `some` type.
[SimplyDanny](https://github.com/SimplyDanny)

* Support linting only provided file paths with command plugins.
[DanSkeel](https://github.com/DanSkeel)

Expand Down
1 change: 1 addition & 0 deletions Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ public let builtInRules: [any Rule.Type] = [
NumberSeparatorRule.self,
ObjectLiteralRule.self,
OneDeclarationPerFileRule.self,
OpaqueOverExistentialParameterRule.self,
OpeningBraceRule.self,
OperatorFunctionWhitespaceRule.self,
OperatorUsageWhitespaceRule.self,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
import SwiftSyntax

@SwiftSyntaxRule(correctable: true, optIn: true)
struct OpaqueOverExistentialParameterRule: Rule {
var configuration = SeverityConfiguration<Self>(.warning)

static let description = RuleDescription(
identifier: "opaque_over_existential",
name: "Opaque Over Existential Parameter",
description: "Prefer opaque over existential type in function parameter",
kind: .lint,
nonTriggeringExamples: [
Example("func f(_: some P) {}"),
Example("func f(_: (some P)?) {}"),
Example("func f(_: some P & Q) {}"),
Example("func f(_: any P.Type) {}"),
Example("func f(_: (any P.Type)?) {}"),
Example("func f(_: borrowing any ~Copyable.Type) {}"),
Example("func f(_: () -> Int = { let i: any P = p(); return i.get() }) {}"),
Example("func f(_: [any P]) {}"),
Example("func f(_: [any P: any Q]) {}"),
Example("func f(_: () -> any P) {}"),
],
triggeringExamples: [
Example("func f(_: ↓any P) {}"),
Example("func f(_: (↓any P)?) {}"),
Example("func f(_: ↓any P & Q) {}"),
Example("func f(_: borrowing ↓any ~Copyable) {}"),
Example("func f(_: borrowing (↓any ~Copyable)?) {}"),
Example("func f(_: (↓any P, ↓any Q)) {}"),
],
corrections: [
Example("func f(_: any P) {}"):
Example("func f(_: some P) {}"),
Example("func f(_: (any P)?) {}"):
Example("func f(_: (some P)?) {}"),
Example("func f(_: any P & Q) {}"):
Example("func f(_: some P & Q) {}"),
Example("func f(_: /* comment */ any P/* comment*/) {}"):
Example("func f(_: /* comment */ some P/* comment*/) {}"),
Example("func f(_: borrowing (any ~Copyable)?) {}"):
Example("func f(_: borrowing (some ~Copyable)?) {}"),
]
)
}

private extension OpaqueOverExistentialParameterRule {
final class Visitor: ViolationsSyntaxVisitor<ConfigurationType> {
override func visitPost(_ node: FunctionParameterSyntax) {
AnyTypeVisitor(viewMode: .sourceAccurate).walk(tree: node.type, handler: \.anyRanges).forEach { range in
violations.append(.init(
position: range.start,
correction: .init(
start: range.start,
end: range.end,
replacement: "some"
)
))
}
}
}
}

private class AnyTypeVisitor: SyntaxVisitor {
var anyRanges = [(start: AbsolutePosition, end: AbsolutePosition)]()

override func visitPost(_ node: SomeOrAnyTypeSyntax) {
let specifier = node.someOrAnySpecifier
if specifier.tokenKind == .keyword(.any), !node.constraint.isMetaType {
anyRanges.append((specifier.positionAfterSkippingLeadingTrivia, specifier.endPositionBeforeTrailingTrivia))
}
}

override func visit(_: ArrayTypeSyntax) -> SyntaxVisitorContinueKind {
.skipChildren
}

override func visit(_: DictionaryTypeSyntax) -> SyntaxVisitorContinueKind {
.skipChildren
}

override func visit(_: FunctionTypeSyntax) -> SyntaxVisitorContinueKind {
.skipChildren
}
}

private extension TypeSyntax {
var isMetaType: Bool {
if `is`(MetatypeTypeSyntax.self) {
return true
}
if let suppressedType = `as`(SuppressedTypeSyntax.self) {
return suppressedType.type.isMetaType
}
return false
}
}
9 changes: 7 additions & 2 deletions Source/SwiftLintCore/Protocols/CollectingRule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,12 @@ package extension CollectingCorrectableRule where Self: AnalyzerRule {
/// :nodoc:
public extension Array where Element == any Rule {
static func == (lhs: Array, rhs: Array) -> Bool {
if lhs.count != rhs.count { return false }
return !zip(lhs, rhs).contains { !$0.0.isEqualTo($0.1) }
guard lhs.count == rhs.count else {
return false
}
return !zip(lhs, rhs).contains { pair in
let first = pair.0, second = pair.1
return !first.isEqualTo(second)
}
}
}
4 changes: 2 additions & 2 deletions Source/SwiftLintCore/Protocols/Rule.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ public protocol Rule {
/// - parameter rule: The `rule` value to compare against.
///
/// - returns: Whether or not the specified rule is equivalent to the current rule.
func isEqualTo(_ rule: any Rule) -> Bool
func isEqualTo(_ rule: some Rule) -> Bool

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

func isEqualTo(_ rule: any Rule) -> Bool {
func isEqualTo(_ rule: some Rule) -> Bool {
if let rule = rule as? Self {
return configuration == rule.configuration
}
Expand Down
19 changes: 7 additions & 12 deletions Source/swiftlint/Commands/Rules.swift
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ extension SwiftLint {
.list
.sorted { $0.0 < $1.0 }
if configOnly {
rules
.map(\.value)
.map { createInstance(of: $0, using: configuration, configure: !defaultConfig) }
.forEach { printConfig(for: $0) }
rules.forEach { printConfig(for: createInstance(of: $0.value, using: configuration)) }
} else {
let table = TextTable(
ruleList: rules,
Expand All @@ -54,7 +51,7 @@ extension SwiftLint {
private func printDescription(for ruleType: any Rule.Type, with configuration: Configuration) {
let description = ruleType.description

let rule = createInstance(of: ruleType, using: configuration, configure: !defaultConfig)
let rule = createInstance(of: ruleType, using: configuration)
if configOnly {
printConfig(for: rule)
return
Expand All @@ -76,20 +73,18 @@ extension SwiftLint {
}
}

private func printConfig(for rule: any Rule) {
private func printConfig(for rule: some Rule) {
let configDescription = rule.createConfigurationDescription()
if configDescription.hasContent {
print("\(type(of: rule).identifier):")
print(configDescription.yaml().indent(by: 2))
}
}

private func createInstance(of ruleType: any Rule.Type,
using config: Configuration,
configure: Bool) -> any Rule {
configure
? config.configuredRule(forID: ruleType.identifier) ?? ruleType.init()
: ruleType.init()
private func createInstance(of ruleType: any Rule.Type, using config: Configuration) -> any Rule {
defaultConfig
? ruleType.init()
: config.configuredRule(forID: ruleType.identifier) ?? ruleType.init()
}
}
}
Expand Down
6 changes: 6 additions & 0 deletions Tests/GeneratedTests/GeneratedTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -823,6 +823,12 @@ final class OneDeclarationPerFileRuleGeneratedTests: SwiftLintTestCase {
}
}

final class OpaqueOverExistentialParameterRuleGeneratedTests: SwiftLintTestCase {
func testWithDefaultConfiguration() {
verifyRule(OpaqueOverExistentialParameterRule.description)
}
}

final class OpeningBraceRuleGeneratedTests: SwiftLintTestCase {
func testWithDefaultConfiguration() {
verifyRule(OpeningBraceRule.description)
Expand Down
4 changes: 4 additions & 0 deletions Tests/IntegrationTests/default_rule_configurations.yml
Original file line number Diff line number Diff line change
Expand Up @@ -649,6 +649,10 @@ one_declaration_per_file:
severity: warning
meta:
opt-in: true
opaque_over_existential:
severity: warning
meta:
opt-in: true
opening_brace:
severity: warning
ignore_multiline_type_headers: false
Expand Down