False positives testing for array emptiness about psalm HOT 13 OPEN

I found these snippets:

<?php

class Foo {
    /** @var string[]|null */
    protected ?array $arr;

    function __construct()
    {
        $this->arr = [];

        // ...do stuff here that *may* populate $this->arr, or set it to NULL...

        if (empty($this->arr)) {
            echo "empty\n";
        }
    }
}

Psalm output (using commit ef3b018):

ERROR: RedundantCondition - 13:13 - Operand of type true is always truthy

ERROR: RedundantCondition - 13:13 - Type array<never, never> for $this->arr is never non-empty

from psalm.

Could you provide a full example?
When I call something that modifies it as your comment says, it works fine: https://psalm.dev/r/d12fcff59a

from psalm.

I found these snippets:

<?php

class Foo {
    /** @var string[]|null */
    protected ?array $arr;

    function __construct()
    {
        $this->arr = [];

        // ...do stuff here that *may* populate $this->arr, or set it to NULL...
        $this->foo();

        if ($this->arr === [] || $this->arr === null) {
            echo "empty\n";
        }
    }
    
    function foo() : void {
        if (rand(0, 1) > 0) {
        	$this->arr = ['world'];    
        }
    }
}

Psalm output (using commit ef3b018):

No issues!

from psalm.

Could you provide a full example? When I call something that modifies it as your comment says, it works fine: https://psalm.dev/r/d12fcff59a

Hi @kkmuffme - I'm not sure I follow.

The snippet I posted is a minimal example that reproduces the issue. It is from a big app codebase that started to emit warnings after upgrading from 5.19.0 to latest Psalm. I don't see anything wrong with that code.

IMHO your changed code not issuing any warnings just confirm the problem, since your version do the same thing, i.e. $arr === [] || $a === null is almost the same as empty($arr), just more verbose.

from psalm.

I found these snippets:

<?php

class Foo {
    /** @var string[]|null */
    protected ?array $arr;

    function __construct()
    {
        $this->arr = [];

        // ...do stuff here that *may* populate $this->arr, or set it to NULL...
        $this->foo();

        if ($this->arr === [] || $this->arr === null) {
            echo "empty\n";
        }
    }
    
    function foo() : void {
        if (rand(0, 1) > 0) {
        	$this->arr = ['world'];    
        }
    }
}

Psalm output (using commit ef3b018):

No issues!

from psalm.

Here's another minimal example that reproduces the issue, extracted from the same app that started to emit warnings after upgrade:

https://psalm.dev/r/44845ca739

from psalm.

I found these snippets:

<?php

$arr = explode('.', phpversion()); 

if (empty($arr)) {
    echo "Empty\n";
}

Psalm output (using commit ef3b018):

ERROR: DocblockTypeContradiction - 5:5 - Operand of type false is always falsy

ERROR: DocblockTypeContradiction - 5:5 - Docblock-defined type non-empty-list<string> for $arr is never falsy

from psalm.

Maybe a somewhat more minimal case:

/**
 * @param true[] $array
 */
function check_key_in_array(array $array, int $key): void
{
    if (!empty($array[$key])) echo "key $key not empty";
}

gives: ERROR: [RedundantConditionGivenDocblockType](https://psalm.dev/156) - 8:9 - Operand of type true is always truthy

the element is ALWAYS truthy when present, but it can be missing -> empty -> falsy, since the keys are not specified.

https://psalm.dev/r/557faa3f96

from psalm.

I found these snippets:

<?php

/**
 * @param true[] $array
 */
function check_key_in_array(array $array, int $key): void
{
    if (!empty($array[$key])) echo "key $key not empty";
}

Psalm output (using commit ef3b018):

ERROR: RedundantConditionGivenDocblockType - 8:9 - Operand of type true is always truthy

from psalm.

This regression seems to be related to:

• #10578
• #10715
• #10716

It's still reproducible in 5.23.1

from psalm.

@flaviovs your first example is not a minimal example - the place where you put

// ...do stuff here that may populate $this->arr, or set it to NULL...

is the issue - since this code that you claim may modify it, is analyzed by psalm. This is why I provided an example where this code is actually there (and not just a comment) to show it works if there's actual code there, and not just a comment. But feel free to improve upon my example, since again - this is just one example to showcase it works with actual code that is not a comment.

$arr === [] || $a === null is almost the same as empty($arr)

Yeah, almost but not entirely. Anyway you can just use empty: https://psalm.dev/r/206b194230 (the error you get now is bc it's almost the same but not entirely as the verbose check - but this is unrelted to this question. I just wanted to clarify this to prevent further questions in this regard, since that's not the issue we agree.)

Here's another minimal example that reproduces the issue, extracted from the same app that started to emit warnings after upgrade:
https://psalm.dev/r/44845ca739

This error is correct too. explode never returns a falsy (empty) value.

@ArtemGoutsoul can you please open a separate issue, since this is an entirely different issue caused by the true array (afaik there's an issue for that alraedy, but I couldn't find it right now). As a temp workaround you can just use isset() there
EDIT: you already reported this here #10715 which is fixed - what's the purpose of tacking this on here?

from psalm.

I found these snippets:

<?php

class Foo {
    /** @var string[]|null */
    protected ?array $arr;

    function __construct()
    {
        $this->arr = [];

        // ...do stuff here that *may* populate $this->arr, or set it to NULL...
        $this->foo();

        if (empty($this->arr)) {
            echo "empty\n";
        }
    }
    
    function foo() : void {
        if (rand(0, 1) > 0) {
        	$this->arr = ['world'];    
        }
    }
}

Psalm output (using commit ef3b018):

ERROR: RiskyTruthyFalsyComparison - 14:13 - Operand of type array<array-key, string>|null contains type array<array-key, string>, which can be falsy and truthy. This can cause possibly unexpected behavior. Use strict comparison instead.

<?php

$arr = explode('.', phpversion()); 

if (empty($arr)) {
    echo "Empty\n";
}

Psalm output (using commit ef3b018):

ERROR: DocblockTypeContradiction - 5:5 - Operand of type false is always falsy

ERROR: DocblockTypeContradiction - 5:5 - Docblock-defined type non-empty-list<string> for $arr is never falsy

from psalm.

@kkmuffme you are right, maybe it's a separate issue, but should not be #10715, since this issue is still present in 5.23.1

If the fix for #10715 is in the upcoming release and not in 5.23.1, then my comments are a brainfart on my part and should be erased, I should just wait for the next release.

from psalm.