Fix false positive dead catch on property assignment

[rajyan]

fixes phpstan/phpstan#6256

I hope I understood the concept of ThrowPoint correctly...

[rajyan]

Rerunning tests because the failing tests seemed not related to this PR.

[rajyan]

@ondrejmirtes
Hi, this PR is ready for review.
This is my first PR, and I'm looking forward to contribute more!

[ondrejmirtes]

The error might occur if we have a property with a native type int and we try to assign int|string into it. It means that the condition should be !$assignedExprType->isSuperTypeOf($propertyReflection->getWritableType())->yes() instead. Please add a test and modify the implementation.

Also there might be a situation where the property is not defined at all. In that case we should simulate MethodCall to __set and see what throw points that produces (when the method exists). You can do that by calling $this->processExprNode(new MethodCall($var->var, '__set'), $scope, static function (): void {}, $context)->getThrowPoints().

There's a similar bug that you can look to in the next PR :) phpstan/phpstan#4852

Thank you!

[rajyan]

@ondrejmirtes
Thank you very much for your reviews.
I modified the tests and logic with your suggestions. (By the way, my $assignedExprType>isSuperTypeOf($propertyReflection->getWritableType()) was checking in the opposite direction. The tests were useful to understand those cases.)

[rajyan]

@ondrejmirtes
I'm looking deeper into throw points.
May I ask several questions?

1. Difference between explicit/implicit throw points
2. Do you think this case is a explicit one? (I think it can only throw TypeError)
3. If I should create an explicit throw point, is there a better way to get predefined classes type from scope than this?

ThrowPoint::createExplicit($scope, $scope->getType(new New_(new Name(\TypeError::class))), $assignedExpr, false)

[ondrejmirtes]

Instead of $scope->getType(new New_(new Name(\TypeError::class))) you can do new ObjectType(\TypeError::class). See the documentation: https://phpstan.org/developing-extensions/type-system

Explicit/implicit throw points - "explicit" is for cases where the @throws tag is declared and we know the exact type that is thrown.
Implicit throw point is for cases where some kind of exception might be thrown but we're not sure which one. It's for cases where you're calling a function foo() and it has no @throws tag. Also see the blog post: https://phpstan.org/blog/bring-your-exceptions-under-control#what-does-absent-%40throws-above-a-function-mean%3F

So I guess in this case it's an explicit throw point.

[rajyan]

Thank you very much for your information!

Instead of $scope->getType(new New_(new Name(\TypeError::class))) you can do new ObjectType(\TypeError::class).

phpstan-src/src/Type/ObjectType.php

Line 61 in d32942a

private static array $superTypes = [];

Thanks, I had a misunderstanding watching property $superTypes here. I was thinking that calling new ObjectType(\TypeError::class) is not setting the $superTypes, but this property was only a cache.

[ondrejmirtes]

This needs some tests :)

1. Class without the __set magic method - does not throw anything (dead catch reported).
2. Class with __set magic method and @throws void annotation (dead catch reported).
3. Class with __set magic method and @throws SomeException (dead catch not reported).

[rajyan]

👍

For the first case, there is a implicit ThrowPoint already, when method reflection is not found.
I'm not sure with the purpose of this throw point, so I'm gonna check it.

phpstan-src/src/Analyser/NodeScopeResolver.php

Line 1994 in 2167438

$throwPoints[] = ThrowPoint::createImplicit($scope, $expr);

[rajyan]

It was from this case
84b0934

[rajyan]

Ok, understood
I shouldn’t process __set if its not declared.

[ondrejmirtes]

Thank you!