add failing test from https://github.com/phpstan/phpstan/issues/8467

[rajyan]

phpstan/phpstan#8467

[rajyan]

@ondrejmirtes
If my understanding is correct, this test should pass with 1.9.2, but it doesn't 🤔
I think I'm missing something...

[ondrejmirtes]

Yeah, I probably didn't explain myself clearly.

The situation in Composer sources is complicated.

I came up with a simple example. I also thought it'd pass on 1.9.2 but it doesn't.

But if we fix it, it's gonna fix the problem in Composer too I think.

Because you made conditional types smarter, this situation is now happening in more places.

I hope this reasoning is more clear than before 😊

[rajyan]

But if we fix it, it's gonna fix the problem in Composer too I think.
Because you made conditional types smarter, this situation is now happening in more places.
I hope this reasoning is more clear than before

I see. Thanks, I understand.
I haven't yet figured out the cause of the problem, but I'll look for it 👍

[ondrejmirtes]

My theory is that we might need to disable this 35db779 for !$this->polluteScopeWithAlwaysIterableForeach.

[rajyan]

This is one idea to solve the current problem.
Reading
https://phpstan.org/config-reference#pollutescopewithalwaysiterableforeach
I thought that certainty "no" is better for foreach value and key outside the loop?

[ondrejmirtes]

You need "maybe" so that people can ask about it with isset. And for backward compatibility because there are gonna be baselines that have "might not be defined" in their messages.

"no" would not be precise, because that's not how PHP works. Even with the stricter setting (polluteScopeWithAlwaysIterableForeach: false).

[rajyan]

Fair enough. I'll look for another solution.
The conditional expression is created in createConditionalExpressions of mergeWith, so invalidating them should be enough.

[rajyan]

My theory is that we might need to disable this 35db779 for !$this->polluteScopeWithAlwaysIterableForeach.

Sorry, I read this comment now. Yeah, I think so too.

[rajyan]

For the rector test case
https://github.com/rectorphp/rector-src/blob/71b1142ee41eceef21d9c3d3a1bf5bc4f67ad9d2/packages/BetterPhpDocParser/PhpDocParser/DoctrineAnnotationDecorator.php#L83
I think PHPStan is right, this line can be while(true)

rest of the errors are from
https://github.com/symplify/phpstan-rules/blob/66c00e1680806cd1e59c3f90181dbda23a76c8cb/src/Rules/Explicit/NoMixedArrayDimFetchRule.php
this rule. Not sure yet why this change makes $types[$key] as mixed.

PocketMine is wired, I'm not sure why it wasn't an error before this change.
https://github.com/pmmp/PocketMine-MP/blob/e0b07ff3087b652407439a29c941f3b66ca92c86/src/pocketmine/Server.php#L2259

[ondrejmirtes]

I've realized there's a problem. I think it'd be a nice idea to have a new TypeInferenceTestCase subclass, and set polluteScopeWithAlwaysIterableForeach: false in getAdditionalConfigFiles.

That way we can test it fully and not just through DefinedVariableRuleTest.

Because even with polluteScopeWithAlwaysIterableForeach: false, we want this to pass:

// $a is array
foreach ($a as $v) {
    // $a is non-empty-array
}

But where the problem is lies (and what might be hard to fix) is that the conditional types shouldn't create the connection (if $a is non-empty again then $v definitely exists, or something like that).

The current solution might be fine if we don't come up with anything better, but let's try :)

The problem in PocketMine-MP was already ignored, but with a different message probably (non-empty-array).

[ondrejmirtes]

I've just had breakfast and changed my mind - I want to try this in practice :) Feel free to mark this current changeset as ready, I'm gonna merge it, and if you come up with any ideas how to improve it, we can try it out, but I think the current state is okay to try out :)

[rajyan]

The problem in PocketMine-MP was already ignored, but with a different message probably (non-empty-array).

This was a good news, the only concern for me was this test change.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

Thank you :)

[rajyan]

I'll add this to TODOs
phpstan/phpstan#8360 (comment)