Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Psalm does not derive correct element type of array shape member #9801

Open
mstilkerich opened this issue May 20, 2023 · 11 comments
Open

Psalm does not derive correct element type of array shape member #9801

mstilkerich opened this issue May 20, 2023 · 11 comments

Comments

@mstilkerich
Copy link
Contributor

Hello,

after upgrading to psalm 5.11, I am getting a spurious error that did not occur with a previous 5.x version of psalm (unfortunately I do not know the exact version I used before, I think I upgraded last around January 2023).

I tried to condense the code snippet to something minimal:
https://psalm.dev/r/64389d2bd0

Inside the switch, psalm knows the value of $name an carries it as a string constant type. Therefore it should be able to know which member of the array shape is accessed when using $name as index, but apparently it does not. When I use the string literal as index instead (see second variant of the function), it works.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/64389d2bd0
<?php

/**
 * @psalm-type PropTypes = array{
 *   'lkey'?: list<array{'content-type': string, version: string}>,
 *   'skey'?: string,
 * }
 */
class Test {
    /**
     * @psalm-var PropTypes
     */
    public $props = [];

    public function storeProperty(string $name): void
    {
        switch ($name) {
            case 'lkey':
                if (!isset($this->props[$name])) {
                    $this->props[$name] = [];
                }
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }
    
    public function storePropertyWorks(string $name): void
    {
        switch ($name) {
            case 'lkey':
                if (!isset($this->props[$name])) {
                    $this->props[$name] = [];
                }
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props['lkey'][] = $addrData;
                break;
        }
    }
}
Psalm output (using commit 0221ff1):

ERROR: InvalidPropertyAssignmentValue - 23:17 - $this->props with declared type 'array{lkey?: list<array{'content-type': string, version: string}>, skey?: string}' cannot be assigned type 'array{lkey: list<array{'content-type': string, version: string}>|string, skey?: string}'

@robchett
Copy link
Contributor

Tracked the regression down to 2327917, a fix for #9433

@orklah
Copy link
Collaborator

orklah commented Oct 17, 2023

@ptomulik if you're able to take a look, it would be very helpful :)

@ptomulik
Copy link
Contributor

I guess, we shall start from here: #9439 (comment)

@ptomulik
Copy link
Contributor

ptomulik commented Oct 18, 2023

Found another variant for this bug, but this time it seems to be independent of 2327917:

https://psalm.dev/r/338c7bae15

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/338c7bae15
<?php

/**
 * @psalm-type PropTypes = array{
 *   lkey?: list<array{'content-type': string, version: string}>,
 *   skey?: string,
 * }
 */
class Test {
    /**
     * @psalm-var PropTypes
     */
    public $props = [];

    public function storeProperty(string $name): void
    {
        switch ($name) {
            case 'lkey':
                if (!isset($this->props[$name])) { }
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }

    public function storePropertyWorks(string $name): void
    {
        switch ($name) {
            case 'lkey':
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }
}
Psalm output (using commit 24168f6):

ERROR: InvalidPropertyAssignmentValue - 21:17 - $this->props with declared type 'array{lkey?: list<array{'content-type': string, version: string}>, skey?: string}' cannot be assigned type 'array{lkey: list<array{'content-type': string, version: string}>|string, skey?: string}'

@ptomulik
Copy link
Contributor

This gets more and more interesting, deleted skey from the previous example and the error has gone

https://psalm.dev/r/5223e6b4cd

now get it back, but change the, seeminly unrelated skey type to, let it be, int, one error turned into two

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

looks like types from different keys of array interfere here, reverting 2327917 doesn't help

can anyone explain? @orklah, @robchett?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/5223e6b4cd
<?php

/**
 * @psalm-type PropTypes = array{
 *   lkey?: list<array{'content-type': string, version: string}>,
 * }
 */
class Test {
    /**
     * @psalm-var PropTypes
     */
    public $props = [];

    public function storeProperty(string $name): void
    {
        switch ($name) {
            case 'lkey':
                if (!isset($this->props[$name])) { }
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }

    public function storePropertyWorks(string $name): void
    {
        switch ($name) {
            case 'lkey':
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }
}
Psalm output (using commit 24168f6):

No issues!
https://psalm.dev/r/1e02b68bcd
<?php

/**
 * @psalm-type PropTypes = array{
 *   lkey?: list<array{'content-type': string, version: string}>,
 *   skey?: int
 * }
 */
class Test {
    /**
     * @psalm-var PropTypes
     */
    public $props = [];

    public function storeProperty(string $name): void
    {
        switch ($name) {
            case 'lkey':
                if (!isset($this->props[$name])) { }
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }

    public function storePropertyWorks(string $name): void
    {
        switch ($name) {
            case 'lkey':
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name][] = $addrData;
                break;
        }
    }
}
Psalm output (using commit 24168f6):

ERROR: PossiblyInvalidArrayAssignment - 21:17 - Cannot access array value on non-array variable $this->props[$name] of type int

ERROR: InvalidPropertyAssignmentValue - 21:17 - $this->props with declared type 'array{lkey?: list<array{'content-type': string, version: string}>, skey?: int}' cannot be assigned type 'array{lkey: int|list<array{'content-type': string, version: string}>, skey?: int}'

@ptomulik
Copy link
Contributor

Possibly my PR #9439 didn't fix issue, just turned false-negatives into false positives, looks like #9433 is still here.

@ptomulik
Copy link
Contributor

$this->props[$name] vs $this->props[$name . '']

https://psalm.dev/r/1149743dfb

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/1149743dfb
<?php

/**
 * @psalm-type PropTypes = array{
 *   lkey?: list<array{'content-type': string, version: string}>,
 *   skey?: int
 * }
 */
class Test {
    /**
     * @psalm-var PropTypes
     */
    public $props = [];

    public function storeProperty(string $name): void
    {
        switch ($name) {
            case 'lkey':
                if (!isset($this->props[$name])) { }
                $addrData = [ 'content-type' => 'text/vcard', 'version' => '3.0' ];
                $this->props[$name . ''][] = $addrData;
                $this->props[$name][] = $addrData;
                break;
        }
    }
}
Psalm output (using commit 24168f6):

ERROR: PossiblyInvalidArrayAssignment - 22:17 - Cannot access array value on non-array variable $this->props[$name] of type int

ERROR: InvalidPropertyAssignmentValue - 22:17 - $this->props with declared type 'array{lkey?: list<array{'content-type': string, version: string}>, skey?: int}' cannot be assigned type 'array{lkey: int|list<array{'content-type': string, version: string}>, skey?: int}'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants