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 5903 #1051
Fix/issue 5903 #1051
Conversation
src/Analyser/NodeScopeResolver.php
Outdated
@@ -823,6 +823,9 @@ private function processStmtNode( | |||
if (!$isIterableAtLeastOnce->no()) { | |||
$throwPoints = array_merge($throwPoints, $finalScopeResult->getThrowPoints()); | |||
} | |||
if (!$scope->getType($stmt->expr)->isArray()->yes()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about this instead?
!(new ObjectType(\ArrayAccess))->isSuperTypeOf($scope->getType($stmt->expr)-)->no()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will make it not report a false positive for example array|null
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! I'll look into it.
1345f32
to
83e4284
Compare
Sorry, I used a wrong type in the review and fixed it myself :) 83e4284 (ArrayAccess has nothing to do with iteration). Thank you. |
Oh, you are right. Sorry that I couldn't notice it. I’m wondering why did the tests passed in local. |
Because these objects might implement ArrayAccess in a subclass :) (They're not final.) So the assumption an exception might be thrown is still correct. |
I see. Thank you for clarifying. |
fixes phpstan/phpstan#5903