diff --git a/CHANGELOG.md b/CHANGELOG.md index 40a2d53690..7478aea5b5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -48,6 +48,11 @@ [Martin Redington](https://github.com/mildm8nnered) [#5120](https://github.com/realm/SwiftLint/issues/5120) +* Fix some unexpected rule enablement interactions between parent and + child configurations. + [Martin Redington](https://github.com/mildm8nnered) + [#4876](https://github.com/realm/SwiftLint/issues/4876) + ## 0.52.4: Lid Switch #### Breaking diff --git a/Source/SwiftLintCore/Extensions/Configuration+RulesWrapper.swift b/Source/SwiftLintCore/Extensions/Configuration+RulesWrapper.swift index 766ac3420d..6ae6fe5f3f 100644 --- a/Source/SwiftLintCore/Extensions/Configuration+RulesWrapper.swift +++ b/Source/SwiftLintCore/Extensions/Configuration+RulesWrapper.swift @@ -233,10 +233,7 @@ internal extension Configuration { // Only use parent disabled / optIn if child config doesn't tell the opposite return .default( - disabled: Set(childDisabled).union(Set(disabled.filter { !childOptIn.contains($0) })) - .filter { - !isOptInRule($0, allRulesWrapped: newAllRulesWrapped) - }, + disabled: Set(childDisabled).union(Set(disabled.filter { !childOptIn.contains($0) })), optIn: Set(childOptIn).union(Set(optIn.filter { !childDisabled.contains($0) })) .filter { isOptInRule($0, allRulesWrapped: newAllRulesWrapped) @@ -262,7 +259,7 @@ internal extension Configuration { // Allow parent only rules that weren't disabled via the child config // & opt-ins from the child config return .only(Set( - childOptIn + onlyRules.filter { !childDisabled.contains($0) } + childOptIn.union(onlyRules).filter { !childDisabled.contains($0) } )) case .allEnabled: diff --git a/Tests/SwiftLintFrameworkTests/ConfigurationTests+MultipleConfigs.swift b/Tests/SwiftLintFrameworkTests/ConfigurationTests+MultipleConfigs.swift index f6dc87dc18..67d4611214 100644 --- a/Tests/SwiftLintFrameworkTests/ConfigurationTests+MultipleConfigs.swift +++ b/Tests/SwiftLintFrameworkTests/ConfigurationTests+MultipleConfigs.swift @@ -301,6 +301,122 @@ extension ConfigurationTests { ) } + func testParentChildOptInAndDisable() { + struct TestCase: Equatable { + let optedInInParent: Bool + let disabledInParent: Bool + let optedInInChild: Bool + let disabledInChild: Bool + let isEnabled: Bool + var message: String { + "optedInInParent = \(optedInInParent) " + + "disabledInParent = \(disabledInParent) " + + "optedInInChild = \(optedInInChild) " + + "disabledInChild = \(disabledInChild)" + } + } + let testCases: [TestCase] = [ + // swiftlint:disable line_length + TestCase(optedInInParent: false, disabledInParent: false, optedInInChild: false, disabledInChild: false, isEnabled: false), + TestCase(optedInInParent: true, disabledInParent: false, optedInInChild: false, disabledInChild: false, isEnabled: true), + TestCase(optedInInParent: false, disabledInParent: true, optedInInChild: false, disabledInChild: false, isEnabled: false), + TestCase(optedInInParent: true, disabledInParent: true, optedInInChild: false, disabledInChild: false, isEnabled: false), + TestCase(optedInInParent: false, disabledInParent: false, optedInInChild: true, disabledInChild: false, isEnabled: true), + TestCase(optedInInParent: true, disabledInParent: false, optedInInChild: true, disabledInChild: false, isEnabled: true), + TestCase(optedInInParent: false, disabledInParent: true, optedInInChild: true, disabledInChild: false, isEnabled: true), + TestCase(optedInInParent: true, disabledInParent: true, optedInInChild: true, disabledInChild: false, isEnabled: true), + TestCase(optedInInParent: false, disabledInParent: false, optedInInChild: false, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: true, disabledInParent: false, optedInInChild: false, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: false, disabledInParent: true, optedInInChild: false, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: true, disabledInParent: true, optedInInChild: false, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: false, disabledInParent: false, optedInInChild: true, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: true, disabledInParent: false, optedInInChild: true, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: false, disabledInParent: true, optedInInChild: true, disabledInChild: true, isEnabled: false), + TestCase(optedInInParent: true, disabledInParent: true, optedInInChild: true, disabledInChild: true, isEnabled: false) + // swiftlint:enable line_length + ] + XCTAssertEqual(testCases.unique.count, 4 * 4) + let ruleType = ImplicitReturnRule.self + XCTAssertTrue((ruleType as Any) is OptInRule.Type) + let ruleIdentifier = ruleType.description.identifier + for testCase in testCases { + let parentConfiguration = Configuration(rulesMode: .default( + disabled: testCase.disabledInParent ? [ruleIdentifier] : [], + optIn: testCase.optedInInParent ? [ruleIdentifier] : [] + )) + let childConfiguration = Configuration(rulesMode: .default( + disabled: testCase.disabledInChild ? [ruleIdentifier] : [], + optIn: testCase.optedInInChild ? [ruleIdentifier] : [] + )) + let mergedConfiguration = parentConfiguration.merged(withChild: childConfiguration, rootDirectory: "") + let isEnabled = mergedConfiguration.contains(rule: ruleType) + XCTAssertEqual(isEnabled, testCase.isEnabled, testCase.message) + } + } + + func testParentChildDisableForDefaultRule() { + struct TestCase: Equatable { + let disabledInParent: Bool + let disabledInChild: Bool + let isEnabled: Bool + var message: String { + "disabledInParent = \(disabledInParent) disabledInChild = \(disabledInChild)" + } + } + let testCases: [TestCase] = [ + TestCase(disabledInParent: false, disabledInChild: false, isEnabled: true), + TestCase(disabledInParent: true, disabledInChild: false, isEnabled: false), + TestCase(disabledInParent: false, disabledInChild: true, isEnabled: false), + TestCase(disabledInParent: true, disabledInChild: true, isEnabled: false) + ] + XCTAssertEqual(testCases.unique.count, 2 * 2) + let ruleType = BlanketDisableCommandRule.self + XCTAssertFalse(ruleType is OptInRule.Type) + let ruleIdentifier = ruleType.description.identifier + for testCase in testCases { + let parentConfiguration = Configuration( + rulesMode: .default(disabled: testCase.disabledInParent ? [ruleIdentifier] : [], optIn: []) + ) + let childConfiguration = Configuration( + rulesMode: .default(disabled: testCase.disabledInChild ? [ruleIdentifier] : [], optIn: []) + ) + let mergedConfiguration = parentConfiguration.merged(withChild: childConfiguration, rootDirectory: "") + let isEnabled = mergedConfiguration.contains(rule: ruleType) + XCTAssertEqual(isEnabled, testCase.isEnabled, testCase.message) + } + } + + func testParentOnlyRulesAndChildOptInAndDisabled() { + struct TestCase: Equatable { + let optedInInChild: Bool + let disabledInChild: Bool + let isEnabled: Bool + var message: String { + "optedInInChild = \(optedInInChild) disabledInChild = \(disabledInChild)" + } + } + let testCases: [TestCase] = [ + TestCase(optedInInChild: false, disabledInChild: false, isEnabled: true), + TestCase(optedInInChild: true, disabledInChild: false, isEnabled: true), + TestCase(optedInInChild: false, disabledInChild: true, isEnabled: false), + TestCase(optedInInChild: true, disabledInChild: true, isEnabled: false) + ] + XCTAssertEqual(testCases.unique.count, 2 * 2) + let ruleType = ImplicitReturnRule.self + XCTAssertTrue((ruleType as Any) is OptInRule.Type) + let ruleIdentifier = ruleType.description.identifier + let parentConfiguration = Configuration(rulesMode: .only([ruleIdentifier])) + for testCase in testCases { + let childConfiguration = Configuration(rulesMode: .default( + disabled: testCase.disabledInChild ? [ruleIdentifier] : [], + optIn: testCase.optedInInChild ? [ruleIdentifier] : [] + )) + let mergedConfiguration = parentConfiguration.merged(withChild: childConfiguration, rootDirectory: "") + let isEnabled = mergedConfiguration.contains(rule: ruleType) + XCTAssertEqual(isEnabled, testCase.isEnabled, testCase.message) + } + } + // MARK: - Remote Configs func testValidRemoteChildConfig() { FileManager.default.changeCurrentDirectoryPath(Mock.Dir.remoteConfigChild) diff --git a/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ChildConfig/Test2/expected.yml b/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ChildConfig/Test2/expected.yml index 4e5c3fc58f..1f2108866c 100644 --- a/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ChildConfig/Test2/expected.yml +++ b/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ChildConfig/Test2/expected.yml @@ -2,7 +2,10 @@ opt_in_rules: - private_action disabled_rules: + - empty_count - force_cast + - private_outlet + - trailing_closure line_length: 80 diff --git a/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ParentConfig/Test2/expected.yml b/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ParentConfig/Test2/expected.yml index 4e5c3fc58f..1f2108866c 100644 --- a/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ParentConfig/Test2/expected.yml +++ b/Tests/SwiftLintFrameworkTests/Resources/ProjectMock/ParentConfig/Test2/expected.yml @@ -2,7 +2,10 @@ opt_in_rules: - private_action disabled_rules: + - empty_count - force_cast + - private_outlet + - trailing_closure line_length: 80