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

Boolean match in strict binary operands mode emits an error #8562

Closed
danog opened this issue Oct 10, 2022 · 6 comments
Closed

Boolean match in strict binary operands mode emits an error #8562

danog opened this issue Oct 10, 2022 · 6 comments

Comments

@danog
Copy link
Collaborator

danog commented Oct 10, 2022

https://psalm.dev/r/e58299f88a

ERROR: RedundantIdentityWithTrue - a.php:8:13 - The "=== true" part of this comparison is redundant (see https://psalm.dev/228)
            $item instanceof a => 'a',


ERROR: RedundantIdentityWithTrue - a.php:9:13 - The "=== true" part of this comparison is redundant (see https://psalm.dev/228)
            $item instanceof b => 'b',
@psalm-github-bot
Copy link

I found these snippets:

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

class a {}
class b {}

function a(object $item): string {
          return match (true) {
            $item instanceof a => 'a',
            $item instanceof b => 'b',
          };
}
Psalm output (using commit 028ac7f):

No issues!

@orklah
Copy link
Collaborator

orklah commented Oct 10, 2022

Do you have this error on another version of Psalm? The snippet seems fine as is

@danog
Copy link
Collaborator Author

danog commented Oct 10, 2022

This error is happening on upstream master, but the strictBinaryOperands="true" config flag must be set (can't do that in the website).

@orklah
Copy link
Collaborator

orklah commented Oct 10, 2022

Ok :)

The fix should just be to check whether $conditional is a VirtualNotIdentical here:

And there is another with VirtualIdentical in the same file

@danog
Copy link
Collaborator Author

danog commented Oct 10, 2022

Hmm, isn't it a bit hacky?
Couldn't we miss some legit issue, maybe generated by some plugin which relies on virtual AST?
I was thinking of introducing a context property, instead...

@orklah
Copy link
Collaborator

orklah commented Oct 10, 2022

Well, I introduced Virtual nodes exactly for this kind of reason (I was creating a plugin and I needed to differentiate between real code nodes and Virtual nodes created by Psalm for easier analysis. See #5215). So here, Psalm is creating a virtual node with === instead of a match expression so just by checking that, you can deduce it's not really a code that's in the file so it shouldn't be considered as unnecessary

danog added a commit to nicelocal/psalm that referenced this issue Oct 12, 2022
orklah added a commit that referenced this issue Oct 12, 2022
@danog danog closed this as completed Oct 13, 2022
danog added a commit to nicelocal/psalm that referenced this issue Oct 14, 2022
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