Skip to content

Commit

Permalink
Fix --only-rule config issues (realm#5773)
Browse files Browse the repository at this point in the history
  • Loading branch information
mildm8nnered authored Oct 25, 2024
1 parent e87efff commit 9ea4374
Show file tree
Hide file tree
Showing 16 changed files with 186 additions and 91 deletions.
9 changes: 9 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,15 @@
[Martin Redington](https://github.com/mildm8nnered)
[#5788](https://github.com/realm/SwiftLint/issues/5788)

* Fixes the `--only-rule` command line option, when a default `.swiftlint.yml`
is absent. Additionally rules specified with `--only-rule` on the command
line can now be disabled in a child configuration, to allow specific
directories to be excluded from the rule (or from being auto-corrected by
the rule), and `--only-rule` can now be specified multiple times
to run multiple rules.
[Martin Redington](https://github.com/mildm8nnered)
[#5711](https://github.com/realm/SwiftLint/issues/5711)

## 0.57.0: Squeaky Clean Cycle

#### Breaking
Expand Down
4 changes: 2 additions & 2 deletions Source/SwiftLintCore/Extensions/Configuration+FileGraph.swift
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ package extension Configuration {
// MARK: - Methods
internal mutating func resultingConfiguration(
enableAllRules: Bool,
onlyRule: String?,
onlyRule: [String],
cachePath: String?
) throws -> Configuration {
// Build if needed
Expand Down Expand Up @@ -250,7 +250,7 @@ package extension Configuration {
private func merged(
configurationData: [(configurationDict: [String: Any], rootDirectory: String)],
enableAllRules: Bool,
onlyRule: String?,
onlyRule: [String],
cachePath: String?
) throws -> Configuration {
// Split into first & remainder; use empty dict for first if the array is empty
Expand Down
20 changes: 11 additions & 9 deletions Source/SwiftLintCore/Extensions/Configuration+Parsing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ extension Configuration {
dict: [String: Any],
ruleList: RuleList = RuleRegistry.shared.list,
enableAllRules: Bool = false,
onlyRule: String? = nil,
onlyRule: [String] = [],
cachePath: String? = nil
) throws {
func defaultStringArray(_ object: Any?) -> [String] { [String].array(of: object) ?? [] }
Expand Down Expand Up @@ -81,7 +81,7 @@ extension Configuration {
analyzerRules: analyzerRules
)

if onlyRule == nil {
if onlyRule.isEmpty {
Self.validateConfiguredRulesAreEnabled(
parentConfiguration: parentConfiguration,
configurationDictionary: dict,
Expand Down Expand Up @@ -174,12 +174,12 @@ extension Configuration {
}

switch rulesMode {
case .allEnabled:
case .allCommandLine, .onlyCommandLine:
return
case .only(let onlyRules):
case .onlyConfiguration(let onlyRules):
let issue = validateConfiguredRuleIsEnabled(onlyRules: onlyRules, ruleType: ruleType)
issue?.print()
case let .default(disabled: disabledRules, optIn: optInRules):
case let .defaultConfiguration(disabled: disabledRules, optIn: optInRules):
let issue = validateConfiguredRuleIsEnabled(
parentConfiguration: parentConfiguration,
disabledRules: disabledRules,
Expand All @@ -201,9 +201,11 @@ extension Configuration {
var disabledInParentRules: Set<String> = []
var allEnabledRules: Set<String> = []

if case .only(let onlyRules) = parentConfiguration?.rulesMode {
if case .onlyConfiguration(let onlyRules) = parentConfiguration?.rulesMode {
enabledInParentRules = onlyRules
} else if case .default(let parentDisabledRules, let parentOptInRules) = parentConfiguration?.rulesMode {
} else if case .defaultConfiguration(
let parentDisabledRules, let parentOptInRules
) = parentConfiguration?.rulesMode {
enabledInParentRules = parentOptInRules
disabledInParentRules = parentDisabledRules
}
Expand Down Expand Up @@ -243,7 +245,7 @@ extension Configuration {
allEnabledRules: Set<String>,
ruleType: any Rule.Type
) -> Issue? {
if case .allEnabled = parentConfiguration?.rulesMode {
if case .allCommandLine = parentConfiguration?.rulesMode {
if disabledRules.contains(ruleType.identifier) {
return Issue.ruleDisabledInDisabledRules(ruleID: ruleType.identifier)
}
Expand All @@ -264,7 +266,7 @@ extension Configuration {
if enabledInParentRules.union(optInRules).isDisjoint(with: allIdentifiers) {
return Issue.ruleNotEnabledInOptInRules(ruleID: ruleType.identifier)
}
} else if case .only(let enabledInParentRules) = parentConfiguration?.rulesMode,
} else if case .onlyConfiguration(let enabledInParentRules) = parentConfiguration?.rulesMode,
enabledInParentRules.isDisjoint(with: allIdentifiers) {
return Issue.ruleNotEnabledInParentOnlyRules(ruleID: ruleType.identifier)
}
Expand Down
47 changes: 29 additions & 18 deletions Source/SwiftLintCore/Extensions/Configuration+RulesMode.swift
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,21 @@ public extension Configuration {
/// The default rules mode, which will enable all rules that aren't defined as being opt-in
/// (conforming to the `OptInRule` protocol), minus the rules listed in `disabled`, plus the rules listed in
/// `optIn`.
case `default`(disabled: Set<String>, optIn: Set<String>)
case defaultConfiguration(disabled: Set<String>, optIn: Set<String>)

/// Only enable the rules explicitly listed.
case only(Set<String>)
/// Only enable the rules explicitly listed in the configuration files.
case onlyConfiguration(Set<String>)

/// Only enable the rule(s) explicitly listed on the command line (and their aliases). `--only-rule` can be
/// specified multiple times to enable multiple rules.
case onlyCommandLine(Set<String>)

/// Enable all available rules.
case allEnabled
case allCommandLine

internal init(
enableAllRules: Bool,
onlyRule: String?,
onlyRule: [String],
onlyRules: [String],
optInRules: [String],
disabledRules: [String],
Expand All @@ -48,9 +52,9 @@ public extension Configuration {
}

if enableAllRules {
self = .allEnabled
} else if let onlyRule {
self = .only(Set([onlyRule]))
self = .allCommandLine
} else if onlyRule.isNotEmpty {
self = .onlyCommandLine(Set(onlyRule))
} else if onlyRules.isNotEmpty {
if disabledRules.isNotEmpty || optInRules.isNotEmpty {
throw Issue.genericWarning(
Expand All @@ -61,7 +65,7 @@ public extension Configuration {
}

warnAboutDuplicates(in: onlyRules + analyzerRules)
self = .only(Set(onlyRules + analyzerRules))
self = .onlyConfiguration(Set(onlyRules + analyzerRules))
} else {
warnAboutDuplicates(in: disabledRules)

Expand All @@ -86,33 +90,40 @@ public extension Configuration {
}

warnAboutDuplicates(in: effectiveOptInRules + effectiveAnalyzerRules)
self = .default(disabled: Set(disabledRules), optIn: Set(effectiveOptInRules + effectiveAnalyzerRules))
self = .defaultConfiguration(
disabled: Set(disabledRules), optIn: Set(effectiveOptInRules + effectiveAnalyzerRules)
)
}
}

internal func applied(aliasResolver: (String) -> String) -> Self {
switch self {
case let .default(disabled, optIn):
return .default(
case let .defaultConfiguration(disabled, optIn):
return .defaultConfiguration(
disabled: Set(disabled.map(aliasResolver)),
optIn: Set(optIn.map(aliasResolver))
)

case let .only(onlyRules):
return .only(Set(onlyRules.map(aliasResolver)))
case let .onlyConfiguration(onlyRules):
return .onlyConfiguration(Set(onlyRules.map(aliasResolver)))

case let .onlyCommandLine(onlyRules):
return .onlyCommandLine(Set(onlyRules.map(aliasResolver)))

case .allEnabled:
return .allEnabled
case .allCommandLine:
return .allCommandLine
}
}

internal func activateCustomRuleIdentifiers(allRulesWrapped: [ConfigurationRuleWrapper]) -> Self {
// In the only mode, if the custom rules rule is enabled, all custom rules are also enabled implicitly
// This method makes the implicitly explicit
switch self {
case let .only(onlyRules) where onlyRules.contains { $0 == CustomRules.description.identifier }:
case let .onlyConfiguration(onlyRules) where onlyRules.contains {
$0 == CustomRules.identifier
}:
let customRulesRule = (allRulesWrapped.first { $0.rule is CustomRules })?.rule as? CustomRules
return .only(onlyRules.union(Set(customRulesRule?.customRuleIdentifiers ?? [])))
return .onlyConfiguration(onlyRules.union(Set(customRulesRule?.customRuleIdentifiers ?? [])))

default:
return self
Expand Down
50 changes: 31 additions & 19 deletions Source/SwiftLintCore/Extensions/Configuration+RulesWrapper.swift
Original file line number Diff line number Diff line change
Expand Up @@ -34,18 +34,18 @@ internal extension Configuration {
let customRulesFilter: (RegexConfiguration<CustomRules>) -> (Bool)
var resultingRules = [any Rule]()
switch mode {
case .allEnabled:
case .allCommandLine:
customRulesFilter = { _ in true }
resultingRules = allRulesWrapped.map(\.rule)

case var .only(onlyRulesRuleIdentifiers):
case let .onlyConfiguration(onlyRulesRuleIdentifiers), let .onlyCommandLine(onlyRulesRuleIdentifiers):
customRulesFilter = { onlyRulesRuleIdentifiers.contains($0.identifier) }
onlyRulesRuleIdentifiers = validate(ruleIds: onlyRulesRuleIdentifiers, valid: validRuleIdentifiers)
let onlyRulesRuleIdentifiers = validate(ruleIds: onlyRulesRuleIdentifiers, valid: validRuleIdentifiers)
resultingRules = allRulesWrapped.filter { tuple in
onlyRulesRuleIdentifiers.contains(type(of: tuple.rule).description.identifier)
}.map(\.rule)

case var .default(disabledRuleIdentifiers, optInRuleIdentifiers):
case var .defaultConfiguration(disabledRuleIdentifiers, optInRuleIdentifiers):
customRulesFilter = { !disabledRuleIdentifiers.contains($0.identifier) }
disabledRuleIdentifiers = validate(ruleIds: disabledRuleIdentifiers, valid: validRuleIdentifiers)
optInRuleIdentifiers = validate(optInRuleIds: optInRuleIdentifiers, valid: validRuleIdentifiers)
Expand Down Expand Up @@ -75,11 +75,11 @@ internal extension Configuration {

lazy var disabledRuleIdentifiers: [String] = {
switch mode {
case let .default(disabled, _):
case let .defaultConfiguration(disabled, _):
return validate(ruleIds: disabled, valid: validRuleIdentifiers, silent: true)
.sorted(by: <)

case let .only(onlyRules):
case let .onlyConfiguration(onlyRules), let .onlyCommandLine(onlyRules):
return validate(
ruleIds: Set(allRulesWrapped
.map { type(of: $0.rule).description.identifier }
Expand All @@ -88,7 +88,7 @@ internal extension Configuration {
silent: true
).sorted(by: <)

case .allEnabled:
case .allCommandLine:
return []
}
}()
Expand Down Expand Up @@ -147,7 +147,7 @@ internal extension Configuration {
let validRuleIdentifiers = self.validRuleIdentifiers.union(child.validRuleIdentifiers)
let newMode: RulesMode
switch child.mode {
case let .default(childDisabled, childOptIn):
case let .defaultConfiguration(childDisabled, childOptIn):
newMode = mergeDefaultMode(
newAllRulesWrapped: newAllRulesWrapped,
child: child,
Expand All @@ -156,13 +156,21 @@ internal extension Configuration {
validRuleIdentifiers: validRuleIdentifiers
)

case let .only(childOnlyRules):
// Always use the child only rules
newMode = .only(childOnlyRules)
case let .onlyConfiguration(childOnlyRules):
// Use the child only rules, unless the parent is onlyRule
switch mode {
case let .onlyCommandLine(onlyRules):
newMode = .onlyCommandLine(onlyRules)
default:
newMode = .onlyConfiguration(childOnlyRules)
}
case let .onlyCommandLine(onlyRules):
// Always use the only rule
newMode = .onlyCommandLine(onlyRules)

case .allEnabled:
case .allCommandLine:
// Always use .allEnabled mode
newMode = .allEnabled
newMode = .allCommandLine
}

// Assemble & return merged rules
Expand Down Expand Up @@ -225,20 +233,20 @@ internal extension Configuration {
let childOptIn = child.validate(optInRuleIds: childOptIn, valid: validRuleIdentifiers)

switch mode { // Switch parent's mode. Child is in default mode.
case var .default(disabled, optIn):
case var .defaultConfiguration(disabled, optIn):
disabled = validate(ruleIds: disabled, valid: validRuleIdentifiers)
optIn = child.validate(optInRuleIds: optIn, valid: validRuleIdentifiers)

// Only use parent disabled / optIn if child config doesn't tell the opposite
return .default(
return .defaultConfiguration(
disabled: Set(childDisabled).union(Set(disabled.filter { !childOptIn.contains($0) })),
optIn: Set(childOptIn).union(Set(optIn.filter { !childDisabled.contains($0) }))
.filter {
isOptInRule($0, allRulesWrapped: newAllRulesWrapped)
}
)

case var .only(onlyRules):
case var .onlyConfiguration(onlyRules):
// Also add identifiers of child custom rules iff the custom_rules rule is enabled
// (parent custom rules are already added)
if (onlyRules.contains { $0 == CustomRules.description.identifier }) {
Expand All @@ -256,13 +264,17 @@ internal extension Configuration {

// Allow parent only rules that weren't disabled via the child config
// & opt-ins from the child config
return .only(Set(
return .onlyConfiguration(Set(
childOptIn.union(onlyRules).filter { !childDisabled.contains($0) }
))

case .allEnabled:
case let .onlyCommandLine(onlyRules):
// Like .allEnabled, rules can be disabled in a child config
return .onlyCommandLine(onlyRules.filter { !childDisabled.contains($0) })

case .allCommandLine:
// Opt-in to every rule that isn't disabled via child config
return .default(
return .defaultConfiguration(
disabled: childDisabled
.filter {
!isOptInRule($0, allRulesWrapped: newAllRulesWrapped)
Expand Down
12 changes: 9 additions & 3 deletions Source/SwiftLintCore/Models/Configuration.swift
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ public struct Configuration {
/// - parameter writeBaseline: The path to write a baseline to.
/// - parameter checkForUpdates: Check for updates to SwiftLint.
package init(
rulesMode: RulesMode = .default(disabled: [], optIn: []),
rulesMode: RulesMode = .defaultConfiguration(disabled: [], optIn: []),
allRulesWrapped: [ConfigurationRuleWrapper]? = nil,
ruleList: RuleList = RuleRegistry.shared.list,
fileGraph: FileGraph? = nil,
Expand Down Expand Up @@ -210,7 +210,7 @@ public struct Configuration {
public init(
configurationFiles: [String], // No default value here to avoid ambiguous Configuration() initializer
enableAllRules: Bool = false,
onlyRule: String? = nil,
onlyRule: [String] = [],
cachePath: String? = nil,
ignoreParentAndChildConfigs: Bool = false,
mockedNetworkResults: [String: String] = [:],
Expand All @@ -230,7 +230,13 @@ public struct Configuration {
defer { basedOnCustomConfigurationFiles = hasCustomConfigurationFiles }

let currentWorkingDirectory = FileManager.default.currentDirectoryPath.bridge().absolutePathStandardized()
let rulesMode: RulesMode = enableAllRules ? .allEnabled : .default(disabled: [], optIn: [])
let rulesMode: RulesMode = if enableAllRules {
.allCommandLine
} else if onlyRule.isNotEmpty {
.onlyCommandLine(Set(onlyRule))
} else {
.defaultConfiguration(disabled: [], optIn: [])
}

// Try obtaining cached config
let cacheIdentifier = "\(currentWorkingDirectory) - \(configurationFiles)"
Expand Down
4 changes: 2 additions & 2 deletions Source/SwiftLintFramework/LintOrAnalyzeCommand.swift
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ package struct LintOrAnalyzeOptions {
let cachePath: String?
let ignoreCache: Bool
let enableAllRules: Bool
let onlyRule: String?
let onlyRule: [String]
let autocorrect: Bool
let format: Bool
let compilerLogPath: String?
Expand Down Expand Up @@ -78,7 +78,7 @@ package struct LintOrAnalyzeOptions {
cachePath: String?,
ignoreCache: Bool,
enableAllRules: Bool,
onlyRule: String?,
onlyRule: [String],
autocorrect: Bool,
format: Bool,
compilerLogPath: String?,
Expand Down
10 changes: 8 additions & 2 deletions Source/swiftlint/Commands/Analyze.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,14 @@ extension SwiftLint {
var compilerLogPath: String?
@Option(help: "The path of a compilation database to use when running AnalyzerRules.")
var compileCommands: String?
@Option(help: "Run only the specified rule, ignoring `only_rules`, `opt_in_rules` and `disabled_rules`.")
var onlyRule: String?
@Option(
parsing: .singleValue,
help: """
Run only the specified rule, ignoring `only_rules`, `opt_in_rules` and `disabled_rules`.
Can be specified repeatedly to run multiple rules.
"""
)
var onlyRule: [String] = []
@Argument(help: pathsArgumentDescription(for: .analyze))
var paths = [String]()

Expand Down
Loading

0 comments on commit 9ea4374

Please sign in to comment.