SwiftLint icon indicating copy to clipboard operation
SwiftLint copied to clipboard

False-positive for `xct_specific_matcher ` rule with optional boolean

Open fila95 opened this issue 4 years ago • 5 comments

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

  • SwiftLint version (run swiftlint version to be sure)? 0.43.0
  • Installation method used (Homebrew, CocoaPods, building from source, etc)? CocoaPods
  • Paste your configuration file:
only_rules:
  # Prefer specific XCTest matchers over XCTAssertEqual and XCTAssertNotEqual
  - xct_specific_matcher
  • Are you using nested configurations? If so, paste their relative paths and respective contents.
  • Which Xcode version are you using (check xcodebuild -version)? 12.3
  • Do you have a sample that shows the issue? Run echo "[string here]" | swiftlint lint --no-cache --use-stdin --enable-all-rules to quickly test if your example is really demonstrating the issue. If your example is more complex, you can use swiftlint lint --path [file here] --no-cache --enable-all-rules.
class TestTests: XCTestCase {
  var optionalBoolean: Bool? {
    return false
  }

  func testExample() throws {
    // This triggers a violation:
    XCTAssertEqual(self.optionalBoolean, true)
  }
}

fila95 avatar Mar 16 '21 15:03 fila95

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.

rlpm avatar Nov 08 '23 17:11 rlpm

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.

SimplyDanny avatar Nov 11 '23 12:11 SimplyDanny

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?

mildm8nnered avatar Jan 28 '24 18:01 mildm8nnered

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.

SimplyDanny avatar Jan 28 '24 18:01 SimplyDanny