Improve foreach scope pollution
#3618
Conversation
|
I don't get it. What kind of bug is there in the foreach that needs fixing? |
|
Setting something inside it that does not exist in scope already basically. Weird edge case, but the same as with |
|
But it indeed doesn't look like this was even reported. Not sure if it's worth it then. |
|
|
||
| assertType('int<0, 1>', $c); | ||
| assertNativeType('int', $c); | ||
| assertVariableCertainty(TrinaryLogic::createYes(), $c); |
There was a problem hiding this comment.
This should not be yes but maybe. That's the point of polluteScopeWithAlwaysIterableForeach: false.
Please if for loop currently behaves like that then please fix the behaviour too.
There was a problem hiding this comment.
Oh, the docs just talk about variables set in the loop init statements or the key and value variables. If so, my previous PR basically needs to be reverted and the issue it fixed was by design? :)
I can do this in the evening btw.
There was a problem hiding this comment.
Oh yeah, sorry about that phpstan/phpstan#9550 (comment)
There was a problem hiding this comment.
No worries. Should I open a PR for all those tests only? In case they add value, otherwise I don't mind of course.
There was a problem hiding this comment.
It's fine, I think it's well tested :)
2 participants
equivalent of #3617 but for
foreachandpolluteScopeWithAlwaysIterableForeachset tofalse.not sure yet if there's a better way of dealing with the keyVar/valueVar variable expression checks..
don't know yet what this might be fixing..