-
Notifications
You must be signed in to change notification settings - Fork 510
Include intializations from parent scope for anonymous functions #2607
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
Include intializations from parent scope for anonymous functions #2607
Conversation
src/Node/ClassPropertiesNode.php
Outdated
if ($usageScope->isInAnonymousFunction() && $usageScope->getParentScope() !== null) { | ||
$hasInitialization = $hasInitialization->or($usageScope->getParentScope()->hasExpressionType(new PropertyInitializationExpr($propertyName))); | ||
} |
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.
maybe only enter this IF, if the outer !hasInitialization->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.
Good point, thanks.
I also moved the initialization of $hasInitialization
below the IF which ignores functions other than the constructor (https://github.com/phpstan/phpstan-src/pull/2607/files#diff-0e5b4f9339ad4cf44915b25c9ea53bf150de4b6fd060856247cc74b0fc0c54eaR225). Since the conditions in that IF do not depend on $hasInitialization
, we can continue
the loop earlier. I hope that's ok.
9c6262b
to
0a513d2
Compare
Is there anything I need to do to move this forward? The failing tests do not seem to be caused by the changes introduced in this PR. |
I've been thinking about this, and the main thing here is that I'm not sure that this is the right fix. I just thought about it and I couldn't find a way how to break this so we're probably good to go. Time and feedback will tell us if there's something wrong with it :) Thank you! |
I see, thanks for explaining. |
This looks pretty similar to the problem you fixed here, can you please look into it? Thank you: phpstan/phpstan#10048 |
Fixes bug #9831