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

Passing class-string-map to variadic crashes psalm #5434

Closed
AndrolGenhald opened this issue Mar 19, 2021 · 4 comments
Closed

Passing class-string-map to variadic crashes psalm #5434

AndrolGenhald opened this issue Mar 19, 2021 · 4 comments

Comments

@AndrolGenhald
Copy link
Collaborator

https://psalm.dev/r/93f8d3fbc4

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/93f8d3fbc4
<?php

class A {}

/** @param array<array-key, mixed> $args */
function takesVariadic(...$args): void {
}

/** @var class-string-map<A, A> */
$map = [A::class => new A()];
takesVariadic(...$map);
Psalm encountered an internal error:

/vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php: Argument 2 passed to Psalm\Internal\Analyzer\Statements\Expression\Call\ArgumentAnalyzer::verifyType() must be an instance of Psalm\Type\Union, null given, called in /vendor/vimeo/psalm/src/Psalm/Internal/Analyzer/Statements/Expression/Call/ArgumentAnalyzer.php on line 508

@AndrolGenhald
Copy link
Collaborator Author

AndrolGenhald commented Mar 19, 2021

So the issue is here, and the @var annotation prevents psalm from catching the issue.

I was thinking about adding a @psalm-type Type=array{array?: array-types, int?: int-types, etc} to Union and having getAtomicTypes() return that, which would be beneficial in a lot of other places as well, but it's currently a non-empty-array and it's not possible to have a non-empty object like array when all keys are optional. It looks like object like arrays are supposed to be nonempty when there are required keys, but that's a bit finicky too: https://psalm.dev/r/23869e7fec

Would it be a good idea to add support for non-empty-array{a?: string, b?: string}?

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/23869e7fec
<?php

/**
 * @psalm-suppress UnusedParam
 * @param non-empty-array<string, string> $arg
 */
function test(array $arg): void {}

/** @var array{a?: string, b: string} */
$a = ['b' => 'b'];
test($a); // 'b' is required, $a should be non-empty-array

/** @var array{a: string, b?: string} */
$b = ['a' => 'a'];
test($b); // works
Psalm output (using commit 42d3bce):

ERROR: ArgumentTypeCoercion - 11:6 - Argument 1 of test expects non-empty-array<string, string>, parent type array{a?: string, b: string} provided

@muglug muglug closed this as completed in c4aea7c May 14, 2021
@muglug
Copy link
Collaborator

muglug commented May 14, 2021

I think it might be useful to have an alias of all the possible array types for such situations (all types where the getKey method returns 'array')

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

No branches or pull requests

2 participants