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

InvalidReturnStatement for lists with shape inferred from PHP code with conditions #8942

Closed
alies-dev opened this issue Dec 19, 2022 · 6 comments
Labels

Comments

@alies-dev
Copy link
Contributor

Psalm: 5.3.0

list<string> should be a superset of list{0?: string, 1: string}.

https://psalm.dev/r/c1147fc79d

@psalm-github-bot
Copy link

I found these snippets:

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

/** @return non-empty-list<string> */
function buildListWithOptionalValues(): array {
    $list = [];
    $entropy = random_int(0, 2);
    if ($entropy === 0) {
        $list[] = 'A';
    } elseif ($entropy === 1) {
        $list[] = 'B';
    }

    $list[] = 'C';

    return $list;
}

buildListWithOptionalValues();
Psalm output (using commit 96cb44b):

ERROR: InvalidReturnStatement - 15:12 - The inferred type 'list{0?: 'A'|'B', 1: 'C'}' does not match the declared return type 'non-empty-list<string>' for buildListWithOptionalValues

ERROR: InvalidReturnType - 3:13 - The declared return type 'non-empty-list<string>' for buildListWithOptionalValues is incorrect, got 'list{0?: 'A'|'B', 1: 'C'}'

@danog
Copy link
Collaborator

danog commented Dec 19, 2022

list{0?: string, 1: string} is actually not a valid list type (so the error would be correct, but the type should've also been marked as array), and it shouldn't have been generated in the first place (a list{0: 'A'|'B'|'C', 1?: 'C'} should've been generated instead).

I've already started work to fix this and a number of other issues, it's a bit tricky :)

@danog danog added the bug label Dec 19, 2022
danog added a commit to nicelocal/psalm that referenced this issue Dec 19, 2022
@boesing
Copy link
Contributor

boesing commented Dec 19, 2022

Not sure if it is worth handling this.

Even if array{0: string, 1: string} would be interpreted as list that would be a flaw, because the array-key order is probably not really handled by psalm.

https://psalm.dev/r/2c3fa92ffa
https://3v4l.org/3FHbY#v8.1.13

Or I am misinterpreting something here. Maybe. Just stumbled upon this and was confused.

@psalm-github-bot
Copy link

I found these snippets:

https://psalm.dev/r/2c3fa92ffa
<?php


/** @return list<string> */
function createListWithTwoStringElements(): array
{
    return ['foo', 'bar'];
}

/** @return array{0:string,1:string} */
function createArrayWithTwoStringElements(): array
{
	return [
    	1 => 'bar',
        0 => 'foo',
    ];
}

var_dump(array_is_list(createListWithTwoStringElements()), array_is_list(createArrayWithTwoStringElements()));
Psalm output (using commit 96cb44b):

ERROR: RedundantCondition - 19:10 - list<string> always contains list<mixed>

ERROR: ForbiddenCode - 19:1 - Unsafe var_dump

@danog
Copy link
Collaborator

danog commented Dec 19, 2022

array{0: string, 1: string} is, correctly, not interpreted as a list, so it's not an issue.

@danog
Copy link
Collaborator

danog commented Dec 19, 2022

Fixed!

@danog danog closed this as completed Dec 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants