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

Fix enum crash #10643

Closed
wants to merge 3 commits into from
Closed

Fix enum crash #10643

wants to merge 3 commits into from

Conversation

AegirLeet
Copy link

Fixes #10560.

First commit fixes the crash.

Second commit fixes the InvalidEnumCaseValue revealed once the crashing is fixed.

Third commit adds two test cases for the crash fix.

I'd like to add some more test cases to validate the value is resolved correctly, but not sure where.

@weirdan
Copy link
Collaborator

weirdan commented Feb 3, 2024

Does https://psalm.dev/r/38dc5a68ea (referencing the enum below rather than above) work with your PR?

Copy link

I found these snippets:

https://psalm.dev/r/38dc5a68ea
<?php declare(strict_types=1);

enum AnotherEnum: string
{
    case ANOTHER_VALUE = SomeEnum::SOME_VALUE->value;
}

enum SomeEnum: string
{
    case SOME_VALUE = 'SOME_VALUE';
}
Psalm encountered an internal error:

/vendor/vimeo/psalm/src/Psalm/Internal/PhpVisitor/Reflector/ClassLikeNodeScanner.php: Failed to infer case value for ANOTHER_VALUE

@AegirLeet
Copy link
Author

No, that still fails. Looks like the ClassLikeStorageProvider doesn't know about SomeEnum at that point. This seems to be a general issue with enums referencing things declared later. Seems to work fine with two classes and consts though (https://psalm.dev/r/5d409f074e).

I can look into it, but probably not before monday.

Copy link

I found these snippets:

https://psalm.dev/r/5d409f074e
<?php

declare(strict_types=1);

class Foo
{
    public const BAR = Baz::QUX;
}

class Baz
{
    public const QUX = 123;
}

/** @psalm-trace $x */
$x = Foo::BAR;
echo $x;
Psalm output (using commit bf57d59):

INFO: Trace - 16:1 - $x: 123

weirdan added a commit to weirdan/psalm that referenced this pull request Feb 5, 2024
Resolves a number of long-standing bugs ('Failed to infer case value ...')

Fixes vimeo#10374
Fixes vimeo#10560
Fixes vimeo#10643
Fixes vimeo#8978
weirdan added a commit to weirdan/psalm that referenced this pull request Feb 5, 2024
Resolves a number of long-standing bugs ('Failed to infer case value ...')

Fixes vimeo#10374
Fixes vimeo#10560
Fixes vimeo#10643
Fixes vimeo#8978
weirdan added a commit to weirdan/psalm that referenced this pull request Feb 5, 2024
Resolves a number of long-standing bugs ('Failed to infer case value ...')

Fixes vimeo#10374
Fixes vimeo#10560
Fixes vimeo#10643
Fixes vimeo#8978
@AegirLeet
Copy link
Author

@weirdan Is this still needed with #10655?

@weirdan
Copy link
Collaborator

weirdan commented Feb 5, 2024

No, that PR addresses the issue you were trying to fix in a more general way.

@weirdan weirdan closed this Feb 5, 2024
@AegirLeet AegirLeet deleted the fix-enum-crash branch February 5, 2024 11:31
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 this pull request may close these issues.

"Failed to infer case value" when enum case references another enum
2 participants