Print invalid keys when configuration parsing fails (realm#5350)

coffmark · Nov 14, 2023 · fb4ef95 · fb4ef95
1 parent 9b25efa
commit fb4ef95
Show file tree

Hide file tree

Showing 8 changed files with 30 additions and 12 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,7 +10,9 @@
 
 #### Enhancements
 
-* None.
+* Print invalid keys when configuration parsing fails.  
+  [SimplyDanny](https://github.com/SimplyDanny)
+  [#5347](https://github.com/realm/SwiftLint/pull/5347)
 
 #### Bug Fixes
 

diff --git a/Source/SwiftLintCore/Extensions/Configuration+Parsing.swift b/Source/SwiftLintCore/Extensions/Configuration+Parsing.swift
@@ -142,7 +142,7 @@ extension Configuration {
         // Log an error when supplying invalid keys in the configuration dictionary
         let invalidKeys = Set(dict.keys).subtracting(validKeys(ruleList: ruleList))
         if invalidKeys.isNotEmpty {
-            Issue.invalidConfigurationKeys(invalidKeys.sorted()).print()
+            Issue.invalidRuleIDs(invalidKeys).print()
         }
     }
 

diff --git a/Source/SwiftLintCore/Models/Issue.swift b/Source/SwiftLintCore/Models/Issue.swift
@@ -15,7 +15,10 @@ public enum Issue: LocalizedError, Equatable {
     case invalidConfiguration(ruleID: String)
 
     /// Some configuration keys are invalid.
-    case invalidConfigurationKeys([String])
+    case invalidConfigurationKeys(ruleID: String, keys: Set<String>)
+
+    /// Used rule IDs are invalid.
+    case invalidRuleIDs(Set<String>)
 
     /// A generic warning specified by a string.
     case genericWarning(String)
@@ -94,8 +97,10 @@ public enum Issue: LocalizedError, Equatable {
             return "'\(old)' has been renamed to '\(new)' and will be completely removed in a future release."
         case let .invalidConfiguration(id):
             return "Invalid configuration for '\(id)'. Falling back to default."
-        case let .invalidConfigurationKeys(keys):
-            return "Configuration contains invalid keys \(keys.joined(separator: ", "))."
+        case let .invalidConfigurationKeys(id, keys):
+            return "Configuration for '\(id)' rule contains the invalid key(s) \(keys.formatted)."
+        case let .invalidRuleIDs(ruleIDs):
+            return "The key(s) \(ruleIDs.formatted) used as rule identifier(s) is/are invalid."
         case let .genericWarning(message), let .genericError(message):
             return message
         case let .ruleDeprecated(id):
@@ -123,3 +128,11 @@ public enum Issue: LocalizedError, Equatable {
         }
     }
 }
+
+private extension Set where Element == String {
+    var formatted: String {
+        sorted()
+            .map { "'\($0)'" }
+            .joined(separator: ", ")
+    }
+}
diff --git a/Source/SwiftLintCore/Models/RuleList.swift b/Source/SwiftLintCore/Models/RuleList.swift
@@ -55,10 +55,13 @@ public struct RuleList {
                     rule: configuredRule,
                     initializedWithNonEmptyConfiguration: isConfigured
                 )
+                continue
+            } catch let issue as Issue {
+                issue.print()
             } catch {
                 Issue.invalidConfiguration(ruleID: identifier).print()
-                rules[identifier] = (ruleType.init(), false)
             }
+            rules[identifier] = (ruleType.init(), false)
         }
 
         // Add remaining rules without configuring them

diff --git a/Source/SwiftLintCoreMacros/RuleConfigurationMacros.swift b/Source/SwiftLintCoreMacros/RuleConfigurationMacros.swift
@@ -55,7 +55,7 @@ enum AutoApply: MemberMacro {
                 \(raw: elementsWithKeyUpdate.joined(separator: "\n"))
                 if !supportedKeys.isSuperset(of: configuration.keys) {
                     let unknownKeys = Set(configuration.keys).subtracting(supportedKeys)
-                    throw Issue.invalidConfigurationKeys(unknownKeys.sorted())
+                    throw Issue.invalidConfigurationKeys(ruleID: Parent.identifier, keys: unknownKeys)
                 }
             }
             """

diff --git a/Tests/MacroTests/AutoApplyTests.swift b/Tests/MacroTests/AutoApplyTests.swift
@@ -44,7 +44,7 @@ final class AutoApplyTests: XCTestCase {
 
                     if !supportedKeys.isSuperset(of: configuration.keys) {
                         let unknownKeys = Set(configuration.keys).subtracting(supportedKeys)
-                        throw Issue.invalidConfigurationKeys(unknownKeys.sorted())
+                        throw Issue.invalidConfigurationKeys(ruleID: Parent.identifier, keys: unknownKeys)
                     }
                 }
             }
@@ -81,7 +81,7 @@ final class AutoApplyTests: XCTestCase {
 
                     if !supportedKeys.isSuperset(of: configuration.keys) {
                         let unknownKeys = Set(configuration.keys).subtracting(supportedKeys)
-                        throw Issue.invalidConfigurationKeys(unknownKeys.sorted())
+                        throw Issue.invalidConfigurationKeys(ruleID: Parent.identifier, keys: unknownKeys)
                     }
                 }
             }
@@ -120,7 +120,7 @@ final class AutoApplyTests: XCTestCase {
                     try $e2.performAfterParseOperations()
                     if !supportedKeys.isSuperset(of: configuration.keys) {
                         let unknownKeys = Set(configuration.keys).subtracting(supportedKeys)
-                        throw Issue.invalidConfigurationKeys(unknownKeys.sorted())
+                        throw Issue.invalidConfigurationKeys(ruleID: Parent.identifier, keys: unknownKeys)
                     }
                 }
             }

diff --git a/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceConfigurationTests.swift b/Tests/SwiftLintFrameworkTests/ExplicitTypeInterfaceConfigurationTests.swift
@@ -21,7 +21,7 @@ class ExplicitTypeInterfaceConfigurationTests: SwiftLintTestCase {
 
     func testInvalidKeyInCustomConfiguration() {
         var config = ExplicitTypeInterfaceConfiguration()
-        checkError(Issue.invalidConfigurationKeys(["invalidKey"])) {
+        checkError(Issue.invalidConfigurationKeys(ruleID: ExplicitTypeInterfaceRule.identifier, keys: ["invalidKey"])) {
             try config.apply(configuration: ["invalidKey": "error"])
         }
     }

diff --git a/Tests/SwiftLintFrameworkTests/RuleConfigurationDescriptionTests.swift b/Tests/SwiftLintFrameworkTests/RuleConfigurationDescriptionTests.swift
@@ -494,7 +494,7 @@ class RuleConfigurationDescriptionTests: XCTestCase {
     func testInvalidKeys() throws {
         var configuration = TestConfiguration()
 
-        checkError(Issue.invalidConfigurationKeys(["unknown", "unsupported"])) {
+        checkError(Issue.invalidConfigurationKeys(ruleID: "RuleMock", keys: ["unknown", "unsupported"])) {
             try configuration.apply(configuration: [
                 "severity": "error",
                 "warning": 3,