Fix get_class() on HasMethodType
#2350
Conversation
|
You've opened the pull request against the latest branch 1.11.x. If your code is relevant on 1.10.x and you want it to be released sooner, please rebase your pull request and change its target to 1.10.x. |
| @@ -85,6 +86,8 @@ static function (Type $type, callable $traverse): Type { | |||
| return new GenericClassStringType($type); | |||
| } elseif ($type instanceof ObjectWithoutClassType) { | |||
| return new ClassStringType(); | |||
| } elseif ($type instanceof HasMethodType) { | |||
| return new ClassStringType(); | |||
There was a problem hiding this comment.
I think it'd make more sense to get rid of this whole monstrosity (because it'd still fail for HasPropertyType for example - feel free to add a test for it) and also other types like ObjectShapeType.
Instead the whole TypeTraverser::map should be replacable with some methods called on Type.
Type::isObject()->no()- always returns falseType::isObject()->yes()- never returns falseType::isObject()->maybe()- might return false
As for the actual type, you could do something like this:
new GenericClassStringType(TypeCombinator::intersect($argType, new ObjectWithoutClassType()));It will change the result in some cases but it will still be correct.
There was a problem hiding this comment.
I think your idea works pretty good. one exception is
PHPStan\Analyser\NodeScopeResolverTest::testFileAsserts with data set "C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser/data/bug-7167.php:11" ('type', 'C:\dvl\Workspace\phpstan-src-...67.php', PHPStan\Type\Constant\ConstantStringType Object (...), PHPStan\Type\Generic\GenericClassStringType Object (...), 11)
Expected type class-string<Bug7167\Foo>, got type class-string<Bug7167\Foo::Value> in C:\dvl\Workspace\phpstan-src-staabm\tests\PHPStan\Analyser/data/bug-7167.php on line 11.
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'class-string<Bug7167\Foo>'
+'class-string<Bug7167\Foo::Value>'
which I am not yet sure how to handle
| if ($argType instanceof ObjectShapeType) { | ||
| return new ClassStringType(); | ||
| } |
There was a problem hiding this comment.
I am sure you won't like it, but I think it seems to be necessary.
without this case we get things like
class-string<object{foo: Bug4890\HelloWorld, bar: int, baz?: string}>
when intersecting ObjectShapeType and ObjectWithoutClassType.
clxmstaab
force-pushed
the
bug4890
branch
2 times, most recently
from
7f8af6f to
77c061f
Compare
|
|
||
| interface Proxy {} | ||
|
|
||
| class HelloWorld |
There was a problem hiding this comment.
see https://phpstan.org/r/b6b95d5a-a3fb-4c29-be47-4e7832f00ddc regarding how this test fails before this PR
|
think I figured it out - I had to re-introduce the TypeTraverser for the EnumCaseObjectType |
staabm
force-pushed
the
bug4890
branch
2 times, most recently
from
f62e1b3 to
f4ae84b
Compare
| if ($type instanceof ObjectWithoutClassType) { | ||
| return new GenericClassStringType($type); | ||
| $isObject = $type->isObject(); | ||
| if ($isObject->yes() || $isObject->maybe()) { |
There was a problem hiding this comment.
For which type here it's going to be a maybe?
|
|
||
| $objectType = TypeCombinator::intersect($type, new ObjectWithoutClassType()); | ||
| if ($objectType instanceof StaticType) { | ||
| $objectType = $objectType->getStaticObjectType(); |
There was a problem hiding this comment.
Why are you checking this after an intersection?
closes phpstan/phpstan#4890