lint: best-practices/no-boolean-literal-comparison shouldn't always be thrown for === false

[giorgiopogliani]

🐞 Describe the Bug

Some php functions returns a union type like json_encode that has return type string|false, so it's should be valid behaviour comparing it to false with the strict equal.

Of course many other functions have this behaviour this is just an example.

🔄 Steps to Reproduce

1. create a test.php with the example
2. run mago lint test.php

⚙️ Configuration (mago.toml)

none

📜 Command Output

warning[best-practices/no-boolean-literal-comparison]: Avoid comparing the boolean literal `false`.
  ┌─ test.php:7:29
  │
7 │ if (json_encode('test') === false) {
  │     ------------------------^^^^^
  │     │                       │
  │     │                       Boolean literal `false` is used here.
  │     Revise the logic to avoid using a boolean literal.
  │
  = Comparisons with a boolean literal can be error-prone.
  = Help: Review and adjust the logic to remove the boolean literal comparison.

warning: found 1 issues: 1 warning(s)

📂 PHP Code Sample (If Applicable)

<?php

if (json_encode('test') === false) {
    echo 'Error';
}

🖥️ Operating System

Linux

📦 How did you install Mago?

Homebrew (brew install mago)

📝 Additional Context

No response

[innocenzi]

I'm also confused by this rule. I have several reports. I'm wondering if the rule was supposed to check for literal boolean strings (eg. 'false' and not false)?

[$isValid, $message] = match (true) {
    is_string($result) => [false, $result],
    $result === false => [false, 'Value did not pass validation.'],
    default => [true, ''],
};

warning[best-practices/no-boolean-literal-comparison]: Avoid comparing the boolean literal `false`.
   ┌─ src/Tempest/Validation/src/Validator.php:69:25
   │
69 │             $result === false => [false, 'Value did not pass validation.'],
   │             ------------^^^^^
   │             │           │
   │             │           Boolean literal `false` is used here.
   │             Revise the logic to avoid using a boolean literal.
   │
   = Comparisons with a boolean literal can be error-prone.
   = Help: Review and adjust the logic to remove the boolean literal comparison.

---

if ($result === false) {
    return [];
}

warning[best-practices/no-boolean-literal-comparison]: Avoid comparing the boolean literal `false`.
   ┌─ src/Tempest/Support/src/Regex/functions.php:28:29
   │
28 │             if ($result === false || $result === 0) {
   │                 ------------^^^^^
   │                 │           │
   │                 │           Boolean literal `false` is used here.
   │                 Revise the logic to avoid using a boolean literal.
   │
   = Comparisons with a boolean literal can be error-prone.
   = Help: Review and adjust the logic to remove the boolean literal comparison.

[azjezz]

This rule is meant to force you to write !$bar instead of $bar === false, i understand that some PHP builtin functions return T|false where this won't be possible, and i think in those cases the best course of action is to either disable the rule, or use @mago-expect to avoid the report.

[giorgiopogliani]

@azjezz Maybe we can change it to be opt-in? I would basically disable it in every project if the behaviour can't change

[innocenzi]

I agree that this should be better opt-in given its current behavior

[azjezz]

the rule is now disabled by default.