diff --git a/CHANGELOG.md b/CHANGELOG.md index 0538a2301..97ac46ee7 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/Source/SwiftLintFramework/Extensions/SwiftLintFile+Cache.swift b/Source/SwiftLintFramework/Extensions/SwiftLintFile+Cache.swift index 1d6642f91..4c7ef4818 100644 --- a/Source/SwiftLintFramework/Extensions/SwiftLintFile+Cache.swift +++ b/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 { diff --git a/Source/SwiftLintFramework/Extensions/SwiftLintFile+Regex.swift b/Source/SwiftLintFramework/Extensions/SwiftLintFile+Regex.swift index 5d3d247ae..b77440a67 100644 --- a/Source/SwiftLintFramework/Extensions/SwiftLintFile+Regex.swift +++ b/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) diff --git a/Source/SwiftLintFramework/Models/Command.swift b/Source/SwiftLintFramework/Models/Command.swift index 247cb208d..ed24b22e7 100644 --- a/Source/SwiftLintFramework/Models/Command.swift +++ b/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 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, line: Int = 0, + public init(action: Action, ruleIdentifiers: Set = [], 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) - else { - return nil + let action = Action(rawValue: actionString) + else { + 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( - rawValue: String( - actionAndModifierScanner.string[actionAndModifierScanner.currentIndex...] - ) - ) + let modifierString = String(actionAndModifierScanner.string[actionAndModifierScanner.currentIndex...]) + modifier = Modifier(rawValue: modifierString) ?? .invalid } 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 [] } } } diff --git a/Source/SwiftLintFramework/Models/PrimaryRuleList.swift b/Source/SwiftLintFramework/Models/PrimaryRuleList.swift index a466c9fe0..5e9280c3d 100644 --- a/Source/SwiftLintFramework/Models/PrimaryRuleList.swift +++ b/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, diff --git a/Source/SwiftLintFramework/Rules/Lint/InvalidSwiftLintCommandRule.swift b/Source/SwiftLintFramework/Rules/Lint/InvalidSwiftLintCommandRule.swift new file mode 100644 index 000000000..ed4dc8593 --- /dev/null +++ b/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 + ) + } + } +} diff --git a/Source/SwiftLintFramework/Visitors/CommandVisitor.swift b/Source/SwiftLintFramework/Visitors/CommandVisitor.swift index 79fa57157..0650ad4bf 100644 --- a/Source/SwiftLintFramework/Visitors/CommandVisitor.swift +++ b/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 command = Command(actionString: actionString, line: line, character: column) + let column = end.column { + let command = Command(actionString: actionString, line: line, character: column) results.append(command) } default: diff --git a/Tests/GeneratedTests/GeneratedTests.swift b/Tests/GeneratedTests/GeneratedTests.swift index 1d1e402c2..9fad03fdb 100644 --- a/Tests/GeneratedTests/GeneratedTests.swift +++ b/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)