realm / SwiftLint

A tool to enforce Swift style and conventions.
https://realm.github.io/SwiftLint
MIT License
18.58k stars 2.21k forks source link

False-positive for `xct_specific_matcher ` rule with optional boolean #3575

Open fila95 opened 3 years ago

fila95 commented 3 years ago

New Issue Checklist

Describe the bug

The xct_specific_matcher rule should not trigger when one of its arguments is an optional boolean.

Complete output when running SwiftLint, including the stack trace and command used
$ Pods/SwiftLint/swiftlint lint TestTests/TestTests.swift
Linting Swift files at paths TestTests/TestTests.swift
Linting 'TestTests.swift' (1/1)
/Users/gif/Desktop/Test/TestTests/TestTests.swift:17:5: warning: XCTest Specific Matcher Violation: Prefer the specific matcher 'XCTAssertTrue' instead. (xct_specific_matcher)
Done linting! Found 1 violation, 0 serious in 1 file.

Environment

only_rules:
  # Prefer specific XCTest matchers over XCTAssertEqual and XCTAssertNotEqual
  - xct_specific_matcher
class TestTests: XCTestCase {
  var optionalBoolean: Bool? {
    return false
  }

  func testExample() throws {
    // This triggers a violation:
    XCTAssertEqual(self.optionalBoolean, true)
  }
}
rlpm commented 10 months ago

Note that SwiftLint suggests changing (e.g.) XCTAssert(self.optionalBoolean == true) to XCTAssertEqual(self.optionalBoolean, true):

warning: XCTest Specific Matcher Violation: Prefer the specific matcher 'XCTAssertEqual' instead (xct_specific_matcher)

And once you do, it suggests changing again to XCTAssertTrue(self.optionalBoolean):

warning: XCTest Specific Matcher Violation: Prefer the specific matcher 'XCTAssertTrue' instead (xct_specific_matcher)

But doing so results in a compiler error:

error: optional type 'Bool?' cannot be used as a boolean; test for '!= nil' instead

The compiler correctly will not coerce the Bool? expression into a Bool.


Note also that nil != true && nil != false:

let a:Bool? = nil
let b:Bool? = false
let c:Bool? = true
print("\(String(describing:a)) == true: \(a == true)")
print("\(String(describing:a)) == false: \(a == false)")
print("\(String(describing:b)) == true: \(b == true)")
print("\(String(describing:b)) == false: \(b == false)")
print("\(String(describing:c)) == true: \(c == true)")
print("\(String(describing:c)) == false: \(c == false)")

yields:

nil == true: false
nil == false: false
Optional(false) == true: false
Optional(false) == false: true
Optional(true) == true: true
Optional(true) == false: false

That is, the compiler automatically wraps the (e.g.) true in XCTAssert(self.optionalBoolean == true) in an optional, and then compares the optionals, which is arguably the behavior the author intended, to ensure the LHS of the comparison was both non-nil and true.

SimplyDanny commented 10 months ago

This is a known shortcoming of this rule that we need to accept. It's impossible to get that right on the syntax-level (where SwiftLint operates on). It would need to know the type of b in XCTAssertEqual(b, true) to be sure if a refactoring to XCTAssert(b) is allowed or not.

mildm8nnered commented 7 months ago

I just enabled xct_specific_matcher, and came across this. True that we cannot always determine if b is optional, but in cases like XCTAssertTrue(someVar?.x), shouldn't we be able to catch those cases?

SimplyDanny commented 7 months ago

I just enabled xct_specific_matcher, and came across this. True that we cannot always determine if b is optional, but in cases like XCTAssertTrue(someVar?.x), shouldn't we be able to catch those cases?

Yes, that's possible.

alwold commented 2 months ago

One way I've gotten around this is by making the value to compare explicitly optional, like this:

XCTAssertEqual(self.optionalBoolean, .some(true))

That seems to satisfy the rule. It's a little more noisy, but it's also a little more explicit, which is maybe good 🤷