phpstan-src icon indicating copy to clipboard operation
phpstan-src copied to clipboard

Consider `MixedType` explicitness in `MethodParameterComparisonHelper::isTypeCompatible`

Open herndlm opened this issue 3 years ago • 2 comments

Closes https://github.com/phpstan/phpstan/issues/7415

Doesn't feel like the most elegant solution though.. I played around a bit with MixedType::isSuperTypeOf but did not find something better unfortunately.

herndlm avatar Jul 01 '22 21:07 herndlm

Yeah, the issue https://github.com/phpstan/phpstan/issues/7415 is really weird. I don't think your fix is correct, because the message "is not covariant with tentative return type void of method" should be triggered only when considering native return types. The codepath should only by executed for them. It looks wrong to me to exclude explicit mixed types to fix this case - comparing PHPDoc-based types (generics) shouldn't be done when checking tentative return types...

ondrejmirtes avatar Jul 10 '22 12:07 ondrejmirtes

I need to think more about this but the changes here did not activate the "is not covariant with tentative return type void of method", they were already there. The errors about mixed returns are missing though.

If I understood it correctly, than the problem is that MethodParameterComparisonHelper::isTypeCompatible is currently completely ignoring mixed method parameters (explicitly set via native return type or not set at all, resulting in an implicit mixed). And the prototype parameter type became explicit mixed, which triggers an error on 8.1 if the parameter type is not also set to explicit mixed, right? And that was the only thing I was looking at here, basically trying to distinguish the explicit and implicit mixed types to make it trigger the expected error. I'm not excluding mixed, I think I'm less excluding it if I'm not mistaken.

But this is a kind of error I'm not good with and very unsure :)

herndlm avatar Jul 10 '22 21:07 herndlm

I'm not sure what to do with this one. It makes sense to me, but maybe I misunderstood what was going on. I'll give it a good old last force-push and if this is the wrong solution we can just close it :)

herndlm avatar Aug 23 '22 18:08 herndlm

Alright, thank you :)

ondrejmirtes avatar Aug 24 '22 09:08 ondrejmirtes

I reviewed the situation and indeed it looks correct.

ondrejmirtes avatar Aug 24 '22 09:08 ondrejmirtes

I have a follow-up for you. This is allowed in PHP: https://3v4l.org/GAoP2

But PHPStan started reporting this as an error after this PR: https://phpstan.org/r/66db5967-78af-4abe-b21e-12b311dafcbb

ondrejmirtes avatar Aug 24 '22 12:08 ondrejmirtes