From 87bfcc68323abe1bd8658a6fcf297d19492efdb2 Mon Sep 17 00:00:00 2001 From: William Laverty Date: Thu, 5 Mar 2026 13:22:21 -0800 Subject: [PATCH] Add `disallow_default_parameter` opt-in rule (#6506) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Danny Mösch --- .swiftlint.yml | 1 + CHANGELOG.md | 5 + .../Models/BuiltInRules.swift | 1 + .../DiscouragedDefaultParameterRule.swift | 91 +++++++++++++++++++ ...ouragedDefaultParameterConfiguration.swift | 21 +++++ Tests/GeneratedTests/GeneratedTests_02.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_03.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_04.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_05.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_06.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_07.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_08.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_09.swift | 12 +-- Tests/GeneratedTests/GeneratedTests_10.swift | 6 ++ .../Resources/default_rule_configurations.yml | 6 ++ 15 files changed, 179 insertions(+), 48 deletions(-) create mode 100644 Source/SwiftLintBuiltInRules/Rules/Lint/DiscouragedDefaultParameterRule.swift create mode 100644 Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/DiscouragedDefaultParameterConfiguration.swift diff --git a/.swiftlint.yml b/.swiftlint.yml index b38db5ca6..3f2746153 100644 --- a/.swiftlint.yml +++ b/.swiftlint.yml @@ -21,6 +21,7 @@ disabled_rules: - conditional_returns_on_newline - contrasted_opening_brace - convenience_type + - discouraged_default_parameter - discouraged_optional_collection - explicit_acl - explicit_enum_raw_value diff --git a/CHANGELOG.md b/CHANGELOG.md index 26939f09a..4f00bac89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,11 @@ [SimplyDanny](https://github.com/SimplyDanny) [#6501](https://github.com/realm/SwiftLint/issues/6501) +* Add `discouraged_default_parameter` opt-in rule that flags default parameter + values in functions with configurable access levels. + [William-Laverty](https://github.com/William-Laverty) + [#6488](https://github.com/realm/SwiftLint/issues/6488) + * Add `ignored_literal_argument_functions` option to the `force_unwrapping` rule to skip violations for configurable function calls when all arguments are literal values (e.g. `URL(string: "https://example.com")!`). Defaults diff --git a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift index d3d2b8221..764095f97 100644 --- a/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift +++ b/Source/SwiftLintBuiltInRules/Models/BuiltInRules.swift @@ -39,6 +39,7 @@ public let builtInRules: [any Rule.Type] = [ DirectReturnRule.self, DiscardedNotificationCenterObserverRule.self, DiscouragedAssertRule.self, + DiscouragedDefaultParameterRule.self, DiscouragedDirectInitRule.self, DiscouragedNoneNameRule.self, DiscouragedObjectLiteralRule.self, diff --git a/Source/SwiftLintBuiltInRules/Rules/Lint/DiscouragedDefaultParameterRule.swift b/Source/SwiftLintBuiltInRules/Rules/Lint/DiscouragedDefaultParameterRule.swift new file mode 100644 index 000000000..ed4f15f49 --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/Lint/DiscouragedDefaultParameterRule.swift @@ -0,0 +1,91 @@ +import SwiftSyntax + +@SwiftSyntaxRule(optIn: true) +struct DiscouragedDefaultParameterRule: Rule { + var configuration = DiscouragedDefaultParameterConfiguration() + + static let description = RuleDescription( + identifier: "discouraged_default_parameter", + name: "Discouraged Default Parameter", + description: "Default parameter values should not be used in functions with certain access levels.", + rationale: """ + By discouraging default parameter values in functions, that are exposed to other source files in the module + or package and their consumers, we can promote call sites and reduce the likelihood of bugs caused by + unexpected (or changed) default values being used. + """, + kind: .lint, + nonTriggeringExamples: [ + Example("public func foo(bar: Int = 0) {}"), + Example("open func foo(bar: Int = 0) {}"), + Example("func foo(bar: Int) {}"), + Example("private func foo(bar: Int = 0) {}"), + Example("fileprivate func foo(bar: Int = 0) {}"), + Example("public init(value: Int = 42) {}"), + Example( + "func foo(bar: Int = 0) {}", + configuration: ["disallowed_access_levels": ["private"]] + ), + ], + triggeringExamples: [ + Example("func foo(bar: Int ↓= 0) {}"), + Example("internal func foo(bar: Int ↓= 0) {}"), + Example("package func foo(bar: Int ↓= 0) {}"), + Example("func foo(bar: Int ↓= 0, baz: String ↓= \"\") {}"), + Example("init(value: Int ↓= 42) {}"), + Example( + "private func foo(bar: Int ↓= 0) {}", + configuration: ["disallowed_access_levels": ["private"]] + ), + Example( + "fileprivate func foo(bar: Int ↓= 0) {}", + configuration: ["disallowed_access_levels": ["fileprivate"]] + ), + ] + ) +} + +private extension DiscouragedDefaultParameterRule { + final class Visitor: ViolationsSyntaxVisitor { + override func visitPost(_ node: FunctionDeclSyntax) { + collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause) + } + + override func visitPost(_ node: InitializerDeclSyntax) { + collectViolations(modifiers: node.modifiers, parameterClause: node.signature.parameterClause) + } + + override func visitPost(_ node: SubscriptDeclSyntax) { + collectViolations(modifiers: node.modifiers, parameterClause: node.parameterClause) + } + + private func collectViolations( + modifiers: DeclModifierListSyntax, + parameterClause: FunctionParameterClauseSyntax + ) { + guard let accessLevel = effectiveAccessLevel(modifiers), + configuration.disallowedAccessLevels.contains(accessLevel) else { + return + } + let levelName = accessLevel.rawValue + for param in parameterClause.parameters { + if let defaultValue = param.defaultValue { + violations.append( + ReasonedRuleViolation( + position: defaultValue.positionAfterSkippingLeadingTrivia, + reason: "Default parameter values should not be used in '\(levelName)' functions" + ) + ) + } + } + } + + private func effectiveAccessLevel(_ modifiers: DeclModifierListSyntax) + -> DiscouragedDefaultParameterConfiguration.AccessLevel? { + if modifiers.contains(keyword: .private) { return .private } + if modifiers.contains(keyword: .fileprivate) { return .fileprivate } + if modifiers.contains(keyword: .package) { return .package } + if modifiers.contains(keyword: .public) || modifiers.contains(keyword: .open) { return nil } + return .internal + } + } +} diff --git a/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/DiscouragedDefaultParameterConfiguration.swift b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/DiscouragedDefaultParameterConfiguration.swift new file mode 100644 index 000000000..1df96dc9d --- /dev/null +++ b/Source/SwiftLintBuiltInRules/Rules/RuleConfigurations/DiscouragedDefaultParameterConfiguration.swift @@ -0,0 +1,21 @@ +import SwiftLintCore + +@AutoConfigParser +struct DiscouragedDefaultParameterConfiguration: SeverityBasedRuleConfiguration { + @ConfigurationElement(key: "severity") + private(set) var severityConfiguration = SeverityConfiguration(.warning) + @ConfigurationElement(key: "disallowed_access_levels") + private(set) var disallowedAccessLevels: Set = [.internal, .package] + + @AcceptableByConfigurationElement + enum AccessLevel: String, Comparable { + case `private` + case `fileprivate` + case `internal` + case `package` + + static func < (lhs: Self, rhs: Self) -> Bool { + lhs.rawValue < rhs.rawValue + } + } +} diff --git a/Tests/GeneratedTests/GeneratedTests_02.swift b/Tests/GeneratedTests/GeneratedTests_02.swift index c8d05b274..da1a25175 100644 --- a/Tests/GeneratedTests/GeneratedTests_02.swift +++ b/Tests/GeneratedTests/GeneratedTests_02.swift @@ -79,6 +79,12 @@ final class DiscouragedAssertRuleGeneratedTests: SwiftLintTestCase { } } +final class DiscouragedDefaultParameterRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(DiscouragedDefaultParameterRule.description) + } +} + final class DiscouragedDirectInitRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(DiscouragedDirectInitRule.description) @@ -150,9 +156,3 @@ final class EmptyCountRuleGeneratedTests: SwiftLintTestCase { verifyRule(EmptyCountRule.description) } } - -final class EmptyEnumArgumentsRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(EmptyEnumArgumentsRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_03.swift b/Tests/GeneratedTests/GeneratedTests_03.swift index d90a7c116..0ee3ec83b 100644 --- a/Tests/GeneratedTests/GeneratedTests_03.swift +++ b/Tests/GeneratedTests/GeneratedTests_03.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class EmptyEnumArgumentsRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(EmptyEnumArgumentsRule.description) + } +} + final class EmptyParametersRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(EmptyParametersRule.description) @@ -150,9 +156,3 @@ final class ForWhereRuleGeneratedTests: SwiftLintTestCase { verifyRule(ForWhereRule.description) } } - -final class ForceCastRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(ForceCastRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_04.swift b/Tests/GeneratedTests/GeneratedTests_04.swift index 20a6b1a48..65ac6b2d3 100644 --- a/Tests/GeneratedTests/GeneratedTests_04.swift +++ b/Tests/GeneratedTests/GeneratedTests_04.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class ForceCastRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(ForceCastRule.description) + } +} + final class ForceTryRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(ForceTryRule.description) @@ -150,9 +156,3 @@ final class LegacyCGGeometryFunctionsRuleGeneratedTests: SwiftLintTestCase { verifyRule(LegacyCGGeometryFunctionsRule.description) } } - -final class LegacyConstantRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(LegacyConstantRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_05.swift b/Tests/GeneratedTests/GeneratedTests_05.swift index 748d2cf3b..4d375ad87 100644 --- a/Tests/GeneratedTests/GeneratedTests_05.swift +++ b/Tests/GeneratedTests/GeneratedTests_05.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class LegacyConstantRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(LegacyConstantRule.description) + } +} + final class LegacyConstructorRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(LegacyConstructorRule.description) @@ -150,9 +156,3 @@ final class NSLocalizedStringRequireBundleRuleGeneratedTests: SwiftLintTestCase verifyRule(NSLocalizedStringRequireBundleRule.description) } } - -final class NSNumberInitAsFunctionReferenceRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(NSNumberInitAsFunctionReferenceRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_06.swift b/Tests/GeneratedTests/GeneratedTests_06.swift index 4e16ca229..3b33529a2 100644 --- a/Tests/GeneratedTests/GeneratedTests_06.swift +++ b/Tests/GeneratedTests/GeneratedTests_06.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class NSNumberInitAsFunctionReferenceRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(NSNumberInitAsFunctionReferenceRule.description) + } +} + final class NSObjectPreferIsEqualRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(NSObjectPreferIsEqualRule.description) @@ -150,9 +156,3 @@ final class PeriodSpacingRuleGeneratedTests: SwiftLintTestCase { verifyRule(PeriodSpacingRule.description) } } - -final class PreferAssetSymbolsRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(PreferAssetSymbolsRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_07.swift b/Tests/GeneratedTests/GeneratedTests_07.swift index 257050f83..488b71647 100644 --- a/Tests/GeneratedTests/GeneratedTests_07.swift +++ b/Tests/GeneratedTests/GeneratedTests_07.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class PreferAssetSymbolsRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(PreferAssetSymbolsRule.description) + } +} + final class PreferConditionListRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(PreferConditionListRule.description) @@ -150,9 +156,3 @@ final class RedundantDiscardableLetRuleGeneratedTests: SwiftLintTestCase { verifyRule(RedundantDiscardableLetRule.description) } } - -final class RedundantNilCoalescingRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(RedundantNilCoalescingRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_08.swift b/Tests/GeneratedTests/GeneratedTests_08.swift index bb3481ea8..a18c6bdb9 100644 --- a/Tests/GeneratedTests/GeneratedTests_08.swift +++ b/Tests/GeneratedTests/GeneratedTests_08.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class RedundantNilCoalescingRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(RedundantNilCoalescingRule.description) + } +} + final class RedundantObjcAttributeRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(RedundantObjcAttributeRule.description) @@ -150,9 +156,3 @@ final class StrictFilePrivateRuleGeneratedTests: SwiftLintTestCase { verifyRule(StrictFilePrivateRule.description) } } - -final class StrongIBOutletRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(StrongIBOutletRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_09.swift b/Tests/GeneratedTests/GeneratedTests_09.swift index 73827e518..8195d79e6 100644 --- a/Tests/GeneratedTests/GeneratedTests_09.swift +++ b/Tests/GeneratedTests/GeneratedTests_09.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class StrongIBOutletRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(StrongIBOutletRule.description) + } +} + final class SuperfluousElseRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(SuperfluousElseRule.description) @@ -150,9 +156,3 @@ final class UnneededSynthesizedInitializerRuleGeneratedTests: SwiftLintTestCase verifyRule(UnneededSynthesizedInitializerRule.description) } } - -final class UnneededThrowsRuleGeneratedTests: SwiftLintTestCase { - func testWithDefaultConfiguration() { - verifyRule(UnneededThrowsRule.description) - } -} diff --git a/Tests/GeneratedTests/GeneratedTests_10.swift b/Tests/GeneratedTests/GeneratedTests_10.swift index 002ad994e..b647b6a30 100644 --- a/Tests/GeneratedTests/GeneratedTests_10.swift +++ b/Tests/GeneratedTests/GeneratedTests_10.swift @@ -7,6 +7,12 @@ @testable import SwiftLintCore import TestHelpers +final class UnneededThrowsRuleGeneratedTests: SwiftLintTestCase { + func testWithDefaultConfiguration() { + verifyRule(UnneededThrowsRule.description) + } +} + final class UnownedVariableCaptureRuleGeneratedTests: SwiftLintTestCase { func testWithDefaultConfiguration() { verifyRule(UnownedVariableCaptureRule.description) diff --git a/Tests/IntegrationTests/Resources/default_rule_configurations.yml b/Tests/IntegrationTests/Resources/default_rule_configurations.yml index 983a06776..cb679168b 100644 --- a/Tests/IntegrationTests/Resources/default_rule_configurations.yml +++ b/Tests/IntegrationTests/Resources/default_rule_configurations.yml @@ -205,6 +205,12 @@ discouraged_assert: meta: opt-in: true correctable: false +discouraged_default_parameter: + severity: warning + disallowed_access_levels: [internal, package] + meta: + opt-in: true + correctable: false discouraged_direct_init: severity: warning types: ["Bundle", "Bundle.init", "NSError", "NSError.init", "UIDevice", "UIDevice.init"]