[#3477] Fix bug that prevented the reconfiguration of a custom rule i…

…n a child config
realm · Jan 14, 2021 · 2d17045 · 2d17045
1 parent a39e72e
commit 2d17045
Show file tree

Hide file tree

Showing 5 changed files with 55 additions and 17 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -25,17 +25,21 @@
 
 #### Bug Fixes
 
+* Fix `custom_rules` merging when the parent configuration is based on `only_rules`.  
+  [Frederick Pietschmann](https://github.com/fredpi)
+  [#3468](https://github.com/realm/SwiftLint/issues/3468)
+
 * Fix misleading warnings about rules defined in the `custom_rules` not
   being available (when using multiple configurations).  
   [Frederick Pietschmann](https://github.com/fredpi)
   [#3472](https://github.com/realm/SwiftLint/issues/3472)
 
-* Fix `custom_rules` merging when the parent configuration is based on `only_rules`.  
+* Fix bug that prevented the reconfiguration of a custom rule in a child config.  
   [Frederick Pietschmann](https://github.com/fredpi)
-  [#3468](https://github.com/realm/SwiftLint/issues/3468)
+  [#3477](https://github.com/realm/SwiftLint/issues/3477)
 
-  * Fix typos in configuration options for `file_name` rule.  
-    [advantis](https://github.com/advantis)
+* Fix typos in configuration options for `file_name` rule.  
+  [advantis](https://github.com/advantis)
 
 ## 0.42.0: He Chutes, He Scores
 

diff --git a/Source/SwiftLintFramework/Extensions/Configuration+RulesWrapper.swift b/Source/SwiftLintFramework/Extensions/Configuration+RulesWrapper.swift
@@ -205,10 +205,12 @@ internal extension Configuration {
 
             // Create new custom rules rule, prioritizing child custom rules
             var configuration = CustomRulesConfiguration()
-            configuration.customRuleConfigurations = Array(
-                Set(childCustomRulesRule.configuration.customRuleConfigurations)
-                    .union(Set(parentCustomRulesRule.configuration.customRuleConfigurations))
-            )
+            configuration.customRuleConfigurations = childCustomRulesRule.configuration.customRuleConfigurations
+                + parentCustomRulesRule.configuration.customRuleConfigurations.filter { parentConfig in
+                    !childCustomRulesRule.configuration.customRuleConfigurations.contains { childConfig in
+                        childConfig.identifier == parentConfig.identifier
+                    }
+                }
             var newCustomRulesRule = CustomRules()
             newCustomRulesRule.configuration = configuration
 

diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests+Mock.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests+Mock.swift
@@ -45,6 +45,9 @@ internal extension ConfigurationTests {
             static var _2CustomRulesDisabled: String {
                 Dir.level2.stringByAppendingPathComponent("custom_rules_disabled.yml")
             }
+            static var _2CustomRulesReconfig: String {
+                Dir.level2.stringByAppendingPathComponent("custom_rules_reconfig.yml")
+            }
             static var _3: String { Dir.level3.stringByAppendingPathComponent(Configuration.defaultFileName) }
             static var nested: String { Dir.nested.stringByAppendingPathComponent(Configuration.defaultFileName) }
         }
@@ -70,6 +73,9 @@ internal extension ConfigurationTests {
             static var _2CustomRulesDisabled: Configuration {
                 Configuration(configurationFiles: [Yml._2CustomRulesDisabled])
             }
+            static var _2CustomRulesReconfig: Configuration {
+                Configuration(configurationFiles: [Yml._2CustomRulesReconfig])
+            }
             static var _3: Configuration { Configuration(configurationFiles: [Yml._3]) }
             static var nested: Configuration { Configuration(configurationFiles: [Yml.nested]) }
         }

diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests+MultipleConfigs.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests+MultipleConfigs.swift
@@ -76,13 +76,13 @@ extension ConfigurationTests {
         )
         guard let mergedCustomRules = mergedConfiguration.rules.first(where: { $0 is CustomRules }) as? CustomRules
             else {
-            return XCTFail("Custom rule are expected to be present")
+            return XCTFail("Custom rules are expected to be present")
         }
         XCTAssertTrue(
-            mergedCustomRules.configuration.customRuleConfigurations.contains(where: { $0.identifier == "no_abc" })
+            mergedCustomRules.configuration.customRuleConfigurations.contains { $0.identifier == "no_abc" }
         )
         XCTAssertTrue(
-            mergedCustomRules.configuration.customRuleConfigurations.contains(where: { $0.identifier == "no_abcd" })
+            mergedCustomRules.configuration.customRuleConfigurations.contains { $0.identifier == "no_abcd" }
         )
     }
 
@@ -93,13 +93,13 @@ extension ConfigurationTests {
         )
         guard let mergedCustomRules = mergedConfiguration.rules.first(where: { $0 is CustomRules }) as? CustomRules
             else {
-            return XCTFail("Custom rule are expected to be present")
+            return XCTFail("Custom rules are expected to be present")
         }
         XCTAssertFalse(
-            mergedCustomRules.configuration.customRuleConfigurations.contains(where: { $0.identifier == "no_abc" })
+            mergedCustomRules.configuration.customRuleConfigurations.contains { $0.identifier == "no_abc" }
         )
         XCTAssertTrue(
-            mergedCustomRules.configuration.customRuleConfigurations.contains(where: { $0.identifier == "no_abcd" })
+            mergedCustomRules.configuration.customRuleConfigurations.contains { $0.identifier == "no_abcd" }
         )
     }
 
@@ -110,16 +110,37 @@ extension ConfigurationTests {
         )
         guard let mergedCustomRules = mergedConfiguration.rules.first(where: { $0 is CustomRules }) as? CustomRules
             else {
-            return XCTFail("Custom rule are expected to be present")
+            return XCTFail("Custom rules are expected to be present")
         }
         XCTAssertTrue(
-            mergedCustomRules.configuration.customRuleConfigurations.contains(where: { $0.identifier == "no_abc" })
+            mergedCustomRules.configuration.customRuleConfigurations.contains { $0.identifier == "no_abc" }
         )
         XCTAssertTrue(
-            mergedCustomRules.configuration.customRuleConfigurations.contains(where: { $0.identifier == "no_abcd" })
+            mergedCustomRules.configuration.customRuleConfigurations.contains { $0.identifier == "no_abcd" }
         )
     }
 
+    func testCustomRulesReconfiguration() {
+        // Custom Rule severity gets reconfigured to "error"
+        let mergedConfiguration = Mock.Config._0CustomRulesOnly.merged(
+            withChild: Mock.Config._2CustomRulesReconfig,
+            rootDirectory: ""
+        )
+        guard let mergedCustomRules = mergedConfiguration.rules.first(where: { $0 is CustomRules }) as? CustomRules
+            else {
+            return XCTFail("Custom rules are expected to be present")
+        }
+        XCTAssertEqual(
+            mergedCustomRules.configuration.customRuleConfigurations.filter { $0.identifier == "no_abc" }.count, 1
+        )
+        guard let customRule = (mergedCustomRules.configuration.customRuleConfigurations.first {
+            $0.identifier == "no_abc"
+        }) else {
+            return XCTFail("Custom rule is expected to be present")
+        }
+        XCTAssertEqual(customRule.severityConfiguration.severity, .error)
+    }
+
     // MARK: - Nested Configurations
     func testLevel0() {
         XCTAssertEqual(Mock.Config._0.configuration(for: SwiftLintFile(path: Mock.Swift._0)!),

diff --git a/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/Level1/Level2/custom_rules_reconfig.yml b/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/Level1/Level2/custom_rules_reconfig.yml
@@ -0,0 +1,5 @@
+custom_rules:
+  no_abc:
+    name: "Don't use abc"
+    regex: 'abc'
+    severity: 'error'