Don't remember types after impure assignments#5514
Don't remember types after impure assignments#5514staabm wants to merge 15 commits intophpstan:2.1.xfrom
Conversation
staabm
force-pushed
the
no-impre
branch
4 times, most recently
from
a84fa7c to
7f91cb0
Compare
|
This pull request has been marked as ready for review. |
| || $expr instanceof Expr\StaticCall | ||
| ) | ||
| ) { | ||
| if (!$this->rememberPossiblyImpureFunctionValues) { |
There was a problem hiding this comment.
I'm not sure to understand the logic here.
If we rememberPossiblyImpureFunctionValues: false, you automatically continue.
But what if rememberPossiblyImpureFunctionValues is false BUT the function has @phpstan-pure ? We should not continue no ?
There was a problem hiding this comment.
my thinking is:
the logic we have here depends on impure points, which only can exist or not. we cannot reflect a "maybe has impure point" situation.
thats why I allow narrowing using function/method/static-calls in assignments only when rememberPossiblyImpureFunctionValues=true to be defensive.
There was a problem hiding this comment.
When rememberPossiblyImpureFunctionValues=true and the method is maybe impure, seems like we're calling
$scope->assignExpression(new PossiblyImpureCallExpr
not sure if it helps.
Another idea:
Couldn't we add manually an ImpurePoint for non Pure method when rememberPossiblyImpureFunctionValues=false ?
|
Competing PR: #5528 I'll let you decide which one you like more. I'm also looking forward to issue-bot results. |
|
Why isn't issue-bot reporting any changes for this PR? I'd understand if there was already a comment changing to |
|
In case of my PR it is reporting changes: https://github.com/phpstan/phpstan-src/actions/runs/24893411085 |
|
It was your idea in the end and it seems I did it wrong. Lets merge the working one than :-) |
4 participants
Allow remembering results of assignments with callables (conditional expressions), when the call is pure.
closes phpstan/phpstan#9455
closes phpstan/phpstan#5207