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

The issue message for the $<tmp coalesce var>101 is not understandable #9807

Open
jorgsowa opened this issue May 23, 2023 · 5 comments · May be fixed by #10687
Open

The issue message for the $<tmp coalesce var>101 is not understandable #9807

jorgsowa opened this issue May 23, 2023 · 5 comments · May be fixed by #10687
Labels
easy problems Issues that can be fixed without background knowledge of Psalm good first issue Help wanted

Comments

@jorgsowa
Copy link
Contributor

https://psalm.dev/r/ec8201d718

The description of the issues is not understandable. The term $<tmp coalesce var>101 doesn't explain what's wrong exactly. The phrase $<tmp coalesce var>101 should be probably annotated differently.

ERROR: [RedundantCondition](https://psalm.dev/122) - 7:10 - Type array<array-key, mixed> for $<tmp coalesce var>101 is never null

ERROR: [TypeDoesNotContainNull](https://psalm.dev/090) - 7:33 - Cannot resolve types for $<tmp coalesce var>101 - array<array-key, mixed> does not contain null
@psalm-github-bot
Copy link

I found these snippets:

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

function test_function(?int $number):array {
    return $number ? [$number] : [];
}

var_dump(test_function(null) ?? []);
Psalm output (using commit f90118c):

ERROR: RedundantCondition - 7:10 - Type array<array-key, mixed> for $<tmp coalesce var>101 is never null

ERROR: TypeDoesNotContainNull - 7:33 - Cannot resolve types for $<tmp coalesce var>101 - array<array-key, mixed> does not contain null

ERROR: ForbiddenCode - 7:1 - Unsafe var_dump

@orklah
Copy link
Collaborator

orklah commented May 23, 2023

It comes from here:

$left_var_id = '$<tmp coalesce var>' . (int) $left_expr->getAttribute('startFilePos');

When processing a coalesce operator, Psalm will create a fake expression that looks like this: isset($<tmp coalesce var>XXX) ? $<tmp coalesce var>XXX : [];

the $<tmp coalesce var>XXX must be absolutely unique in the scope because otherwise it may clash with an existing variable in the scope. That's why the XXX is actually the position of the first char of the expression in the file, that way there won't be two clashing variables.

I agree the current syntax is confusing, feel free to make a PR and propose something better (keep in mind that it should be a unique name and must not be something that could already be a variable in the scope)

@orklah orklah added easy problems Issues that can be fixed without background knowledge of Psalm Help wanted good first issue labels May 23, 2023
@AkshatHotCode
Copy link

@orklah I've noticed an issue related to Psalm errors and code readability in our project. I'm planning to work on this by introducing a more descriptive variable name in a coalesce operation.

Would appreciate any thoughts or collaboration on this. Thanks!

@orklah
Copy link
Collaborator

orklah commented Feb 9, 2024

CoalesceAnalyzer is in need of a lot of love!

We have multiple bugs related to the logic in here and the fact that we transform the coalesce into a fake ternary is causing some of those.

But if you want to focus on the readability issue, my previous message summarized what is needed, a unique string that can't conflict with any variable in the scope nor any other similar ternary (for example, imagine your have $a ?? []; in two consecutive lines, the string you generate must be different between the two

We could maybe go with Type array<array-key, mixed> for **$a at line 108 char 5486** is never null that way it is both unique and descriptive

@AkshatHotCode
Copy link

Got it, will fix this issue and raise a PR for the same, At the same time feel you to share more issues that are related to this. Will try and help on them too!

AkshatHotCode added a commit to AkshatHotCode/psalm that referenced this issue Feb 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy problems Issues that can be fixed without background knowledge of Psalm good first issue Help wanted
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants