False-positive RedundantCondition

[gharlan]

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

<?php

/**
 * @param DateTime[] $bar
 */
function foo(?DateTime $foo, array $bar): void {
    if (
        $foo && !in_array($foo, $bar, true)
        || !$foo && $bar
    ) {
        return;
    }
    
    if ($foo) {
    }
}

[psalm-github-bot]

I found these snippets:

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

<?php

/**
 * @param DateTime[] $bar
 */
function foo(?DateTime $foo, array $bar): void {
    if (
        $foo && !in_array($foo, $bar, true)
        || !$foo && $bar
    ) {
        return;
    }
    
    if ($foo) {
    }
}

Psalm output (using commit f0b2142):

ERROR: RedundantCondition - 14:9 - Operand of type DateTime is always truthy

[AndrolGenhald]

It looks like the issue is with the negation of strict in_array, this and this should be identical, but Psalm seems to think that if $int is not in $ints then it must not be an int, which is incorrect. It's probably just comparing the types somewhere without accounting for the fact that they can have the same type but still be different.

Scratch that, not sure what I was thinking there. Here's something that's actually useful though, doing a strict comparison against null fixes it.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/39b01f458c

<?php

/** @param list<int> $ints */
function foobar(?int $int, array $ints): bool
{
    if ($int === null) {
        return true;
    }

    return in_array($int, $ints, true);
}

Psalm output (using commit f0b2142):

No issues!

https://psalm.dev/r/37e040f7fe

<?php

/** @param list<int> $ints */
function foobar(?int $int, array $ints): bool
{
    if (!in_array($int, $ints, true)) {
        return false;
    }

    return $int === null;
}

Psalm output (using commit f0b2142):

ERROR: TypeDoesNotContainNull - 10:12 - int does not contain null

[AndrolGenhald]

Well I git bisected it to #6233, but I'm not actually sure that's at fault, it seems more like a boolean algebra problem. I think this is the same problem without in_array (proof it's wrong since I keep messing it up). git bisecting that points to a832d77, which seems more relevant. Maybe @muglug can help with this?

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/3395485445

<?php

function foo(?int $foo, int $bar): void {
    if (
        $foo && $foo !== 1
        || !$foo && $bar
    ) {
        return;
    }
    
    if ($foo === null) {
    }
}

Psalm output (using commit f0b2142):

ERROR: TypeDoesNotContainNull - 11:9 - int does not contain null

[muglug]

The conjunctive normal form version (which Psalm uses internally) also shows the bug: https://psalm.dev/r/edc2283627

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/edc2283627

<?php

function baz(?int $foo): void {
    if (
        (!$foo || $foo !== 5) && ($foo || rand(0,1)) && ($foo !== 5 || rand(0, 1))
    ) {
        return;
    }
    
    if ($foo === null) {}
}

Psalm output (using commit f0b2142):

ERROR: TypeDoesNotContainNull - 10:9 - int does not contain null

[muglug]

Fixed in the master branch

[muglug]

The culrpit was bad logic here.

It was simplifying (A || B || !B) to (A), whereas it should have simplified it to true

[gharlan]

@muglug Thanks for the quick fix. But shouldn't the fix target the 4.x branch?

[orklah]

We're starting to have big differences between 4.x and master.

Fixing this in 4.x means basically fixing an old version then rebasing that on master and getting conflicts, so basically fixing it twice.

Recent events kinda disrupted availability and motivation of the team, I was hoping Psalm 5 would be available by now. In the meantime, it means some bugs are getting fixed on master unfortunately.

However, feel free to switch to our master branch and report us any bug that could appear!