Problematic `== true/false` boolean comparisons

Evolution Discussion
,
1

Intro

This isn't an evolution proposal; it's my research on what feels like a weird snaggy bit around bool comparisons in Swift involving three-value logic. The snag makes it too easy to write fragile or incorrect code.

I'm interested in ways to make this fragile code harder to write, or at least have it signposted better when you're doing it (warnings etc).

This has been discussed before, see forum links at end.

The issue

Swift has extensions/code for == that allow comparison of Optional<Bool> with Bool, essentially sneaking in a kind of three-value logic. Not sure of the historical reasons for this, but it allows various mistakes and fragile code to be created without any compiler warning.

Example: in order to check if an optional string has data -- in which we need it to be non-nil and .isEmpty == false -- we would idiomatically write:

let str: String? = ...
if let str, !str.isEmpty {

However, I've seen this being done and even promoted as 'nicer' code:

if str?.isEmpty == false {

This works as per desired behaviour, but it's not good code; it's fragile. For example, someone might come along later and want to code the opposite check ("it's nil or .isEmpty == true"), and quite understandably might just flip the boolean:

if str?.isEmpty == true {

This is not actually the opposite check -- it's buggy, it will regard nil as containing data (because neither true nor false are equal to nil).

So IMO there's too much cognitive load to reason about/change this code, and you're expected to know false != nil to fully understand it.
(Note also that if someone flipped the code by changing the check to != false instead of == true, it would be a correct change! Which is very counter-intuitive.)

Here's another example of broken code I've seen in the wild: it's supposed to be guarding against an array having true at the end:

let bools: [Bool] = []
guard bools.last == false else {
    print("Error: last bool in array is true")
}

So .last returns an optional, and nil != false, so if array is empty we enter the guard.
(And there's no compiler warning for the code above.)

Bool comparisons considered harmful?

Thought:

Is there ever an actual need to write == true or == false in Swift? It's not idiomatic, and crucially the ability to do so allows the sorts of fragility detailed above to happen.

Things that might help

Sometimes people like explicit bool checks for clarity (even though it's not idiomatic) -- in those cases, extension on Bool could provide .isTrue and .isFalse (and no handling implemented for Optional<Bool>).

SwiftLint has a rule to warn you off creating option Bool properties (discouraged_optional_boolean Reference). This is only heading off one possible cause of the issue though; optional chaining and funcs that return optional bool (e.g. [Bool].last) can still introduce opportunity for the problem without any compiler warning AFAICS.

We can write a specialisation on == that generates a warning, at least:

@available(*, deprecated, message: "Comparing Optional<Boolean> using == is not safe practice. Unwrap the value and use direct expression evaluation e.g. 'if boolExpression' or 'if !boolExpression'")
func == (lhs: Bool?, rhs: Bool) -> Bool {
    guard let unwrappedLHS = lhs else { return false }
    return unwrappedLHS == rhs
}
// (not shown: other two variants for `Bool, Bool?` and `Bool?, Bool?`)

and this code does generate compiler warnings for me for these two problem cases:

let strings: [String] = []
if strings.last?.isEmpty == false { }

let str: String? = nil
if str?.isEmpty == false { }

Links

There have been a few related proposal for SwiftLint rules that never panned out:

Related forum posts:

8 Likes
2

FWIW, I do consider this an anti-pattern in Swift, and in all languages that don’t use a fuzzy notion of “truthiness” for conditionals. Even in languages like JS, Python, and Ruby that accept non-boolean values as if conditions, it’s definitely a code smell.

== true / == false is a habit we specifically attempt to train out of students at Macalester. One little data point, but it’s something.

None of that is really an answer to the language safety question, but it does suggest we’re at least somewhat more in the realm of “the language doesn’t stop you from writing bad code” than in the realm of “the language exhibits unreasonable behavior for reasonable code.”

6 Likes
3

Thanks Paul. I agree on it being an anti pattern, I always call out == true/false in reviews.

And as you note, the logic here of Swift is indeed internally consistent; it’s more that it “gives you enough rope” so to speak.

If I saw some of the code samples above in a project (e.g. the .last example) I’d be thinking “does the author know what the behaviour is and they intended this? Or is this a booby trap in waiting?”

Maybe it would be useful for compiler to emit a warning when you’re explicitly comparing booleans and optional Booleans, which my == specialisation gives a sketch of doing.

1 Like
4

There are situations where this is reasonable when you have something other than a boolean literal on both sides of the ==. For example, here’s some code to count how many contiguous runs of identical values there are in an array of booleans:

func countGroups(values: [Bool]) -> Int {
    var groupCount = 0
    var prevValue: Bool? = nil

    for curVal in values {
        if curVal != prevValue {  // Comparing Bool to Bool?
            groupCount += 1       // Always count first one, no matter the value!
        }
        prevValue = curVal
    }

    return groupCount
}

(Yes, there are other ways to code this; yes, it could be generic; yes, it’s a silly little toy problem. But there are more realistic contexts where that same “compare to optional previous” pattern might show up and be desirable.)

I could see emitting a warning when one side of an == is a boolean literal, not just a boolean expression — but that feels to me more like a job for SwiftLint than for the compiler.

4 Likes
5

It would not make sense* to require .some for Bool but not all other Equatable types.

let bool: _? = Bool.random()
#expect(bool == .some(true))

let int: _? = Int.random(in: 0...2)
#expect(int == 1)

* Edit: Maybe that's not true. Seems worthy of discussion; it's not immediately obvious to me exactly what makes Bool a problem, but it does seem somewhat special in this regard. I think it's something like, "Optional promotion can make people think Bool?'s == only needs to consider two values, when the real number is three".

6

I think compiler might produce a warning in such code along with quick fix using if-let to address this.

For instance, string interpolation has warning for optionals, encouraging dealing with it there. The case presented here is probably even more error-prone and better be addressed via compiler.

As for explicit boolean comparison, for false there are justification on improved readability against exclamation mark, so discouraging this way of writing doesn’t seem appealing to me.

2 Likes
7

I've also seen != true to permit both false and nil; what would be the expected workaround for that?

2 Likes
8

I think the answer is “don’t do that”—use if foo == nil || !foo { instead.

4 Likes
9

Alternatively, if !(foo ?? false)

2 Likes
10

To add another terrible option:

if [nil, false].contains(foo) {

If you don't like the behavior, just be explicit instead of letting the language perform optional promotion.

if foo != .some(true) {
1 Like
11

IMO, != true is fine. The point of the code is to make sense to the reader. The compiler doesn't care either way. Sometimes it may make sense to do (optBool ?? false) == false. Sometimes it might feel better to do optBool != true.

Go with what most clearly expresses the intent of the code, and if it's still not clear enough to you, add comments explaining what's going on and why.

15 Likes
12

I see your thinking, now. But == does not operate on Bool?, specifically.

Because of that, your suggestions are possible.

let foo: Optional = false
_ = foo == true // Warning: '==' is deprecated
_ = true == foo // Warning: '==' is deprecated
_ = foo != false // Warning: '==' is deprecated
_ = false != foo // Warning: '==' is deprecated
_ = foo == .some(true) // No warning.
_ = foo != .some(false) // No warning.

Does anybody actually care about the second-form (Yoda) overloads? They're not in the examples here. And they break @Paul_Cantrell's suggestion.

public extension Bool {
  @available(*, deprecated) static func == (_: Self?, _: Self) -> Self { .init() }
  @available(*, deprecated) static func == (_: Self, _: Self?) -> Self { .init() }
  @available(*, deprecated) static func != (_: Self?, _: Self) -> Self { .init() }
  @available(*, deprecated) static func != (_: Self, _: Self?) -> Self { .init() }
}
13

This isn’t valid because foo is still of type Bool? on the RHS of the ||.

Fair, I’ve seen this before.

14

Yes, my mistake. You can write this as if foo == nil || !(foo!) {, but if !(foo ?? false) { is easier to understand. You might also be able to do guard let foo, foo else { return } depending on context.

I believe this situation motivated someone to ask for an unless keyword (edit: or ifn't if you’re @Joe_Groff), but ultimately how often does one actually test for an optional Boolean being nil or false without also handling the case where it’s true?

1 Like
15

Sorry, that looks cute but I have found it really hard to parse mentally.

Ditto.

Short writing makes long reading. :slight_smile:

if let foo {
   // was set
   ...
}
else {
   // fuzzy
   // was not even set
   ...
}
1 Like
16 
let someOptionalBool: Bool? = ...

switch someOptionalBool {
  case nil, false:
    // value is nil or false
  case true:
    // value is true
}

When there are three possible values, why not switch over all possible values?

17

That is not exhaustive. You need to unwrap.

To use this only for an if statement looks quite terrible, I think.

if ({
  switch someOptionalBool {
  case nil, false?: true
  case true?: false
  }
} ()) {
  // `someOptionalBool` is not true, here.
}

You can also be explicit like this, but only for one of the three possibilities. There is no syntax for handling two, if one of them is nil:

if case true? = someOptionalBool { }
if case .some(true) = someOptionalBool { }

// Works without the `?`, of course, too, due to `Optional` promotion.
if case true = someOptionalBool { }

:face_with_diagonal_mouth::

if ({
  if case true? = someOptionalBool { false } else { true }
} ()) { }
18

I’m surprised case nil, false?, true? is considered exhaustive, but case nil, false, true is not. That feels like it should be a bug.

There’s no need to use the switch statement for an if statement’s condition — switch is already a branching construct.

4 Likes
19

I think your initial expression is pretty idiomatic.

For complex ones I usually resort to using map/flatMap on Optionals:

if strings.last.map({ !$0.isEmpty }) ?? false {
1 Like
20

It is. But sure, you can be explicit and use true? or .some(true) instead of just true. However, they all match the same case.

Agreed, but why would you do that?! You'd certainly want to place the code that would go in the if-branch, inside the switch, non?