False positiv PossiblyFalseOperand

[VincentLanglet]

https://psalm.dev/r/0072ed98c0

If I split the if, it works: https://psalm.dev/r/1ddfac6d56

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/0072ed98c0

```php getFoo(); if ($foo === false || !isset($array[$foo - 1])) { return null; } return $array[$foo - 1]; } } ``` ``` Psalm output (using commit 35b6a93): ERROR: PossiblyFalseOperand - 19:23 - Left operand cannot be falsable, got false|int ```

https://psalm.dev/r/1ddfac6d56

```php getFoo(); if ($foo === false) { return null; } if (!isset($array[$foo - 1])) { return null; } return $array[$foo - 1]; } } ``` ``` Psalm output (using commit 35b6a93): No issues! ```

[orklah]

I'll not add the "bug" label because there are cases where Psalm avoids calculating all possible cases when they're hard to reason about and I think this is one of them. However this precise case doesn't seem so hard at first glance so it might be worth looking if we can improve the situation

[VincentLanglet]

Any idea how to do this enhancement @orklah ?

[orklah]

https://github.com/vimeo/psalm/blob/6abce3525ac34f815aaa3b7953739b744ca5c39e/src/Psalm/Internal/Analyzer/Statements/Block/IfElseAnalyzer.php#L261

This is the comment I thought about when I answered. If you want to try, I'd recommend trying to understand what's going on here. If I'm correct, the goal would be to have something in $reconcilable_if_types

[VincentLanglet]

Code is

$reconcilable_if_types = Algebra::getTruthsFromFormula(
            $if_context->clauses,
            \spl_object_id($stmt->cond),
            $cond_referenced_var_ids,
            $active_if_types
        );

and then

if ($reconcilable_if_types) {
    ...

I'm not sure to understand what is a reconciliable_if_type, and how psalm will understand it won't occur after the if, since I have an early return...

Looking at getTruthsFromFormula, there is the following code

foreach ($clauses as $clause) {
            if (!$clause->reconcilable || count($clause->possibilities) !== 1) {
                continue;
            }
            ...

According to the phpdoc of a Clause,

/**
     * An array of strings of the form
     * [
     *     '$a' => ['falsy'],
     *     '$b' => ['!falsy'],
     *     '$c' => ['!null'],
     *     '$d' => ['string', 'int']
     * ]
     *
     * representing the formula
     *
     * !$a || $b || $c !== null || is_string($d) || is_int($d)
     *
     * @var array<string, non-empty-list<string>>
     */
    public $possibilities;

The code

$foo === false || !isset($array[$foo - 1])

is ['$foo' => ['!false'], something_else]. So the count is 2 and I think I end up in the continue.

I'm not sure how to change the code. The fact is in the if, I can't be sure if !isset($array[$foo - 1]) or if $foo === false. But after the early return, I do know that both check are false.

I do know how to modify stub or minor bugfix, but this is too much complicated for me...

[orklah]

Yep, I struggle a lot in there too.

Short and probably twisted explanation: Psalm read conditions and transform them into assertions. This is done in AssertionFinder for example. It looks like $a => string, $b => int, $a => numeric This is valid only for a scope (for example the inside of the if). They can be negated (to infer the type inside the else for example) and they can be reconciled ($a => string + $a => numeric = $a => numeric-string) getTruthsFromFormula and simplifyCNF is where the Psalm's magic happens. It's a lot of boolean operations to convert complex conditions into simple clauses. To quote Matt on that

it’s very unlikely there is a bug in that particular code

so I think the change must be before or after those.