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

Optional array key is not recognized with !empty #3297

Closed
mrpavlikov opened this issue May 13, 2020 · 8 comments
Closed

Optional array key is not recognized with !empty #3297

mrpavlikov opened this issue May 13, 2020 · 8 comments
Labels
Milestone

Comments

@mrpavlikov
Copy link

Bug report

Optional array key is not recongized when using !empty, only works with isset.

Code snippet that reproduces the problem

https://phpstan.org/r/896f13be-9ac0-43bd-9940-d53fbdb6e4be

Expected output

Expected no error.

@ondrejmirtes ondrejmirtes added this to the Easy fixes milestone Nov 22, 2020
@ArtemGoutsoul
Copy link

ArtemGoutsoul commented Jun 9, 2021

Sems like a very common use case - checking if a value exists and then using it.

isset is handling this correctly, but changing !empty usages to isset would require also checking that the value is not falsy... E.g.:

if (isset($param['opt']) && $param['opt']) {

Works!: https://phpstan.org/r/2ae43dc1-4ec4-419f-8f43-7f4345b9e328

This would feel a bit wrong since empty has exactly this functionality...

@ArtemGoutsoul
Copy link

Could this be handled exactly the same way as isset at least? isset check causes phpstan to consider that the key exists, !empty could do the same thing. If someone could point to where is this implemented for isset, I could probably try to implement it for !empty?

@ondrejmirtes
Copy link
Member

It definitely can. Look for Isset_ handling in TypeSpecifier.

@ArtemGoutsoul
Copy link

Thanks for the pointer!

I don't really understand the code yet, since I've never worked with code analyzers / parsers. But could the issue be here?

For Expr\Isset it adds new HasOffsetType($scope->getType($var->dim)), on line https://github.com/phpstan/phpstan-src/blob/3cebee92aaea0271f5f4eef269d6142ea2c00eef/src/Analyser/TypeSpecifier.php#L692, but in the else block (https://github.com/phpstan/phpstan-src/blob/3cebee92aaea0271f5f4eef269d6142ea2c00eef/src/Analyser/TypeSpecifier.php#L702) which seems to be for !empty ( $expr instanceof Expr\Empty_ && $context->false() ? ), this same logic is missing.

@ondrejmirtes
Copy link
Member

First try to write a failing test, probably in https://github.com/phpstan/phpstan-src/blob/master/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php. And then try to do whatever you think should work :)

@ArtemGoutsoul
Copy link

ArtemGoutsoul commented Jul 2, 2021

Thanks for your pointers @ondrejmirtes! Even though I still don't understand the code well, I created a failing test and fixed it. Hopefully the PR is ok (and lets see if the whole CI pipeline passes :) ).

@ondrejmirtes
Copy link
Member

Fixed by: phpstan/phpstan-src#569

@github-actions
Copy link

github-actions bot commented Aug 3, 2021

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 3, 2021
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

3 participants