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

Refinement of key existence is wrong when using a AND in a condition #7224

Closed
stof opened this issue May 12, 2022 · 7 comments
Closed

Refinement of key existence is wrong when using a AND in a condition #7224

stof opened this issue May 12, 2022 · 7 comments
Labels
Milestone

Comments

@stof
Copy link
Contributor

stof commented May 12, 2022

Bug report

When using if (!isset($a['first_key']) && !isset($a['second_key'])) return;, phpstan refines first_key as not being optional in the array shape of $a after the early return

Code snippet that reproduces the problem

https://phpstan.org/r/292eadfe-27e6-4668-b7e5-ccc91c581290

Expected output

The array_key_exists call is not reported as useless by strict rules, because team_name can indeed be missing when reaching it.

Did PHPStan help you today? Did it make you happy in any way?

@ondrejmirtes
Copy link
Member

/cc @rajyan Sounds like something for you :)

@rajyan
Copy link
Contributor

rajyan commented May 13, 2022

The problem is explained here phpstan/phpstan-src#1307 (comment)

The isset reverNonNullability process is not working for optional keys now, so simply calling a no effect isset call can causes the same issue.
https://phpstan.org/r/38eec6d5-b006-454f-831f-17ae030321d5

@phpstan-bot
Copy link
Contributor

@stof After the latest push in 1.8.x, PHPStan now reports different result with your code snippet:

@@ @@
  7: Dumped type: array{id: string, name?: string, team_name?: string|null}
-12: Dumped type: array{id: string, name?: string, team_name: string|null}
-16: Call to function array_key_exists() with 'team_name' and array{id: string, name?: string, team_name: string|null} will always evaluate to true.
+12: Dumped type: array{id: string, name?: string, team_name?: string|null}
Full report
Line Error
7 `Dumped type: array{id: string, name?: string, team_name?: string
12 `Dumped type: array{id: string, name?: string, team_name?: string

@phpstan-bot
Copy link
Contributor

@rajyan After the latest push in 1.8.x, PHPStan now reports different result with your code snippet:

@@ @@
  7: Dumped type: array{id: string, name?: string, team_name?: string|null}
-12: Dumped type: array{id: string, name?: string, team_name: string|null}
-16: Call to function array_key_exists() with 'team_name' and array{id: string, name?: string, team_name: string|null} will always evaluate to true.
+12: Dumped type: array{id: string, name?: string, team_name?: string|null}
Full report
Line Error
7 `Dumped type: array{id: string, name?: string, team_name?: string
12 `Dumped type: array{id: string, name?: string, team_name?: string

@ondrejmirtes
Copy link
Member

@rajyan This is how I fixed revertNonNullability :) phpstan/phpstan-src@0654852?w=1

The issue was that Scope::specifyExpressionType also changed ArrayDimFetch::$var type without having EnsuredNonNullabilityResult know about it. So I discovered this changed expression by comparing the type against $originalScope, and added another EnsuredNonNullabilityResult with that so that it knows it has to be reverted.

@rajyan
Copy link
Contributor

rajyan commented Jul 26, 2022

@ondrejmirtes
Thank you very much for the fix! Looks good to me 👍

@github-actions
Copy link

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 27, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

4 participants