Warn if a configured rule is not enabled

[marcelofabri]

Closes #1350

Pending tests.

[SwiftLintBot]

	12 Messages
📖	Linting Aerial with this PR took 0.42s vs 0.38s on master (10% slower)
📖	Linting Alamofire with this PR took 2.98s vs 2.83s on master (5% slower)
📖	Linting Firefox with this PR took 12.76s vs 12.37s on master (3% slower)
📖	Linting Kickstarter with this PR took 19.04s vs 18.14s on master (4% slower)
📖	Linting Moya with this PR took 1.41s vs 1.34s on master (5% slower)
📖	Linting Nimble with this PR took 1.67s vs 1.63s on master (2% slower)
📖	Linting Quick with this PR took 0.53s vs 0.5s on master (6% slower)
📖	Linting Realm with this PR took 2.78s vs 2.61s on master (6% slower)
📖	Linting SourceKitten with this PR took 1.05s vs 0.92s on master (14% slower)
📖	Linting Sourcery with this PR took 4.41s vs 4.1s on master (7% slower)
📖	Linting Swift with this PR took 13.4s vs 12.75s on master (5% slower)
📖	Linting WordPress with this PR took 11.41s vs 10.84s on master (5% slower)

Generated by 🚫 Danger

[codecov-io]

Codecov Report

Merging #1806 into master will decrease coverage by 0.03%.
The diff coverage is 90.66%.

@@            Coverage Diff             @@
##           master    #1806      +/-   ##
==========================================
- Coverage   88.79%   88.76%   -0.04%     
==========================================
  Files         229      229              
  Lines       11284    11315      +31     
==========================================
+ Hits        10020    10044      +24     
- Misses       1264     1271       +7

Impacted Files	Coverage Δ
Source/SwiftLintFramework/Models/RuleList.swift	100% <100%> (ø)	⬆️
...s/SwiftLintFrameworkTests/ConfigurationTests.swift	92.52% <100%> (ø)	⬆️
...urce/SwiftLintFramework/Models/Configuration.swift	87.97% <100%> (+0.64%)	⬆️
Tests/SwiftLintFrameworkTests/TestHelpers.swift	72.25% <100%> (+0.14%)	⬆️
...ntFramework/Extensions/Configuration+Parsing.swift	90% <69.56%> (-4.45%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5993f6c...6821a2b. Read the comment docs.

[jpsim]

I'm not thrilled about the changes to the public API and the addition of the ConfiguredRule type. Is there any way this could be done by extending Rule instead?

This also requires manually keeping two states in sync: the isDefaultConfiguration flag and whether or not init(configuration:) or configuration.apply(_:) was invoked on the Rule. It'd be nice if the flag could be automatically derived.

At the very least, here are two suggested patches to the current state of this PR:

    Return early in validateConfiguredRulesAreEnabled

diff --git a/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift b/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
index cf653582..2a48c439 100644
--- a/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
+++ b/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
@@ -167,6 +167,10 @@ extension Configuration {
 
     internal static func validateConfiguredRulesAreEnabled(rulesMode: Configuration.RulesMode,
                                                            configuredRules: [ConfiguredRule]) {
+        if case .allEnabled = rulesMode {
+            return
+        }
+
         let rulesWithConfiguration = configuredRules.flatMap {
             $0.isDefaultConfiguration ? nil : $0.rule
         }
@@ -177,7 +181,7 @@ extension Configuration {
 
             switch rulesMode {
             case .allEnabled:
-                return
+                fatalError("Unreachable")
             case .whitelisted(let whitelist):
                 if Set(whitelist).isDisjoint(with: identifiers) {
                     queuedPrintError("\(message), but it is not present on " +

and

    Reuse RuleDescription
    
    to avoid a dynamic runtime type check

diff --git a/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift b/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
index 2a48c439..288a2263 100644
--- a/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
+++ b/Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
@@ -176,8 +176,9 @@ extension Configuration {
         }
 
         for rule in rulesWithConfiguration {
-            let identifiers = type(of: rule).description.allIdentifiers
-            let message = "Found a configuration for '\(type(of: rule).description.identifier)' rule"
+            let ruleDescription = type(of: rule).description
+            let identifiers = ruleDescription.allIdentifiers
+            let message = "Found a configuration for '\(ruleDescription.identifier)' rule"
 
             switch rulesMode {
             case .allEnabled:

[marcelofabri]

The only way I can think of is adding a flag to the Rule (or ConfigurationProviderRule) protocol, but that'd add a requirement that all rules will need to declare that property.

[jpsim]

We'd have to reshuffle quite a few things in order to get it to be automated I think.

Looking at OSSCheck, it looks like this might have a slight impact on performance too. Would you mind confirming if that's the case?