diff --git a/CHANGELOG.md b/CHANGELOG.md index b575048de4..1da6210b02 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index c0403acbaa..6d25c0e66a 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -139,6 +139,7 @@ public let builtInRules: [any Rule.Type] = [ NumberSeparatorRule.self, ObjectLiteralRule.self, OneDeclarationPerFileRule.self, + OpaqueOverExistentialParameterRule.self, OpeningBraceRule.self, OperatorFunctionWhitespaceRule.self, OperatorUsageWhitespaceRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/OpaqueOverExistentialParameterRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/OpaqueOverExistentialParameterRule.swift new file mode 100644 index 0000000000..5d2765b010 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/OpaqueOverExistentialParameterRule.swift @@ -0,0 +1,82 @@ +import SwiftSyntax + +@SwiftSyntaxRule(correctable: true, optIn: true) +struct OpaqueOverExistentialParameterRule: Rule { + var configuration = SeverityConfiguration(.warning) + + static let description = RuleDescription( + identifier: "opaque_over_existential", + name: "Opaque Over Existential Parameter", + description: "Prefer opaque type 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() }) {}"), + ], + 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 { + 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)) + } + } +} + +private extension TypeSyntax { + var isMetaType: Bool { + if `is`(MetatypeTypeSyntax.self) { + return true + } + if let suppressedType = `as`(SuppressedTypeSyntax.self) { + return suppressedType.type.isMetaType + } + return false + } +} diff --git a/Source/SwiftLintCore/Protocols/CollectingRule.swift b/Source/SwiftLintCore/Protocols/CollectingRule.swift index 3bacde8e81..b641979398 100644 --- a/Source/SwiftLintCore/Protocols/CollectingRule.swift +++ b/Source/SwiftLintCore/Protocols/CollectingRule.swift @@ -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) + } } } diff --git a/Source/SwiftLintCore/Protocols/Rule.swift b/Source/SwiftLintCore/Protocols/Rule.swift index 5e0214b678..1d3500f615 100644 --- a/Source/SwiftLintCore/Protocols/Rule.swift +++ b/Source/SwiftLintCore/Protocols/Rule.swift @@ -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`. /// @@ -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 } diff --git a/Source/swiftlint/Commands/Rules.swift b/Source/swiftlint/Commands/Rules.swift index ea79fc957a..930b41e317 100644 --- a/Source/swiftlint/Commands/Rules.swift +++ b/Source/swiftlint/Commands/Rules.swift @@ -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, @@ -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 @@ -76,7 +73,7 @@ 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):") @@ -84,12 +81,10 @@ extension SwiftLint { } } - 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() } } } diff --git a/Tests/GeneratedTests/GeneratedTests.swift b/Tests/GeneratedTests/GeneratedTests.swift index 6ef660cb4d..e79cad68f0 100644 --- a/Tests/GeneratedTests/GeneratedTests.swift +++ b/Tests/GeneratedTests/GeneratedTests.swift @@ -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) diff --git a/Tests/IntegrationTests/default_rule_configurations.yml b/Tests/IntegrationTests/default_rule_configurations.yml index 717d6aec93..a511fc26a8 100644 --- a/Tests/IntegrationTests/default_rule_configurations.yml +++ b/Tests/IntegrationTests/default_rule_configurations.yml @@ -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