dnrops
/
SwiftLint
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Add new `blanket_disable_command` rule (#4731)

This commit is contained in:
Martin Redington 2023-03-07 20:43:53 +00:00 committed by GitHub
parent 084ad9dfd4
commit 0bd8a7aba6
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 366 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
.sourcery/GeneratedTests.stencil
View File

@ -3,6 +3,7 @@
import SwiftLintTestHelpers
import XCTest
// swiftlint:disable:next blanket_disable_command
// swiftlint:disable file_length single_test_class type_name
{% for rule in types.structs %}

5
CHANGELOG.md
View File

@ -93,6 +93,11 @@
* Catch more valid `no_magic_numbers` violations.
[JP Simard](https://github.com/jpsim)
* Add `blanket_disable_command` rule that checks whether
rules are re-enabled after being disabled.
[Martin Redington](https://github.com/mildm8nnered)
[#4731](https://github.com/realm/SwiftLint/pull/4731)
* Add `invalid_swiftlint_command` rule that validates
`// swiftlint:enable` and `disable` commands.
[Martin Redington](https://github.com/mildm8nnered)

1
Source/SwiftLintFramework/Extensions/Collection+Windows.swift
View File

@ -1,3 +1,4 @@
// swiftlint:disable:next blanket_disable_command
// swiftlint:disable all
// Copied from https://github.com/apple/swift-algorithms/blob/main/Sources/Algorithms/Windows.swift

3
Source/SwiftLintFramework/Extensions/Configuration+Parsing.swift
View File

@ -1,4 +1,5 @@
// swiftlint:disable inclusive_language - To ease a migration from the previous `whitelist_rules`
// swiftlint:disable:next blanket_disable_command
// swiftlint:disable inclusive_language - To ease migration from `whitelist_rules`
extension Configuration {
// MARK: - Subtypes

1
Source/SwiftLintFramework/Models/PrimaryRuleList.swift
View File

@ -10,6 +10,7 @@ let builtInRules: [Rule.Type] = [
ArrayInitRule.self,
AttributesRule.self,
BalancedXCTestLifecycleRule.self,
BlanketDisableCommandRule.self,
BlockBasedKVORule.self,
CaptureVariableRule.self,
ClassDelegateProtocolRule.self,

198
Source/SwiftLintFramework/Rules/Lint/BlanketDisableCommandRule.swift Normal file
View File

@ -0,0 +1,198 @@
import SwiftSyntax
struct BlanketDisableCommandRule: ConfigurationProviderRule {
var configuration = BlanketDisableCommandConfiguration()
init() {}
static let description = RuleDescription(
identifier: "blanket_disable_command",
name: "Blanket Disable Command",
description: "swiftlint:disable commands should be re-enabled before the end of the file",
kind: .lint,
nonTriggeringExamples: [
Example("""
// swiftlint:disable unused_import
// swiftlint:enable unused_import
"""),
Example("""
// swiftlint:disable unused_import unused_declaration
// swiftlint:enable unused_import
// swiftlint:enable unused_declaration
"""),
Example("// swiftlint:disable:this unused_import"),
Example("// swiftlint:disable:next unused_import"),
Example("// swiftlint:disable:previous unused_import")
],
triggeringExamples: [
Example("// swiftlint:disable ↓unused_import"),
Example("""
// swiftlint:disable unused_import ↓unused_declaration
// swiftlint:enable unused_import
"""),
Example("""
// swiftlint:disable unused_import
// swiftlint:disable ↓unused_import
// swiftlint:enable unused_import
"""),
Example("""
// swiftlint:enable ↓unused_import
""")
].skipWrappingInCommentTests().skipDisableCommandTests()
)
func validate(file: SwiftLintFile) -> [StyleViolation] {
var violations: [StyleViolation] = []
var ruleIdentifierToCommandMap: [RuleIdentifier: Command] = [:]
var disabledRuleIdentifiers: Set<RuleIdentifier> = []
for command in file.commands {
if command.action == .disable {
violations += validateAlreadyDisabledRules(
for: command,
in: file,
disabledRuleIdentifiers: disabledRuleIdentifiers
)
}
if command.action == .enable {
violations += validateAlreadyEnabledRules(
for: command,
in: file,
disabledRuleIdentifiers: disabledRuleIdentifiers
)
}
if command.modifier != nil {
continue
}
if command.action == .disable {
disabledRuleIdentifiers.formUnion(command.ruleIdentifiers)
command.ruleIdentifiers.forEach { ruleIdentifierToCommandMap[$0] = command }
}
if command.action == .enable {
disabledRuleIdentifiers.subtract(command.ruleIdentifiers)
command.ruleIdentifiers.forEach { ruleIdentifierToCommandMap.removeValue(forKey: $0) }
}
}
violations += validateBlanketDisables(
in: file,
disabledRuleIdentifiers: disabledRuleIdentifiers,
ruleIdentifierToCommandMap: ruleIdentifierToCommandMap
)
violations += validateAlwaysBlanketDisable(file: file)
return violations
}
private func violation(
for command: Command,
ruleIdentifier: RuleIdentifier,
in file: SwiftLintFile,
reason: String
) -> StyleViolation {
violation(for: command, ruleIdentifier: ruleIdentifier.stringRepresentation, in: file, reason: reason)
}
private func violation(
for command: Command,
ruleIdentifier: String,
in file: SwiftLintFile,
reason: String
) -> StyleViolation {
StyleViolation(
ruleDescription: Self.description,
severity: configuration.severity,
location: command.location(of: ruleIdentifier, in: file),
reason: reason
)
}
private func validateAlreadyDisabledRules(
for command: Command,
in file: SwiftLintFile,
disabledRuleIdentifiers: Set<RuleIdentifier>
) -> [StyleViolation] {
let alreadyDisabledRuleIdentifiers = command.ruleIdentifiers.intersection(disabledRuleIdentifiers)
return alreadyDisabledRuleIdentifiers.map {
let reason = "The disabled '\($0.stringRepresentation)' rule was already disabled"
return violation(for: command, ruleIdentifier: $0, in: file, reason: reason)
}
}
private func validateAlreadyEnabledRules(
for command: Command,
in file: SwiftLintFile,
disabledRuleIdentifiers: Set<RuleIdentifier>
) -> [StyleViolation] {
let notDisabledRuleIdentifiers = command.ruleIdentifiers.subtracting(disabledRuleIdentifiers)
return notDisabledRuleIdentifiers.map {
let reason = "The enabled '\($0.stringRepresentation)' rule was not disabled"
return violation(for: command, ruleIdentifier: $0, in: file, reason: reason)
}
}
private func validateBlanketDisables(
in file: SwiftLintFile,
disabledRuleIdentifiers: Set<RuleIdentifier>,
ruleIdentifierToCommandMap: [RuleIdentifier: Command]
) -> [StyleViolation] {
let allowedRuleIdentifiers = configuration.allowedRuleIdentifiers.union(
configuration.alwaysBlanketDisableRuleIdentifiers
)
return disabledRuleIdentifiers.compactMap { disabledRuleIdentifier in
if allowedRuleIdentifiers.contains(disabledRuleIdentifier.stringRepresentation) {
return nil
}
if let command = ruleIdentifierToCommandMap[disabledRuleIdentifier] {
let reason = "The disabled '\(disabledRuleIdentifier.stringRepresentation)' rule " +
"should be re-enabled before the end of the file"
return violation(for: command, ruleIdentifier: disabledRuleIdentifier, in: file, reason: reason)
}
return nil
}
}
private func validateAlwaysBlanketDisable(file: SwiftLintFile) -> [StyleViolation] {
var violations: [StyleViolation] = []
guard configuration.alwaysBlanketDisableRuleIdentifiers.isEmpty == false else {
return []
}
for command in file.commands {
let ruleIdentifiers: Set<String> = Set(command.ruleIdentifiers.map { $0.stringRepresentation })
let intersection = ruleIdentifiers.intersection(configuration.alwaysBlanketDisableRuleIdentifiers)
if command.action == .enable {
violations.append(contentsOf: intersection.map {
let reason = "The '\($0)' rule applies to the whole file and thus doesn't need to be re-enabled"
return violation(for: command, ruleIdentifier: $0, in: file, reason: reason)
})
} else if command.modifier != nil {
violations.append(contentsOf: intersection.map {
let reason = "The '\($0)' rule applies to the whole file and thus cannot be disabled locally " +
"with 'previous', 'this' or 'next'"
return violation(for: command, ruleIdentifier: $0, in: file, reason: reason)
})
}
}
return violations
}
}
private extension Command {
func location(of ruleIdentifier: String, in file: SwiftLintFile) -> Location {
var location = character
if line > 0, line <= file.lines.count {
let line = file.lines[line - 1].content
if let ruleIdentifierIndex = line.range(of: ruleIdentifier)?.lowerBound {
location = line.distance(from: line.startIndex, to: ruleIdentifierIndex) + 1
}
}
return Location(file: file.file.path, line: line, character: location)
}
}

39
Source/SwiftLintFramework/Rules/RuleConfigurations/BlanketDisableCommandConfiguration.swift Normal file
View File

@ -0,0 +1,39 @@
public struct BlanketDisableCommandConfiguration: SeverityBasedRuleConfiguration, Equatable {
public private(set) var severityConfiguration = SeverityConfiguration(.warning)
public private(set) var allowedRuleIdentifiers: Set<String> = [
"file_header",
"file_length",
"file_name",
"file_name_no_space",
"single_test_class"
]
public private(set) var alwaysBlanketDisableRuleIdentifiers: Set<String> = []
public var consoleDescription: String {
"severity: \(severityConfiguration.consoleDescription)" +
", allowed_rules: \(allowedRuleIdentifiers.sorted())" +
", always_blanket_disable: \(alwaysBlanketDisableRuleIdentifiers.sorted())"
}
public mutating func apply(configuration: Any) throws {
guard let configuration = configuration as? [String: Any] else {
throw ConfigurationError.unknownConfiguration
}
if let severityString = configuration["severity"] as? String {
try severityConfiguration.apply(configuration: severityString)
}
if let allowedRuleIdentifiers = configuration["allowed_rules"] as? [String] {
self.allowedRuleIdentifiers = Set(allowedRuleIdentifiers)
}
if let alwaysBlanketDisableRuleIdentifiers = configuration["always_blanket_disable"] as? [String] {
self.alwaysBlanketDisableRuleIdentifiers = Set(alwaysBlanketDisableRuleIdentifiers)
}
}
public var severity: ViolationSeverity {
severityConfiguration.severity
}
}

7
Tests/GeneratedTests/GeneratedTests.swift
View File

@ -5,6 +5,7 @@
import SwiftLintTestHelpers
import XCTest
// swiftlint:disable:next blanket_disable_command
// swiftlint:disable file_length single_test_class type_name
class AccessibilityLabelForImageRuleGeneratedTests: XCTestCase {
@ -49,6 +50,12 @@ class BalancedXCTestLifecycleRuleGeneratedTests: XCTestCase {
}
}
class BlanketDisableCommandRuleGeneratedTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(BlanketDisableCommandRule.description)
}
}
class BlockBasedKVORuleGeneratedTests: XCTestCase {
func testWithDefaultConfiguration() {
verifyRule(BlockBasedKVORule.description)

38
Tests/SwiftLintFrameworkTests/BlanketDisableCommandRuleTests.swift Normal file
View File

@ -0,0 +1,38 @@
@testable import SwiftLintFramework
import XCTest
class BlanketDisableCommandRuleTests: XCTestCase {
private var emptyDescription: RuleDescription {
BlanketDisableCommandRule.description.with(triggeringExamples: []).with(nonTriggeringExamples: [])
}
func testAlwaysBlanketDisable() {
let nonTriggeringExamples = [Example("// swiftlint:disable file_length\n// swiftlint:enable file_length")]
verifyRule(emptyDescription.with(nonTriggeringExamples: nonTriggeringExamples))
let triggeringExamples = [
Example("// swiftlint:disable file_length\n// swiftlint:enable ↓file_length"),
Example("// swiftlint:disable:previous ↓file_length"),
Example("// swiftlint:disable:this ↓file_length"),
Example("// swiftlint:disable:next ↓file_length")
]
verifyRule(emptyDescription.with(triggeringExamples: triggeringExamples),
ruleConfiguration: ["always_blanket_disable": ["file_length"]],
skipCommentTests: true, skipDisableCommandTests: true)
}
func testAlwaysBlanketDisabledAreAllowed() {
let nonTriggeringExamples = [Example("// swiftlint:disable identifier_name\n")]
verifyRule(emptyDescription.with(nonTriggeringExamples: nonTriggeringExamples),
ruleConfiguration: ["always_blanket_disable": ["identifier_name"], "allowed_rules": []],
skipDisableCommandTests: true)
}
func testAllowedRules() {
let nonTriggeringExamples = [
Example("// swiftlint:disable file_length"),
Example("// swiftlint:disable single_test_class")
]
verifyRule(emptyDescription.with(nonTriggeringExamples: nonTriggeringExamples))
}
}

82
Tests/SwiftLintFrameworkTests/CommandTests.swift
View File

@ -237,8 +237,8 @@ class CommandTests: XCTestCase {
func testSuperfluousDisableCommands() {
XCTAssertEqual(
violations(Example("// swiftlint:disable nesting\nprint(123)\n"))[0].ruleIdentifier,
"superfluous_disable_command"
violations(Example("// swiftlint:disable nesting\nprint(123)\n")).map { $0.ruleIdentifier },
["blanket_disable_command", "superfluous_disable_command"]
)
XCTAssertEqual(
violations(Example("// swiftlint:disable:next nesting\nprint(123)\n"))[0].ruleIdentifier,
@ -257,17 +257,33 @@ class CommandTests: XCTestCase {
func testDisableAllOverridesSuperfluousDisableCommand() {
XCTAssert(
violations(
Example("// swiftlint:disable all\n// swiftlint:disable nesting\nprint(123)\n")
Example("""
// swiftlint:disable all
// swiftlint:disable nesting
print(123)
// swiftlint:enable nesting
// swiftlint:enable all
""")
).isEmpty
)
XCTAssert(
violations(
Example("// swiftlint:disable all\n// swiftlint:disable:next nesting\nprint(123)\n")
Example("""
// swiftlint:disable all
// swiftlint:disable:next nesting
print(123)
// swiftlint:enable all
""")
).isEmpty
)
XCTAssert(
violations(
Example("// swiftlint:disable all\n// swiftlint:disable:this nesting\nprint(123)\n")
Example("""
// swiftlint:disable all
// swiftlint:disable:this nesting
print(123)
// swiftlint:enable all
""")
).isEmpty
)
XCTAssert(
@ -280,9 +296,10 @@ class CommandTests: XCTestCase {
func testSuperfluousDisableCommandsIgnoreDelimiter() {
let longComment = "Comment with a large number of words that shouldn't register as superfluous"
XCTAssertEqual(
violations(Example("// swiftlint:disable nesting - \(longComment)\nprint(123)\n"))[0]
.ruleIdentifier,
"superfluous_disable_command"
violations(Example("// swiftlint:disable nesting - \(longComment)\nprint(123)\n")).map {
$0.ruleIdentifier
},
["blanket_disable_command", "superfluous_disable_command"]
)
XCTAssertEqual(
violations(Example("// swiftlint:disable:next nesting - Comment\nprint(123)\n"))[0]
@ -303,8 +320,9 @@ class CommandTests: XCTestCase {
func testInvalidDisableCommands() {
XCTAssertEqual(
violations(Example("// swiftlint:disable nesting_foo\nprint(123)\n"))[0]
.ruleIdentifier,
violations(Example("// swiftlint:disable nesting_foo\n" +
"print(123)\n" +
"// swiftlint:enable nesting_foo\n"))[0].ruleIdentifier,
"superfluous_disable_command"
)
XCTAssertEqual(
@ -328,33 +346,39 @@ class CommandTests: XCTestCase {
1
)
let multipleViolations = violations(Example("// swiftlint:disable nesting this is a comment\n"))
XCTAssertEqual(multipleViolations.count, 5)
XCTAssertTrue(multipleViolations.allSatisfy { $0.ruleIdentifier == "superfluous_disable_command" })
let example = Example("// swiftlint:disable nesting this is a comment\n// swiftlint:enable nesting\n")
let multipleViolations = violations(example)
XCTAssertEqual(multipleViolations.filter({ $0.ruleIdentifier == "superfluous_disable_command" }).count, 9)
XCTAssertEqual(multipleViolations.filter({ $0.ruleIdentifier == "blanket_disable_command" }).count, 4)
let onlyNonExistentRulesViolations = violations(Example("// swiftlint:disable this is a comment\n"))
XCTAssertEqual(onlyNonExistentRulesViolations.count, 4)
XCTAssertTrue(onlyNonExistentRulesViolations.allSatisfy {
$0.ruleIdentifier == "superfluous_disable_command"
})
XCTAssertEqual(
onlyNonExistentRulesViolations.filter({ $0.ruleIdentifier == "superfluous_disable_command" }).count, 4
)
XCTAssertEqual(onlyNonExistentRulesViolations.filter({
$0.ruleIdentifier == "blanket_disable_command"
}).count, 4)
XCTAssertEqual(
violations(Example("print(123)\n// swiftlint:disable:previous nesting_foo\n"))[0].reason,
"'nesting_foo' is not a valid SwiftLint rule; remove it from the disable command"
)
XCTAssertEqual(violations(Example("/* swiftlint:disable nesting */\n")).count, 1)
XCTAssertEqual(violations(Example("/* swiftlint:disable nesting */\n")).count, 2)
}
func testSuperfluousDisableCommandsDisabled() {
XCTAssertEqual(
violations(Example("// swiftlint:disable superfluous_disable_command nesting\nprint(123)\n")),
violations(Example("// swiftlint:disable superfluous_disable_command nesting\n" +
"print(123)\n" +
"// swiftlint:enable superfluous_disable_command nesting\n")),
[]
)
XCTAssertEqual(
violations(Example("// swiftlint:disable superfluous_disable_command\n" +
"// swiftlint:disable nesting\n" +
"print(123)\n")),
"// swiftlint:disable nesting\n" +
"print(123)\n" +
"// swiftlint:enable superfluous_disable_command nesting\n")),
[]
)
XCTAssertEqual(
@ -376,7 +400,9 @@ class CommandTests: XCTestCase {
let configuration = Configuration(rulesMode: rulesMode)
XCTAssertEqual(
violations(Example("// swiftlint:disable nesting\nprint(123)\n"), config: configuration),
violations(Example("// swiftlint:disable nesting\n" +
"print(123)\n" +
"// swiftlint:enable nesting\n"), config: configuration),
[]
)
XCTAssertEqual(
@ -398,14 +424,20 @@ class CommandTests: XCTestCase {
violations(Example("""
// swiftlint:disable all
// swiftlint:disable non_existent_rule_name
// swiftlint:enable non_existent_rule_name
// swiftlint:enable all
"""
)),
[]
)
XCTAssertEqual(
violations(Example(
"// swiftlint:disable superfluous_disable_command\n" +
"// swiftlint:disable non_existent_rule_name\n"
violations(Example("""
// swiftlint:disable superfluous_disable_command
// swiftlint:disable non_existent_rule_name
// swiftlint:enable non_existent_rule_name
// swiftlint:enable superfluous_disable_command
"""
)),
[]
)

1
Tests/SwiftLintFrameworkTests/ConfigurationTests+Mock.swift
View File

@ -1,5 +1,6 @@
@testable import SwiftLintFramework
// swiftlint:disable:next blanket_disable_command
// swiftlint:disable nesting identifier_name
internal extension ConfigurationTests {

12
Tests/SwiftLintFrameworkTests/DisableAllTests.swift
View File

@ -26,7 +26,8 @@ class DisableAllTests: XCTestCase {
/// Tests whether swiftlint:disable all protects properly
func testDisableAll() {
for violatingPhrase in violatingPhrases {
let protectedPhrase = violatingPhrase.with(code: "// swiftlint:disable all\n" + violatingPhrase.code)
let code = "// swiftlint:disable all\n" + violatingPhrase.code + "\n// swiftlint:enable all\n"
let protectedPhrase = violatingPhrase.with(code: code)
XCTAssertEqual(
violations(protectedPhrase).count,
0,
@ -79,7 +80,8 @@ class DisableAllTests: XCTestCase {
// swiftlint:disable all
\(violatingPhrase.code)
\(violatingPhrase.code)
// swiftlint:enable:previous all\n
// swiftlint:enable:previous all
// swiftlint:enable all
""")
XCTAssertEqual(
violations(unprotectedPhrase).count,
@ -111,7 +113,8 @@ class DisableAllTests: XCTestCase {
// swiftlint:disable all
\(violatingPhrase.code)
// swiftlint:enable:next all
\(violatingPhrase.code)\n
\(violatingPhrase.code)
// swiftlint:enable all
""")
XCTAssertEqual(
violations(unprotectedPhrase).count,
@ -144,7 +147,8 @@ class DisableAllTests: XCTestCase {
let unprotectedPhrase = violatingPhrase.with(code: """
// swiftlint:disable all
\(violatingPhrase.code)
\(rawViolatingPhrase)// swiftlint:enable:this all\n"
\(rawViolatingPhrase)// swiftlint:enable:this all
// swiftlint:enable all
""")
XCTAssertEqual(
violations(unprotectedPhrase).count,

2
Tests/SwiftLintFrameworkTests/Exports.swift
View File

@ -1,3 +1,3 @@
// These imports are re-exported to make them available implicitly to all files.
// swiftlint:disable unused_import
// swiftlint:disable:next unused_import
@_exported import SwiftLintTestHelpers

5
Tests/SwiftLintFrameworkTests/LineEndingTests.swift
View File

@ -5,7 +5,10 @@ class LineEndingTests: XCTestCase {
func testCarriageReturnDoesNotCauseError() {
XCTAssert(
violations(
Example("// swiftlint:disable all\r\nprint(123)\r\n")
Example(
"// swiftlint:disable:next blanket_disable_command\r\n" +
"// swiftlint:disable all\r\nprint(123)\r\n"
)
).isEmpty
)
}

2
Tests/SwiftLintFrameworkTests/OpeningBraceRuleTests.swift
View File

@ -2,7 +2,7 @@
import XCTest
class OpeningBraceRuleTests: XCTestCase {
// swiftlint:disable function_body_length
// swiftlint:disable:next function_body_length
func testWithAllowMultilineTrue() {
// Test with `same_line` set to false

3
Tests/SwiftLintFrameworkTests/TypeContentsOrderRuleTests.swift
View File

@ -1,8 +1,8 @@
@testable import SwiftLintFramework
import XCTest
// swiftlint:disable function_body_length
class TypeContentsOrderRuleTests: XCTestCase {
// swiftlint:disable:next function_body_length
func testTypeContentsOrderReversedOrder() {
// Test with reversed `order` entries
let nonTriggeringExamples = [
@ -164,6 +164,7 @@ class TypeContentsOrderRuleTests: XCTestCase {
)
}
// swiftlint:disable:next function_body_length
func testTypeContentsOrderGroupedOrder() {
// Test with grouped `order` entries
let nonTriggeringExamples = [