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

Misparse of enums in list{} #10669

Closed
bwoebi opened this issue Feb 6, 2024 · 6 comments · Fixed by #10678
Closed

Misparse of enums in list{} #10669

bwoebi opened this issue Feb 6, 2024 · 6 comments · Fixed by #10678

Comments

@bwoebi
Copy link

bwoebi commented Feb 6, 2024

https://psalm.dev/r/04fcfb0955

While @return A::X|A::Y standalone works, putting it in a list{} will misinterpret the first part of the expression up to the first colon as array key of the list instead of recognizing the enum value.

Copy link

I found these snippets:

https://psalm.dev/r/04fcfb0955
<?php

enum A {
    case X;
    case Y;
    case Z;
}

/**
 * @return list{A::X|A::Y}
 */
function returnsList(int $c): array {
    if ($c) {
        return [A::X];
    }
    return [A::Y];
}
Psalm output (using commit 4b2c698):

ERROR: InvalidDocblock - 12:1 - :: in array key is only allowed for ::class in docblock for returnsList

@danog
Copy link
Collaborator

danog commented Feb 6, 2024

Just wrap the parts: https://psalm.dev/r/61d2d0ec1c

@danog danog closed this as completed Feb 6, 2024
Copy link

I found these snippets:

https://psalm.dev/r/61d2d0ec1c
<?php

enum A {
    case X;
    case Y;
    case Z;
}

/**
 * @return list{(A::X)|(A::Y)}
 */
function returnsList(int $c): array {
    if ($c) {
        return [A::X];
    }
    return [A::Y];
}
Psalm output (using commit 4b2c698):

No issues!

@bwoebi
Copy link
Author

bwoebi commented Feb 6, 2024

@danog That seems rather like a workaround than a fix - I think it should stay open?

I see no particular parse conflict here, why it should not be possible to parse that?

@danog
Copy link
Collaborator

danog commented Feb 6, 2024

Yeah that can be fixed!

@danog danog reopened this Feb 6, 2024
@weirdan
Copy link
Collaborator

weirdan commented Feb 8, 2024

The reason for not allowing constants in list keys seems to be somewhat technical. Our type AST for keyed arrays uses native php types (int/string) to represent shape keys (they become actual array kes of TKeyedArray::$properties) rather than Psalm types. Thus, we cannot differentiate between A::B and "A::B" - they both have the same string representation, and we cannot reliably resolve constant values during the scanning phase as referenced classes may not have been scanned yet. And we won't be ever able to use standalone constants (e.g. PHP_VERSION) in shape keys using current syntax as it's ambiguous.

But we should be able to support constants in list value types by backtracking and reinterpreting the part being parsed, as a value type, the same way we can do it currently with list{1}.

weirdan added a commit to weirdan/psalm that referenced this issue Feb 8, 2024
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

Successfully merging a pull request may close this issue.

3 participants