Fixed '$this instanceof X will always be false' in traits

[staabm]

closes phpstan/phpstan#3632

test fails without the fix

1) PHPStan\Rules\Classes\ImpossibleInstanceOfRuleTest::testBug3632
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'10: Instanceof between *NEVER* and Bug3632\OtherClass will always evaluate to false.
 '

[staabm]

I have approached the problem from a few different angles. All worked more or less.

At the end I get the feeling maybe this should be fixed at the core by e.g. treating $this just like a simple ObjectType when analysing in trait context

[staabm]

regarding the build: I can see the same errors in other PRs so I think these are unrelated

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

The underyling issues we need to fix is: https://phpstan.org/r/d8b183e1-7099-478e-af58-fdd5e8ab7d21

So it's not sufficient to try to if-else our way out of this problem in a single rule, we need to fix the typesystem here.

So something needs to happen to handling of Instanceof_ in TypeSpecifier.

[staabm]

So something needs to happen to handling of Instanceof_ in TypeSpecifier.

that was a great hint, thank you. I feel dumb for searching the solution in a lot of other phpstan components, but not the TypeSpecifier :)

[ondrejmirtes]

I decided this still doesn't work and we need a different approach. ThisType in a trait context needs to behave a bit differently than in a class. It needs to allow intersection with other classes, almost like an interface.

[staabm]

So the bottom line is, that we need a new type?

[ondrejmirtes]

Some boolean arg in ThisType would suffice. But maybe TraitThisType would work too.

[ondrejmirtes]

I see some problems with this PR.

1. TypeSpecifier shouldn't be touched. Instead, MutatingScope::enterTrait should handle ThisType.
2. Any changes to isSuperTypeOf logic needs to be heavily covered by tests in TypeCombinatorTest - in dataUnion and dataIntersection.
3. There are some cases the method should return no: a) for a final class that does not use the trait. b) For an ObjectType with a trait name c) For a type that's already subtracted in ThisType. This influences how unions and intersections are gonna end up looking like.

[staabm]

for some reason this error is not reported. I get a error though, when running phpstan on this file in isolation.

otherwise I think this looks fine. I will add new TypeCombinator Tests now

[ondrejmirtes]

That's ExistingClassInInstanceOfRule rule, not ImpossibleInstanceOfRule rule.

[phpstan-bot]

This pull request has been marked as ready for review.

[ondrejmirtes]

No need for array_merge. Simply do this before:

$expressionTypes = $this->expressionTypes;
$expressionType['$this'] = $thisHolder;

And the same thing for nativeExpressionTypes.

[staabm]

I see. I was inspired by another place where it was done this way. fixed both in 749fbab

[ondrejmirtes]

That's ExistingClassInInstanceOfRule rule, not ImpossibleInstanceOfRule rule.

[ondrejmirtes]

Thank you.

[staabm]

thanks for the directions. I couldn't have finished this one without your feedback <3.