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

Extend `xct_specific_matcher` rule to check for boolean asserts on (un)equal comparisons (#3858)

This commit is contained in:
Danny Mösch 2023-04-02 12:30:21 +02:00 committed by GitHub
parent b0cbb440c3
commit 16e2bb0f18
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 223 additions and 80 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
CHANGELOG.md
View File

@ -13,6 +13,11 @@
* Add `sorted_enum_cases` rule which warns when enum cases are not sorted.
[kimdv](https://github.com/kimdv)
* Extend `xct_specific_matcher` rule to check for boolean asserts on (un)equal
comparisons.
[SimplyDanny](https://github.com/SimplyDanny)
[#3726](https://github.com/realm/SwiftLint/issues/3726)
#### Bug Fixes
* None.

146
Source/SwiftLintFramework/Rules/Idiomatic/XCTSpecificMatcherRule.swift
View File

@ -1,3 +1,4 @@
import SwiftOperators
import SwiftSyntax
struct XCTSpecificMatcherRule: SwiftSyntaxRule, OptInRule, ConfigurationProviderRule {
@ -21,68 +22,60 @@ struct XCTSpecificMatcherRule: SwiftSyntaxRule, OptInRule, ConfigurationProvider
private extension XCTSpecificMatcherRule {
final class Visitor: ViolationsSyntaxVisitor {
private static let protectedArguments: Set<String> = [
"false", "true", "nil"
]
override func visitPost(_ node: FunctionCallExprSyntax) {
guard let name = node.calledExpression.as(IdentifierExprSyntax.self)?.identifier.text,
let matcher = XCTestMatcher(rawValue: name) else {
return
if let suggestion = TwoArgsXCTAssert.violations(in: node) ?? OneArgXCTAssert.violations(in: node) {
violations.append(ReasonedRuleViolation(
position: node.positionAfterSkippingLeadingTrivia,
reason: "Prefer the specific matcher '\(suggestion)' instead"
))
}
/*
* - Gets the first two arguments and creates an array where the protected
* word is the first one (if any).
*
* Examples:
*
* - XCTAssertEqual(foo, true) -> [true, foo]
* - XCTAssertEqual(true, foo) -> [true, foo]
* - XCTAssertEqual(foo, true, "toto") -> [true, foo]
* - XCTAssertEqual(1, 2, accuracy: 0.1, "toto") -> [1, 2]
*/
let arguments = node.argumentList
.prefix(2)
.map { $0.expression.trimmedDescription }
.sorted { arg1, _ -> Bool in
return Self.protectedArguments.contains(arg1)
}
/*
* - Checks if the number of arguments is two (otherwise there's no need to continue).
* - Checks if the first argument is a protected word (otherwise there's no need to continue).
* - Gets the suggestion for the given protected word (taking in consideration the presence of
* optionals.
*
* Examples:
*
* - equal, [true, foo.bar] -> XCTAssertTrue
* - equal, [true, foo?.bar] -> no violation
* - equal, [nil, foo.bar] -> XCTAssertNil
* - equal, [nil, foo?.bar] -> XCTAssertNil
* - equal, [1, 2] -> no violation
*/
guard arguments.count == 2,
let argument = arguments.first, Self.protectedArguments.contains(argument),
let hasOptional = arguments.last?.contains("?"),
let suggestedMatcher = matcher.suggestion(for: argument, hasOptional: hasOptional) else {
return
}
violations.append(ReasonedRuleViolation(
position: node.positionAfterSkippingLeadingTrivia,
reason: "Prefer the specific matcher '\(suggestedMatcher)' instead"
))
}
}
}
private enum XCTestMatcher: String {
private enum OneArgXCTAssert: String {
case assert = "XCTAssert"
case `true` = "XCTAssertTrue"
case `false` = "XCTAssertFalse"
private enum Comparison: String {
case equal = "=="
case unequal = "!="
}
private func suggestion(for comparisonOperator: Comparison) -> String {
switch (self, comparisonOperator) {
case (.assert, .equal): return "XCTAssertEqual"
case (.true, .equal): return "XCTAssertEqual"
case (.assert, .unequal): return "XCTAssertNotEqual"
case (.true, .unequal): return "XCTAssertNotEqual"
case (.false, .equal): return "XCTAssertNotEqual"
case (.false, .unequal): return "XCTAssertEqual"
}
}
static func violations(in node: FunctionCallExprSyntax) -> String? {
guard let name = node.calledExpression.as(IdentifierExprSyntax.self)?.identifier.text,
let matcher = Self(rawValue: name),
let argument = node.argumentList.first?.expression.as(SequenceExprSyntax.self),
let folded = try? OperatorTable.standardOperators.foldSingle(argument),
let binOp = folded.as(InfixOperatorExprSyntax.self)?.operatorOperand.as(BinaryOperatorExprSyntax.self),
let kind = Comparison(rawValue: binOp.operatorToken.text) else {
return nil
}
return matcher.suggestion(for: kind)
}
}
private enum TwoArgsXCTAssert: String {
case equal = "XCTAssertEqual"
case notEqual = "XCTAssertNotEqual"
func suggestion(for protectedArgument: String, hasOptional: Bool) -> String? {
private static let protectedArguments: Set<String> = [
"false", "true", "nil"
]
private func suggestion(for protectedArgument: String, hasOptional: Bool) -> String? {
switch (self, protectedArgument, hasOptional) {
case (.equal, "true", false): return "XCTAssertTrue"
case (.equal, "false", false): return "XCTAssertFalse"
@ -93,4 +86,51 @@ private enum XCTestMatcher: String {
default: return nil
}
}
static func violations(in node: FunctionCallExprSyntax) -> String? {
guard let name = node.calledExpression.as(IdentifierExprSyntax.self)?.identifier.text,
let matcher = Self(rawValue: name) else {
return nil
}
//
// - Gets the first two arguments and creates an array where the protected
// word is the first one (if any).
//
// Examples:
//
// - XCTAssertEqual(foo, true) -> [true, foo]
// - XCTAssertEqual(true, foo) -> [true, foo]
// - XCTAssertEqual(foo, true, "toto") -> [true, foo]
// - XCTAssertEqual(1, 2, accuracy: 0.1, "toto") -> [1, 2]
//
let arguments = node.argumentList
.prefix(2)
.map { $0.expression.trimmedDescription }
.sorted { arg1, _ -> Bool in
return protectedArguments.contains(arg1)
}
//
// - Checks if the number of arguments is two (otherwise there's no need to continue).
// - Checks if the first argument is a protected word (otherwise there's no need to continue).
// - Gets the suggestion for the given protected word (taking in consideration the presence of
// optionals.
//
// Examples:
//
// - equal, [true, foo.bar] -> XCTAssertTrue
// - equal, [true, foo?.bar] -> no violation
// - equal, [nil, foo.bar] -> XCTAssertNil
// - equal, [nil, foo?.bar] -> XCTAssertNil
// - equal, [1, 2] -> no violation
//
guard arguments.count == 2,
let argument = arguments.first, protectedArguments.contains(argument),
let hasOptional = arguments.last?.contains("?"),
let suggestedMatcher = matcher.suggestion(for: argument, hasOptional: hasOptional) else {
return nil
}
return suggestedMatcher
}
}

23
Source/SwiftLintFramework/Rules/Idiomatic/XCTSpecificMatcherRuleExamples.swift
View File

@ -1,6 +1,7 @@
internal struct XCTSpecificMatcherRuleExamples {
static let nonTriggeringExamples = [
// True/False
Example("XCTAssert(foo"),
Example("XCTAssertFalse(foo)"),
Example("XCTAssertTrue(foo)"),
@ -29,6 +30,7 @@ internal struct XCTSpecificMatcherRuleExamples {
Example("XCTAssertEqual(true, foo?.bar)"),
// Blank spaces
Example("XCTAssert( foo )"),
Example("XCTAssertFalse( foo )"),
Example("XCTAssertTrue( foo )"),
Example("XCTAssertNil( foo )"),
@ -102,6 +104,25 @@ internal struct XCTSpecificMatcherRuleExamples {
Example("↓XCTAssertEqual(false, nil)"),
Example("↓XCTAssertEqual(nil, nil)"),
Example("↓XCTAssertEqual(true, true)"),
Example("↓XCTAssertEqual(false, false)")
Example("↓XCTAssertEqual(false, false)"),
// Equality with `==`
Example("↓XCTAssert(foo == bar)"),
Example("↓XCTAssertTrue( foo == bar )"),
Example("↓XCTAssertFalse(1 == foo)"),
Example("↓XCTAssert(foo == bar, \"toto\")"),
// Inequality with `!=`
Example("↓XCTAssert(foo != bar)"),
Example("↓XCTAssertTrue( foo != bar )"),
Example("↓XCTAssertFalse(1 != foo)"),
Example("↓XCTAssert(foo != bar, \"toto\")"),
// Comparison with `nil`
Example("↓XCTAssert( foo == nil)"),
Example("↓XCTAssert(nil == foo"),
Example("↓XCTAssertTrue( foo != nil)"),
Example("↓XCTAssertFalse(nil != foo"),
Example("↓XCTAssert(foo == nil, \"toto\")")
]
}

1
Tests/SwiftLintFrameworkTests/ConfigurationAliasesTests.swift
View File

@ -8,6 +8,7 @@ final class ConfigurationAliasesTests: XCTestCase {
let ruleConfiguration = [1, 2]
let config = ["mock": ruleConfiguration]
let rules = try testRuleList.allRulesWrapped(configurationDict: config).map { $0.rule }
// swiftlint:disable:next xct_specific_matcher
XCTAssertTrue(rules == [try RuleWithLevelsMock(configuration: ruleConfiguration)])
}

14
Tests/SwiftLintFrameworkTests/ConfigurationTests.swift
View File

@ -26,10 +26,14 @@ class ConfigurationTests: XCTestCase {
// MARK: Tests
func testInit() {
XCTAssert((try? Configuration(dict: [:])) != nil,
"initializing Configuration with empty Dictionary should succeed")
XCTAssert((try? Configuration(dict: ["a": 1, "b": 2])) != nil,
"initializing Configuration with valid Dictionary should succeed")
XCTAssertNotNil(
try? Configuration(dict: [:]),
"initializing Configuration with empty Dictionary should succeed"
)
XCTAssertNotNil(
try? Configuration(dict: ["a": 1, "b": 2]),
"initializing Configuration with valid Dictionary should succeed"
)
}
func testNoConfiguration() {
@ -371,12 +375,14 @@ class ConfigurationTests: XCTestCase {
let ruleConfiguration = [1, 2]
let config = [RuleWithLevelsMock.description.identifier: ruleConfiguration]
let rules = try testRuleList.allRulesWrapped(configurationDict: config).map { $0.rule }
// swiftlint:disable:next xct_specific_matcher
XCTAssertTrue(rules == [try RuleWithLevelsMock(configuration: ruleConfiguration)])
}
func testConfigureFallsBackCorrectly() throws {
let config = [RuleWithLevelsMock.description.identifier: ["a", "b"]]
let rules = try testRuleList.allRulesWrapped(configurationDict: config).map { $0.rule }
// swiftlint:disable:next xct_specific_matcher
XCTAssertTrue(rules == [RuleWithLevelsMock()])
}

8
Tests/SwiftLintFrameworkTests/CyclomaticComplexityConfigurationTests.swift
View File

@ -73,9 +73,9 @@ class CyclomaticComplexityConfigurationTests: XCTestCase {
let config3 = CyclomaticComplexityConfiguration(warning: 10, error: 30, ignoresCaseStatements: false)
let config4 = CyclomaticComplexityConfiguration(warning: 10, error: 40)
let config5 = CyclomaticComplexityConfiguration(warning: 20, error: 30)
XCTAssertFalse(config1 == config2)
XCTAssertTrue(config1 == config3)
XCTAssertFalse(config1 == config4)
XCTAssertFalse(config1 == config5)
XCTAssertNotEqual(config1, config2)
XCTAssertEqual(config1, config3)
XCTAssertNotEqual(config1, config4)
XCTAssertNotEqual(config1, config5)
}
}

4
Tests/SwiftLintFrameworkTests/ImplicitReturnConfigurationTests.swift
View File

@ -19,8 +19,8 @@ class ImplicitReturnConfigurationTests: XCTestCase {
.function,
.getter
])
XCTAssert(configuration.severityConfiguration.severity == .error)
XCTAssertTrue(configuration.includedKinds == expectedKinds)
XCTAssertEqual(configuration.severityConfiguration.severity, .error)
XCTAssertEqual(configuration.includedKinds, expectedKinds)
}
func testImplicitReturnConfigurationThrowsOnUnrecognizedModifierGroup() {

10
Tests/SwiftLintFrameworkTests/LineLengthConfigurationTests.swift
View File

@ -177,21 +177,21 @@ class LineLengthConfigurationTests: XCTestCase {
error: 100,
options: [.ignoreFunctionDeclarations,
.ignoreComments])
XCTAssertFalse(configuration1 == configuration2)
XCTAssertNotEqual(configuration1, configuration2)
let configuration3 = LineLengthConfiguration(warning: 100, error: 200)
XCTAssertFalse(configuration1 == configuration3)
XCTAssertNotEqual(configuration1, configuration3)
let configuration4 = LineLengthConfiguration(warning: 200, error: 100)
XCTAssertFalse(configuration1 == configuration4)
XCTAssertNotEqual(configuration1, configuration4)
let configuration5 = LineLengthConfiguration(warning: 100, error: 100)
XCTAssertTrue(configuration1 == configuration5)
XCTAssertEqual(configuration1, configuration5)
let configuration6 = LineLengthConfiguration(warning: 100,
error: 100,
options: [.ignoreFunctionDeclarations,
.ignoreComments])
XCTAssertTrue(configuration2 == configuration6)
XCTAssertEqual(configuration2, configuration6)
}
}

22
Tests/SwiftLintFrameworkTests/RuleConfigurationTests.swift
View File

@ -242,21 +242,21 @@ class RuleConfigurationTests: XCTestCase {
ignoresComments: true)
let configuration2 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
ignoresComments: true)
XCTAssertFalse(configuration1 == configuration2)
XCTAssertNotEqual(configuration1, configuration2)
let configuration3 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
ignoresComments: true)
XCTAssertTrue(configuration2 == configuration3)
XCTAssertEqual(configuration2, configuration3)
let configuration4 = TrailingWhitespaceConfiguration(ignoresEmptyLines: false,
ignoresComments: false)
XCTAssertFalse(configuration1 == configuration4)
XCTAssertNotEqual(configuration1, configuration4)
let configuration5 = TrailingWhitespaceConfiguration(ignoresEmptyLines: true,
ignoresComments: false)
XCTAssertFalse(configuration1 == configuration5)
XCTAssertNotEqual(configuration1, configuration5)
}
func testTrailingWhitespaceConfigurationApplyConfigurationUpdatesSeverityConfiguration() {
@ -266,7 +266,7 @@ class RuleConfigurationTests: XCTestCase {
do {
try configuration.apply(configuration: ["severity": "error"])
XCTAssert(configuration.severityConfiguration.severity == .error)
XCTAssertEqual(configuration.severityConfiguration.severity, .error)
} catch {
XCTFail("Failed to apply severity")
}
@ -279,7 +279,7 @@ class RuleConfigurationTests: XCTestCase {
let conf1 = ["severity": "error", "excluded": "viewWillAppear(_:)"]
do {
try configuration.apply(configuration: conf1)
XCTAssert(configuration.severityConfiguration.severity == .error)
XCTAssertEqual(configuration.severityConfiguration.severity, .error)
XCTAssertFalse(configuration.resolvedMethodNames.contains("*"))
XCTAssertFalse(configuration.resolvedMethodNames.contains("viewWillAppear(_:)"))
XCTAssertTrue(configuration.resolvedMethodNames.contains("viewWillDisappear(_:)"))
@ -294,7 +294,7 @@ class RuleConfigurationTests: XCTestCase {
] as [String: Any]
do {
try configuration.apply(configuration: conf2)
XCTAssert(configuration.severityConfiguration.severity == .error)
XCTAssertEqual(configuration.severityConfiguration.severity, .error)
XCTAssertFalse(configuration.resolvedMethodNames.contains("*"))
XCTAssertFalse(configuration.resolvedMethodNames.contains("viewWillAppear(_:)"))
XCTAssertTrue(configuration.resolvedMethodNames.contains("viewWillDisappear(_:)"))
@ -311,8 +311,8 @@ class RuleConfigurationTests: XCTestCase {
] as [String: Any]
do {
try configuration.apply(configuration: conf3)
XCTAssert(configuration.severityConfiguration.severity == .warning)
XCTAssert(configuration.resolvedMethodNames.count == 2)
XCTAssertEqual(configuration.severityConfiguration.severity, .warning)
XCTAssertEqual(configuration.resolvedMethodNames.count, 2)
XCTAssertFalse(configuration.resolvedMethodNames.contains("*"))
XCTAssertTrue(configuration.resolvedMethodNames.contains("testMethod1()"))
XCTAssertTrue(configuration.resolvedMethodNames.contains("testMethod2(_:)"))
@ -354,8 +354,8 @@ class RuleConfigurationTests: XCTestCase {
.lazy,
.dynamic
]
XCTAssert(configuration.severityConfiguration.severity == .warning)
XCTAssertTrue(configuration.preferredModifierOrder == expected)
XCTAssertEqual(configuration.severityConfiguration.severity, .warning)
XCTAssertEqual(configuration.preferredModifierOrder, expected)
}
func testModifierOrderConfigurationThrowsOnUnrecognizedModifierGroup() {

1
Tests/SwiftLintFrameworkTests/RuleTests.swift
View File

@ -71,6 +71,7 @@ class RuleTests: XCTestCase {
}
func testRuleArraysWithDifferentCountsNotEqual() {
// swiftlint:disable:next xct_specific_matcher
XCTAssertFalse([RuleMock1(), RuleMock2()] == [RuleMock1()])
}

69
Tests/SwiftLintFrameworkTests/XCTSpecificMatcherRuleTests.swift
View File

@ -115,8 +115,77 @@ class XCTSpecificMatcherRuleTests: XCTestCase {
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertTrue' instead")
}
func testAssertEqual() {
let example = Example("XCTAssert(foo == bar)")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertEqual' instead")
}
func testAssertFalseNotEqual() {
let example = Example("XCTAssertFalse(bar != foo)")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertEqual' instead")
}
func testAssertTrueEqual() {
let example = Example("XCTAssertTrue(foo == 1)")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertEqual' instead")
}
func testAssertNotEqual() {
let example = Example("XCTAssert(foo != bar)")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertNotEqual' instead")
}
func testAssertFalseEqual() {
let example = Example("XCTAssertFalse(bar == foo)")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertNotEqual' instead")
}
func testAssertTrueNotEqual() {
let example = Example("XCTAssertTrue(foo != 1)")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertNotEqual' instead")
}
func testMultipleComparisons() {
let example = Example("XCTAssert(foo == (bar == baz))")
let violations = self.violations(example)
XCTAssertEqual(violations.count, 1)
XCTAssertEqual(violations.first?.reason, "Prefer the specific matcher 'XCTAssertEqual' instead")
}
func testEqualInCommentNotConsidered() {
XCTAssert(noViolation(in: "XCTAssert(foo, \"a == b\")"))
}
func testEqualInFunctionCall() {
XCTAssert(noViolation(in: "XCTAssert(foo(bar == baz))"))
XCTAssert(noViolation(in: "XCTAssertTrue(foo(bar == baz), \"toto\")"))
}
private func violations(_ example: Example) -> [StyleViolation] {
guard let config = makeConfig(nil, XCTSpecificMatcherRule.description.identifier) else { return [] }
return SwiftLintFrameworkTests.violations(example, config: config)
}
private func noViolation(in example: String) -> Bool {
violations(Example(example)).isEmpty
}
}