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.

Sign up for GitHub

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

support opt-in rules #345

Merged
merged 2 commits into from
Jan 12, 2016
Merged

support opt-in rules #345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2f3b209
support opt-in rules
jpsim Jan 12, 2016
5c34ce6
add changelog entry
jpsim Jan 12, 2016
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,10 @@
[Aaron McTavish](https://github.com/aamctustwo)
[#202](https://github.com/realm/SwiftLint/issues/202)

* Opt-in rules are now supported.
[JP Simard](https://github.com/jpsim)
[#256](https://github.com/realm/SwiftLint/issues/256)

##### Bug Fixes

* AutoCorrect for TrailingNewlineRule only removes at most one line.
Expand Down
18 changes: 9 additions & 9 deletions Source/SwiftLintFramework/Models/Configuration.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ public struct Configuration: Equatable {
}

public init?(disabledRules: [String] = [],
enabledRules: [String] = [],
included: [String] = [],
excluded: [String] = [],
reporter: String = "xcode",
Expand All @@ -48,7 +49,6 @@ public struct Configuration: Equatable {
self.useNestedConfigs = useNestedConfigs

// Validate that all rule identifiers map to a defined rule

let validRuleIdentifiers = Configuration.rulesFromDict().map {
$0.dynamicType.description.identifier
}
Expand All @@ -64,9 +64,7 @@ public struct Configuration: Equatable {
}

// Validate that rule identifiers aren't listed multiple times

let ruleSet = Set(validDisabledRules)
if ruleSet.count != validDisabledRules.count {
if Set(validDisabledRules).count != validDisabledRules.count {
let duplicateRules = validDisabledRules.reduce([String: Int]()) { (var accu, element) in
accu[element] = accu[element]?.successor() ?? 1
return accu
Expand All @@ -78,14 +76,17 @@ public struct Configuration: Equatable {
}
self.disabledRules = validDisabledRules

self.rules = rules.filter {
!validDisabledRules.contains($0.dynamicType.description.identifier)
self.rules = rules.filter { rule in
let id = rule.dynamicType.description.identifier
if validDisabledRules.contains(id) { return false }
return enabledRules.contains(id) || !(rule is OptInRule)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd prefer not to have the OptInRule or possibly a different mechanism for it. It makes enablesRules only useful for opting in to non-default rules, rather a definitive whitelist.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. That's not what I was going for, and not what I outlined in #256 (comment).

What you want is a definitive list of rules to run, ignoring enabled_rules and disabled_rules, which now reading back that comment I linked to, was the motivation for a rules config option.

I see value in that, but it does have to be weighed against the complexity of having a white list, a black list, and a list list 😞 .

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I get it, although I got a different meaning from @scottrhoyt 's comment #256 (comment) which seems to imply either a white list or black list, but not both (or opt in).

}
}

public init?(dict: [String: AnyObject]) {
self.init(
disabledRules: dict["disabled_rules"] as? [String] ?? [],
enabledRules: dict["enabled_rules"] as? [String] ?? [],
included: dict["included"] as? [String] ?? [],
excluded: dict["excluded"] as? [String] ?? [],
reporter: dict["reporter"] as? String ?? XcodeReporter.identifier,
Expand Down Expand Up @@ -119,7 +120,7 @@ public struct Configuration: Equatable {
}

public static func rulesFromDict(dict: [String: AnyObject]? = nil,
ruleList: RuleList = masterRuleList) -> [Rule] {
ruleList: RuleList = masterRuleList) -> [Rule] {
var rules = [Rule]()
for rule in ruleList.list.values {
let identifier = rule.description.identifier
Expand Down Expand Up @@ -148,8 +149,7 @@ public struct Configuration: Equatable {
}

public func lintableFilesForPath(path: String) -> [File] {
let allPaths = self.lintablePathsForPath(path)
return allPaths.flatMap { File(path: $0) }
return lintablePathsForPath(path).flatMap { File(path: $0) }
}

public func configForFile(file: File) -> Configuration {
Expand Down
2 changes: 2 additions & 0 deletions Source/SwiftLintFramework/Protocols/Rule.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

import SourceKittenFramework

public protocol OptInRule {}

public protocol Rule {
init() // Rules need to be able to be initialized with default values
static var description: RuleDescription { get }
Expand Down