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/issue 3171 dynamic properties #1237

Merged
merged 6 commits into from Apr 22, 2022

Conversation

rajyan
Copy link
Contributor

@rajyan rajyan commented Apr 22, 2022

fixes phpstan/phpstan#3171 completly

The error was suppressed by #1234
but still happening if checkDynamicProperties is enabled.
https://phpstan.org/r/7092e4dc-cfed-4fcf-810a-7ad5e3b9555e

It is because, ensureNonNullability was not traversing on ArrayDimFetch. d450536.
Also, I think the logic for ensureNonNullability and allowedUndefinedExpression should be consistent with each other, so refactored ensureNonNullability by using lookForExpressionCallback.

As I mentioned in a624060
there is one more possible false positive, property fetch on Class|false (or something like false that doesn't accept fetching properties), but I think it is too complicate to solve now.

When property fetch is called inside isset, the isset truthy scope is more specific than non-null, it can say "the fetched var has a !hasProperty->no() type". Similar type specification is possible in ArrayDimFetch too. (adding this level of specification can solve issues like phpstan/phpstan#7073, but maybe too much)

@rajyan
Copy link
Contributor Author

rajyan commented Apr 22, 2022

The $findMethods option is deleted. b8a6d7f
it had this
https://phpstan.org/r/397d02ef-b073-4dcc-9a96-f6ad0a83dad4
difference between coalesce and isset before, but it should report error on isset too. $this->property can be null and method call on null is an fatal error even inside isset.
coalesce
https://3v4l.org/QhW5u
isset (I added an array dim fetch in this example because without that, it's another fatal error)
https://3v4l.org/a9ITP

@ondrejmirtes
Copy link
Member

Perfect, thank you!

@ondrejmirtes ondrejmirtes merged commit fb83f47 into phpstan:1.6.x Apr 22, 2022
@rajyan rajyan deleted the fix/issue-3171-dynamic-properties branch April 22, 2022 07:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants