From 7499896199a57625453e43075905ed62d6ac1237 Mon Sep 17 00:00:00 2001 From: Marcelo Fabri Date: Tue, 7 Nov 2023 01:11:58 -0800 Subject: [PATCH] Trigger superfluous_disable_command for custom_rules (#4755) Fixes #4754 --- CHANGELOG.md | 6 ++ Source/SwiftLintCore/Models/Linter.swift | 72 ++++++++++++------- Source/SwiftLintCore/Rules/CustomRules.swift | 2 +- .../Rules/SuperfluousDisableCommandRule.swift | 4 +- .../CustomRulesTests.swift | 60 ++++++++++++++++ 5 files changed, 116 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ac61fe0e4b..6b2695a2bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,12 @@ [jszumski](https://github.com/jszumski) [#5246](https://github.com/realm/SwiftLint/pull/5246) +* `superfluous_disable_command` violations are now triggered for + custom rules. + [Marcelo Fabri](https://github.com/marcelofabri) + [Martin Redington](https://github.com/mildm8nnered) + [#4754](https://github.com/realm/SwiftLint/issues/4754) + ## 0.53.0: Laundry List #### Breaking diff --git a/Source/SwiftLintCore/Models/Linter.swift b/Source/SwiftLintCore/Models/Linter.swift index 155e4f528f..0d1c318c45 100644 --- a/Source/SwiftLintCore/Models/Linter.swift +++ b/Source/SwiftLintCore/Models/Linter.swift @@ -16,42 +16,57 @@ private struct LintResult { } private extension Rule { - static func superfluousDisableCommandViolations(regions: [Region], - superfluousDisableCommandRule: SuperfluousDisableCommandRule?, - allViolations: [StyleViolation]) -> [StyleViolation] { + func superfluousDisableCommandViolations(regions: [Region], + superfluousDisableCommandRule: SuperfluousDisableCommandRule?, + allViolations: [StyleViolation]) -> [StyleViolation] { guard regions.isNotEmpty, let superfluousDisableCommandRule else { return [] } - let regionsDisablingCurrentRule = regions.filter { region in - return region.isRuleDisabled(self.init()) - } let regionsDisablingSuperfluousDisableRule = regions.filter { region in return region.isRuleDisabled(superfluousDisableCommandRule) } - return regionsDisablingCurrentRule.compactMap { region -> StyleViolation? in - let isSuperfluousRuleDisabled = regionsDisablingSuperfluousDisableRule.contains { - $0.contains(region.start) + let regionsWithIdentifiers: [(String, [Region])] = { + if let customRules = self as? CustomRules { + return customRules.configuration.customRuleConfigurations.map { configuration in + let regionsDisablingCurrentRule = regions.filter { region in + return region.isRuleDisabled(customRuleIdentifier: configuration.identifier) + } + return (configuration.identifier, regionsDisablingCurrentRule) + } + } else { + let regionsDisablingCurrentRule = regions.filter { region in + return region.isRuleDisabled(self) + } + return [(Self.description.identifier, regionsDisablingCurrentRule)] } + }() - guard !isSuperfluousRuleDisabled else { - return nil - } + return regionsWithIdentifiers.flatMap { ruleIdentifier, regionsDisablingCurrentRule in + regionsDisablingCurrentRule.compactMap { region -> StyleViolation? in + let isSuperfluousRuleDisabled = regionsDisablingSuperfluousDisableRule.contains { + $0.contains(region.start) + } - let noViolationsInDisabledRegion = !allViolations.contains { violation in - return region.contains(violation.location) - } - guard noViolationsInDisabledRegion else { - return nil - } + guard !isSuperfluousRuleDisabled else { + return nil + } - return StyleViolation( - ruleDescription: type(of: superfluousDisableCommandRule).description, - severity: superfluousDisableCommandRule.configuration.severity, - location: region.start, - reason: superfluousDisableCommandRule.reason(for: self) - ) + let noViolationsInDisabledRegion = !allViolations.contains { violation in + return region.contains(violation.location) && violation.ruleIdentifier == ruleIdentifier + } + guard noViolationsInDisabledRegion else { + return nil + } + + return StyleViolation( + ruleDescription: type(of: superfluousDisableCommandRule).description, + severity: superfluousDisableCommandRule.configuration.severity, + location: region.start, + reason: superfluousDisableCommandRule.reason(forRuleIdentifier: ruleIdentifier) + ) + } } } @@ -91,12 +106,19 @@ private extension Rule { return region?.isRuleEnabled(self) ?? true } + let customRulesIDs: [String] = { + guard let customRules = self as? CustomRules else { + return [] + } + return customRules.configuration.customRuleConfigurations.map(\.identifier) + }() let ruleIDs = Self.description.allIdentifiers + + customRulesIDs + (superfluousDisableCommandRule.map({ type(of: $0) })?.description.allIdentifiers ?? []) + [RuleIdentifier.all.stringRepresentation] let ruleIdentifiers = Set(ruleIDs.map { RuleIdentifier($0) }) - let superfluousDisableCommandViolations = Self.superfluousDisableCommandViolations( + let superfluousDisableCommandViolations = superfluousDisableCommandViolations( regions: regions.count > 1 ? file.regions(restrictingRuleIdentifiers: ruleIdentifiers) : regions, superfluousDisableCommandRule: superfluousDisableCommandRule, allViolations: violations diff --git a/Source/SwiftLintCore/Rules/CustomRules.swift b/Source/SwiftLintCore/Rules/CustomRules.swift index d7831979d8..21f440ced5 100644 --- a/Source/SwiftLintCore/Rules/CustomRules.swift +++ b/Source/SwiftLintCore/Rules/CustomRules.swift @@ -90,7 +90,7 @@ struct CustomRules: Rule, CacheDescriptionProvider { } } -private extension Region { +extension Region { func isRuleDisabled(customRuleIdentifier: String) -> Bool { return disabledRuleIdentifiers.contains(RuleIdentifier(customRuleIdentifier)) } diff --git a/Source/SwiftLintCore/Rules/SuperfluousDisableCommandRule.swift b/Source/SwiftLintCore/Rules/SuperfluousDisableCommandRule.swift index 94c9d5ee87..8169997225 100644 --- a/Source/SwiftLintCore/Rules/SuperfluousDisableCommandRule.swift +++ b/Source/SwiftLintCore/Rules/SuperfluousDisableCommandRule.swift @@ -35,9 +35,9 @@ public struct SuperfluousDisableCommandRule: SourceKitFreeRule { return [] } - func reason(for rule: (some Rule).Type) -> String { + func reason(forRuleIdentifier ruleIdentifier: String) -> String { """ - SwiftLint rule '\(rule.description.identifier)' did not trigger a violation in the disabled region; \ + SwiftLint rule '\(ruleIdentifier)' did not trigger a violation in the disabled region; \ remove the disable command """ } diff --git a/Tests/SwiftLintFrameworkTests/CustomRulesTests.swift b/Tests/SwiftLintFrameworkTests/CustomRulesTests.swift index 94ddf5e49e..cfaa7479a6 100644 --- a/Tests/SwiftLintFrameworkTests/CustomRulesTests.swift +++ b/Tests/SwiftLintFrameworkTests/CustomRulesTests.swift @@ -179,6 +179,66 @@ class CustomRulesTests: SwiftLintTestCase { XCTAssertEqual(violations[0].location.character, 6) } + func testSuperfluousDisableCommandWithCustomRules() throws { + let customRulesConfiguration: [String: Any] = [ + "custom1": [ + "regex": "pattern", + "match_kinds": "comment" + ] + ] + + let example = Example( + "// swiftlint:disable custom1\n", + configuration: customRulesConfiguration + ).skipWrappingInCommentTest() + let configuration = try XCTUnwrap(makeConfig(["custom_rules": customRulesConfiguration], "custom_rules")) + let violations = violations(example, config: configuration) + + XCTAssertEqual(violations.count, 1) + XCTAssertTrue(violations.allSatisfy { $0.ruleIdentifier == "superfluous_disable_command" }) + XCTAssertTrue(violations.contains { violation in + violation.description.contains("SwiftLint rule 'custom1' did not trigger a violation") + }) + } + + func testSuperfluousDisableCommandWithMultipleCustomRules() throws { + let customRulesConfiguration: [String: Any] = [ + "custom1": [ + "regex": "pattern", + "match_kinds": "comment" + ], + "custom2": [ + "regex": "10", + "match_kinds": "number" + ], + "custom3": [ + "regex": "100", + "match_kinds": "number" + ] + ] + + let example = Example( + """ + // swiftlint:disable custom1 custom3 + return 10 + + """, + configuration: customRulesConfiguration + ).skipWrappingInCommentTest() + let configuration = try XCTUnwrap(makeConfig(["custom_rules": customRulesConfiguration], "custom_rules")) + let violations = violations(example, config: configuration) + + XCTAssertEqual(violations.count, 3) + XCTAssertEqual(violations.filter { $0.ruleIdentifier == "superfluous_disable_command" }.count, 2) + XCTAssertEqual(violations.filter { $0.ruleIdentifier == "custom2" }.count, 1) + XCTAssertTrue(violations.contains { violation in + violation.description.contains("SwiftLint rule 'custom1' did not trigger a violation") + }) + XCTAssertTrue(violations.contains { violation in + violation.description.contains("SwiftLint rule 'custom3' did not trigger a violation") + }) + } + private func getCustomRules(_ extraConfig: [String: Any] = [:]) -> (Configuration, CustomRules) { var config: [String: Any] = ["regex": "pattern", "match_kinds": "comment"]