Resolve Static throw points

[VincentLanglet]

Closes phpstan/phpstan#11900

[ondrejmirtes]

These are not the right places to do this fix. The MethodReflection has to already answer correctly.

The key lies in UnresolvedMethodPrototypeReflection (interface). ObjectType and StaticType return different implementations of this interface.

The two most important implementations are:

• CallbackUnresolvedMethodPrototypeReflection (used in StaticType)
• CalledOnTypeUnresolvedMethodPrototypeReflection (used in ObjectType)

When called on StaticType, the type has to stay the same/base class only being updated.

When called on ObjectType, the throw type has to be resolved to the current final type.

[VincentLanglet]

These are not the right places to do this fix. The MethodReflection has to already answer correctly.

The key lies in UnresolvedMethodPrototypeReflection (interface). ObjectType and StaticType return different implementations of this interface.

The two most important implementations are:

• CallbackUnresolvedMethodPrototypeReflection (used in StaticType)
• CalledOnTypeUnresolvedMethodPrototypeReflection (used in ObjectType)

When called on StaticType, the type has to stay the same/base class only being updated.

When called on ObjectType, the throw type has to be resolved to the current final type.

Ok, so I did the fix in CallbackUnresolvedMethodPrototypeReflection & CalledOnTypeUnresolvedMethodPrototypeReflection and it works fine for the method and the static method.

But I still get error for the case 3 and 4 (constructor and property hooks) without a specific fix.
Do you know how to achieve the same "static" strategy for constructor & property hooks @ondrejmirtes ?

Should I just split the PR in two and just test the case 1 and 2 for now (without the fix 3 and 4) which already close the issue and cover most of real-life cases ?

[ondrejmirtes]

Should I just split the PR in two and just test the case 1 and 2 for now (without the fix 3 and 4)

Yes please, then open a PR with failing tests after this one is merged

[VincentLanglet]

Should I just split the PR in two and just test the case 1 and 2 for now (without the fix 3 and 4)

Yes please, then open a PR with failing tests after this one is merged

PR updated

[ondrejmirtes]

1. Please remove the property hook and the constructor throws and also the 8.4 requirement.
2. Please add a thorough test for a dead catch rule to make it obvious this really works properly when calling a method on an object variable vs. on $this. Also that changeBaseClass works.

[VincentLanglet]

Please add a thorough test for a dead catch rule to make it obvious this really works properly when calling a method on an object variable vs. on $this. Also that changeBaseClass works.

Sorry I'm not sure to understand what should be the test. Could you give me a snippet ?

I don't see how TestDataException or ADataException will be reported as a dead catch

[ondrejmirtes]

@VincentLanglet Thinking about it, this should rather be a TypeInferenceTestCase in /nsrt/ testing assertType of $e in catch blocks. That way we can test the exact exception type the method throws.

1. $obj->foo() with @throws static should result in $obj type.
2. $this->foo() with @throws static should result in StaticType
3. $this->foo() with @throws static from a parent class should result in an updated StaticType
4. $this->foo() with @throws static in a final class should result in a ObjectType (I think, at least that's what it was at some point)

[ondrejmirtes]

Also, one more place needs fixing. MutatingScope::enterClassMethod should also transformStaticType on $throwType.

Not sure how to test that. Maybe there's already a rule that asks that on $scope->getFunction(). Maybe we need to write a fake one just for tests.

[VincentLanglet]

@VincentLanglet Thinking about it, this should rather be a TypeInferenceTestCase in /nsrt/ testing assertType of $e in catch blocks. That way we can test the exact exception type the method throws.

1. $obj->foo() with @throws static should result in $obj type.
2. $this->foo() with @throws static should result in StaticType
3. $this->foo() with @throws static from a parent class should result in an updated StaticType
4. $this->foo() with @throws static in a final class should result in a ObjectType (I think, at least that's what it was at some point)

I'm not sure about what I should catch since if you look at https://phpstan.org/r/030ef155-337f-4b7d-8361-7bf9f4f6c225
The dumpType return the type caught an not the real type thrown.

Also, one more place needs fixing. MutatingScope::enterClassMethod should also transformStaticType on $throwType.

I assume it should be the same for enterPropertyHook ?

Not sure how to test that.

I dunno either since I'm not even sure about the impact of changing enterClassMethod/enterPropertyHook.