From 3c05cda73bdd4b2437b088aa4c0df4475f539317 Mon Sep 17 00:00:00 2001 From: Frederick Pietschmann <19194800+fredpi@users.noreply.github.com> Date: Tue, 22 Dec 2020 19:41:40 +0100 Subject: [PATCH] [#3477] Fix bug that prevented the reconfiguration of a custom rule in a child config --- CHANGELOG.md | 12 ++++-- .../Configuration+RulesWrapper.swift | 10 +++-- .../ConfigurationTests+Mock.swift | 6 +++ .../ConfigurationTests+MultipleConfigs.swift | 39 ++++++++++++++----- .../Level1/Level2/custom_rules_reconfig.yml | 5 +++ 5 files changed, 55 insertions(+), 17 deletions(-) create mode 100644 Tests/SwiftLintFrameworkTests/Resources/ProjectMock/Level1/Level2/custom_rules_reconfig.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 03ac75f1d95..1a161a2b5b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,17 +26,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 index 85fa0ea3ef9..229046ab567 100644 --- 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 index 0be5cf636fa..aa459dcacd0 100644 --- 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 index ec804e05897..458b0adfe3b 100644 --- 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 new file mode 100644 index 00000000000..947b02b1951 --- /dev/null +++ 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'