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

Incorrect Const Array Type #7729

Closed
rodnaph opened this issue Feb 24, 2022 · 10 comments · Fixed by #7732
Closed

Incorrect Const Array Type #7729

rodnaph opened this issue Feb 24, 2022 · 10 comments · Fixed by #7732

Comments

@rodnaph
Copy link

rodnaph commented Feb 24, 2022

https://psalm.dev/r/81398074e3

(I don't know Psalm but I'm guessing that...) From the above example, Psalm is trying to automatically determine the type of the class constant Foo::VARS (which should be list<string>, which works up until a certain length, but then returns a different type, causing it not to be the intended list<string>.

Remove the last element from the above example and Psalm passes correctly.

Is there a limit here in Psalm? If so is there any way to work around this (annotating the const does not seem to have any effect).

Thanks!

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/81398074e3
<?php

class Foo {
    public const VARS = [
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'y', // REMOVE THIS ARRAY ELEMENT TO MAKE THIS PASS
    ];
}

/** @param list<string> $vars */
function foo(array $vars): void {
    print_r($vars);
}

foo(Foo::VARS);
Psalm output (using commit 997bded):

ERROR: ArgumentTypeCoercion - 65:5 - Argument 1 of foo expects list<string>, parent type non-empty-array<int<0, max>, 'x'|'y'> provided

@orklah
Copy link
Collaborator

orklah commented Feb 24, 2022

There is indeed a limit in the number of offset Psalm allow in a shape. But the inference could be improved in this case to infer non-empty-list<'x'|'y'> instead of non-empty-array<int<0, max>, 'x'|'y'>

@rodnaph
Copy link
Author

rodnaph commented Feb 24, 2022

Thanks for the response!

Do you happen to have have any workarounds to suggest in this case please? It doesn't seem possible to override this ...

/** @var list<string> */
public const VARS = [

https://psalm.dev/r/581ff3a3cd

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/581ff3a3cd
<?php

class Foo {
    /** @var list<string> */
    public const VARS = [
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'y', // REMOVE THIS ARRAY ELEMENT TO MAKE THIS PASS
    ];
}

/** @param list<string> $vars */
function foo(array $vars): void {
    print_r($vars);
}

foo(Foo::VARS);
Psalm output (using commit 997bded):

ERROR: ArgumentTypeCoercion - 66:5 - Argument 1 of foo expects list<string>, parent type non-empty-array<int<0, max>, 'x'|'y'> provided

@orklah
Copy link
Collaborator

orklah commented Feb 24, 2022

Just wait a few days and update Psalm to the next version 😄

@AndrolGenhald
Copy link
Collaborator

We should also probably fix it so that suppression works: https://psalm.dev/r/ecec0a4f0f

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/ecec0a4f0f
<?php

class Foo {
    /** @psalm-suppress ArgumentTypeCoercion */
    public const VARS = [
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'y', // REMOVE THIS ARRAY ELEMENT TO MAKE THIS PASS
    ];
}

/** @param list<string> $vars */
function foo(array $vars): void {
    print_r($vars);
}

foo(Foo::VARS);
Psalm output (using commit 997bded):

ERROR: ArgumentTypeCoercion - 66:5 - Argument 1 of foo expects list<string>, parent type non-empty-array<int<0, max>, 'x'|'y'> provided

@orklah
Copy link
Collaborator

orklah commented Feb 24, 2022

Why add that here? Shouldn't it be here: https://psalm.dev/r/f48dd3e62a ?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/f48dd3e62a
<?php

class Foo {
    public const VARS = [
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'x',
        'y', // REMOVE THIS ARRAY ELEMENT TO MAKE THIS PASS
    ];
}

/** @param list<string> $vars */
function foo(array $vars): void {
    print_r($vars);
}
    /** @psalm-suppress ArgumentTypeCoercion */

foo(Foo::VARS);
Psalm output (using commit 997bded):

No issues!

@AndrolGenhald
Copy link
Collaborator

AndrolGenhald commented Feb 24, 2022

Well now that you fixed it I can't test it 😛
I'll see if I can be bothered to take a look later when I'm back at home. Really all of these issues are due to the improved class const support, ideally the way that the const inferred type is inferred should be the same as the way the assigned type is inferred later when analyzing, then these issues shouldn't pop up again.

Edit: nevermind I'm totally wrong, thought this was something else that's popped up before.

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

Successfully merging a pull request may close this issue.

3 participants