Add new `invalid_swiftlint_command` rule (#4546)

CHANGELOG.md

@@ -89,6 +89,11 @@
 * Catch more valid `no_magic_numbers` violations.  
   [JP Simard](https://github.com/jpsim)
 
+* Add `invalid_swiftlint_command` rule that validates
+  `// swiftlint:enable` and `disable` commands.  
+  [Martin Redington](https://github.com/mildm8nnered)
+  [#4546](https://github.com/realm/SwiftLint/pull/4546)
+
 * Improve `identifier_name` documentation.  
   [Martin Redington](https://github.com/mildm8nnered)
   [#4767](https://github.com/realm/SwiftLint/issues/4767)

Source/SwiftLintFramework/Extensions/SwiftLintFile+Cache.swift

@@ -165,7 +165,8 @@ extension SwiftLintFile {
 
     internal var locationConverter: SourceLocationConverter { locationConverterCache.get(self) }
 
-    internal var commands: [Command] { commandsCache.get(self) }
+    internal var commands: [Command] { commandsCache.get(self).filter { $0.isValid } }
+    internal var invalidCommands: [Command] { commandsCache.get(self).filter { !$0.isValid } }
 
     internal var syntaxTokensByLines: [[SwiftLintSyntaxToken]] {
         guard let syntaxTokensByLines = syntaxTokensByLinesCache.get(self) else {

Source/SwiftLintFramework/Extensions/SwiftLintFile+Regex.swift

@@ -31,6 +31,9 @@ extension SwiftLintFile {
 
             case .enable:
                 disabledRules.subtract(command.ruleIdentifiers)
+
+            case .invalid:
+                break
             }
 
             let start = Location(file: path, line: command.line, character: command.character)

Source/SwiftLintFramework/Models/Command.swift

@@ -8,6 +8,8 @@ public struct Command: Equatable {
         case enable
         /// The rule(s) associated with this command should be disabled by the SwiftLint engine.
         case disable
+        /// The action string was invalid.
+        case invalid
 
         /// - returns: The inverse action that can cancel out the current action, restoring the SwifttLint engine's
         ///            state prior to the current action.
@@ -15,6 +17,7 @@ public struct Command: Equatable {
             switch self {
             case .enable: return .disable
             case .disable: return .enable
+            case .invalid: return .invalid
             }
         }
     }
@@ -27,6 +30,8 @@ public struct Command: Equatable {
         case this
         /// The command should only apply to the line following its definition.
         case next
+        /// The modifier string was invalid.
+        case invalid
     }
 
     /// Text after this delimiter is not considered part of the rule.
@@ -36,6 +41,10 @@ public struct Command: Equatable {
     ///     swiftlint:disable:next force_try - Explanation here
     private static let commentDelimiter = " - "
 
+    var isValid: Bool {
+        action != .invalid && modifier != .invalid && !ruleIdentifiers.isEmpty
+    }
+
     internal let action: Action
     internal let ruleIdentifiers: Set<RuleIdentifier>
     internal let line: Int
@@ -53,7 +62,7 @@ public struct Command: Equatable {
     ///                              defined.
     /// - parameter modifier:        This command's modifier, if any.
     /// - parameter trailingComment: The comment following this command's `-` delimiter, if any.
-    public init(action: Action, ruleIdentifiers: Set<RuleIdentifier>, line: Int = 0,
+    public init(action: Action, ruleIdentifiers: Set<RuleIdentifier> = [], line: Int = 0,
                 character: Int? = nil, modifier: Modifier? = nil, trailingComment: String? = nil) {
         self.action = action
         self.ruleIdentifiers = ruleIdentifiers
@@ -69,24 +78,24 @@ public struct Command: Equatable {
     /// - parameter line:         The line in the source file where this command is defined.
     /// - parameter character:    The character offset within the line in the source file where this command is
     ///                           defined.
-    public init?(actionString: String, line: Int, character: Int) {
+    public init(actionString: String, line: Int, character: Int) {
         let scanner = Scanner(string: actionString)
         _ = scanner.scanString("swiftlint:")
         // (enable|disable)(:previous|:this|:next)
         guard let actionAndModifierString = scanner.scanUpToString(" ") else {
-            return nil
+            self.init(action: .invalid, line: line, character: character)
+            return
         }
         let actionAndModifierScanner = Scanner(string: actionAndModifierString)
         guard let actionString = actionAndModifierScanner.scanUpToString(":"),
-            let action = Action(rawValue: actionString)
+              let action = Action(rawValue: actionString)
-            else {
+        else {
-                return nil
+            self.init(action: .invalid, line: line, character: character)
+            return
         }
-        self.action = action
-        self.line = line
-        self.character = character
 
         let rawRuleTexts = scanner.scanUpToString(Self.commentDelimiter) ?? ""
+        var trailingComment: String?
         if scanner.isAtEnd {
             trailingComment = nil
         } else {
@@ -103,19 +112,26 @@ public struct Command: Equatable {
             return component.isNotEmpty && component != "*/"
         }
 
-        ruleIdentifiers = Set(ruleTexts.map(RuleIdentifier.init(_:)))
+        let ruleIdentifiers = Set(ruleTexts.map(RuleIdentifier.init(_:)))
 
         // Modifier
         let hasModifier = actionAndModifierScanner.scanString(":") != nil
+        let modifier: Modifier?
         if hasModifier {
-            modifier = Modifier(
+            let modifierString = String(actionAndModifierScanner.string[actionAndModifierScanner.currentIndex...])
-              rawValue: String(
+            modifier = Modifier(rawValue: modifierString) ?? .invalid
-                actionAndModifierScanner.string[actionAndModifierScanner.currentIndex...]
-              )
-            )
         } else {
             modifier = nil
         }
+
+        self.init(
+            action: action,
+            ruleIdentifiers: ruleIdentifiers,
+            line: line,
+            character: character,
+            modifier: modifier,
+            trailingComment: trailingComment
+        )
     }
 
     /// Expands the current command into its fully descriptive form without any modifiers.
@@ -142,6 +158,8 @@ public struct Command: Equatable {
                 Self(action: action, ruleIdentifiers: ruleIdentifiers, line: line + 1),
                 Self(action: action.inverse(), ruleIdentifiers: ruleIdentifiers, line: line + 1, character: Int.max)
             ]
+        case .invalid:
+            return []
         }
     }
 }

Source/SwiftLintFramework/Models/PrimaryRuleList.swift

@@ -89,6 +89,7 @@ let builtInRules: [Rule.Type] = [
     InclusiveLanguageRule.self,
     IndentationWidthRule.self,
     InertDeferRule.self,
+    InvalidSwiftLintCommandRule.self,
     IsDisjointRule.self,
     JoinedDefaultParameterRule.self,
     LargeTupleRule.self,

Source/SwiftLintFramework/Rules/Lint/InvalidSwiftLintCommandRule.swift

@@ -0,0 +1,49 @@
+import Foundation
+import SwiftSyntax
+
+struct InvalidSwiftLintCommandRule: ConfigurationProviderRule {
+    var configuration = SeverityConfiguration(.warning)
+
+    init() {}
+
+    static let description = RuleDescription(
+        identifier: "invalid_swiftlint_command",
+        name: "Invalid SwiftLint Command",
+        description: "swiftlint command does not have a valid action or modifier",
+        kind: .lint,
+        nonTriggeringExamples: [
+            Example("// swiftlint:disable unused_import"),
+            Example("// swiftlint:enable unused_import"),
+            Example("// swiftlint:disable:next unused_import"),
+            Example("// swiftlint:disable:previous unused_import"),
+            Example("// swiftlint:disable:this unused_import")
+        ],
+        triggeringExamples: [
+            Example("// swiftlint:"),
+            Example("// swiftlint: "),
+            Example("// swiftlint::"),
+            Example("// swiftlint:: "),
+            Example("// swiftlint:disable"),
+            Example("// swiftlint:dissable unused_import"),
+            Example("// swiftlint:enaaaable unused_import"),
+            Example("// swiftlint:disable:nxt unused_import"),
+            Example("// swiftlint:enable:prevus unused_import"),
+            Example("// swiftlint:enable:ths unused_import"),
+            Example("// swiftlint:enable"),
+            Example("// swiftlint:enable:"),
+            Example("// swiftlint:enable: "),
+            Example("// swiftlint:disable: unused_import")
+        ].skipWrappingInCommentTests()
+    )
+
+    func validate(file: SwiftLintFile) -> [StyleViolation] {
+        file.invalidCommands.map {
+            let location = Location(file: file.path, line: $0.line, character: $0.character)
+            return StyleViolation(
+                ruleDescription: Self.description,
+                severity: configuration.severity,
+                location: location
+            )
+        }
+    }
+}

Source/SwiftLintFramework/Visitors/CommandVisitor.swift

@@ -36,9 +36,9 @@ private extension Trivia {
                     case let actionString = String(comment[lower...]),
                     case let end = locationConverter.location(for: offset + triviaOffset),
                     let line = end.line,
-                    let column = end.column,
+                    let column = end.column
-                    let command = Command(actionString: actionString, line: line, character: column)
                 {
+                    let command = Command(actionString: actionString, line: line, character: column)
                     results.append(command)
                 }
             default:

Tests/GeneratedTests/GeneratedTests.swift

@@ -517,6 +517,12 @@ class InertDeferRuleGeneratedTests: XCTestCase {
     }
 }
 
+class InvalidSwiftLintCommandRuleGeneratedTests: XCTestCase {
+    func testWithDefaultConfiguration() {
+        verifyRule(InvalidSwiftLintCommandRule.description)
+    }
+}
+
 class IsDisjointRuleGeneratedTests: XCTestCase {
     func testWithDefaultConfiguration() {
         verifyRule(IsDisjointRule.description)