False positives testing for array emptiness

[flaviovs]

https://psalm.dev/r/4609d92f6f

Note: using empty() because it's a common pattern in PHP apps to assert nullable arrays. However, removing empty() (i.e. doing if ($this->arr)...) still generates warnings, albeit different ones:

ERROR: [TypeDoesNotContainType](https://psalm.dev/056) - 13:13 - Operand of type array<never, never> is always falsy
ERROR: [TypeDoesNotContainType](https://psalm.dev/056) - 13:13 - Type array<never, never> for $this->arr is always !falsy

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/4609d92f6f

<?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

[kkmuffme]

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

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/d12fcff59a

<?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!

[flaviovs]

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.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/d12fcff59a

<?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!

[flaviovs]

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

[psalm-github-bot[bot]]

I found these snippets:

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

<?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

[ArtemGoutsoul]

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

[psalm-github-bot[bot]]

I found these snippets:

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

<?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

[ArtemGoutsoul]

This regression seems to be related to:

• https://github.com/vimeo/psalm/issues/10578
• https://github.com/vimeo/psalm/issues/10715
• https://github.com/vimeo/psalm/issues/10716

It's still reproducible in 5.23.1

[kkmuffme]

@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 https://github.com/vimeo/psalm/issues/10715 which is fixed - what's the purpose of tacking this on here?

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/206b194230

<?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.

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

<?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

[ArtemGoutsoul]

@kkmuffme you are right, maybe it's a separate issue, but should not be https://github.com/vimeo/psalm/issues/10715, since this issue is still present in 5.23.1

If the fix for https://github.com/vimeo/psalm/issues/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.

[M1ke]

My team are debating this assertion

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

Does anyone have a value of $arr (where arr meets type definition array|null) where the empty case returns a different bool to the || case?

Tried a set here: https://3v4l.org/VlLK6#v8.3.8

[kkmuffme]

Can you please create an example on psalm.dev bc I have no idea what psalm error you're referring too, since the OP issue was a non-issue, then it got hijacked by someone with a completely different issue and I have no idea what you're referring to :-)

[ArtemGoutsoul]

My team are debating this assertion

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

Does anyone have a value of $arr (where arr meets type definition array|null) where the empty case returns a different bool to the || case?

Tried a set here: https://3v4l.org/VlLK6#v8.3.8

For a defined $arr that is array|null the expression $arr === [] || $arr === null is identical to empty($arr), since empty essentially casts to bool (and returns false for undefined vars):

• https://www.php.net/manual/en/function.empty.php
• https://www.php.net/manual/en/language.types.boolean.php#language.types.boolean.casting

[M1ke]

OK great; I was wondering about the "almost the same" comment because I really couldn't see why they were not identical. Someone pointed out that empty is safe for checking undefined values, but that's a different condition.

[flaviovs]

OP here. I've done some more tests an here's what I came up:

Leaving null check out, same results: https://psalm.dev/r/ba93e8414a

Using count($arr) === 0 or $arr === [], same results:

• https://psalm.dev/r/a93bf069be
• https://psalm.dev/r/ee7c8a7f07

So the issue may not be even related to empty() after all.

[psalm-github-bot[bot]]

I found these snippets:

https://psalm.dev/r/ba93e8414a

<?php
class Foo
{
	public array $data = [];

    public function resetData(): void
    {
        $this->data = [];

        $this->changeData();
        if (empty($this->data)) {
            echo "Is empty\n";
        }
    }
    
    public function changeData(): void
    {
        if (rand(0, 10) > 5) {
        	$this->data[] = 1;
    	}
    }
}

Psalm output (using commit 16b24bd):

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

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

https://psalm.dev/r/a93bf069be

<?php
class Foo
{
	public array $data = [];

    public function resetData(): void
    {
        $this->data = [];

        $this->changeData();
        if (count($this->data) === 0) {
            echo "Is empty\n";
        }
    }
    
    public function changeData(): void
    {
        if (rand(0, 10) > 5) {
        	$this->data[] = 1;
    	}
    }
}

Psalm output (using commit 16b24bd):

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

https://psalm.dev/r/ee7c8a7f07

<?php
class Foo
{
	public array $data = [];

    public function resetData(): void
    {
        $this->data = [];

        $this->changeData();
        if ($this->data === []) {
            echo "Is empty\n";
        }
    }
    
    public function changeData(): void
    {
        if (rand(0, 10) > 5) {
        	$this->data[] = 1;
    	}
    }
}

Psalm output (using commit 16b24bd):

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