Trigger superfluous_disable_command for custom_rules (#4755)

Fixes #4754

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

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

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))
     }

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
         """
     }

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"]