Fix TypeUtils::getDirectClassNames for nested type #1380
Conversation
mvorisek
force-pushed
the
testcase_dynamic_model_method
branch
6 times, most recently
from
c1621c7 to
e2444d3
Compare
mvorisek
force-pushed
the
testcase_dynamic_model_method
branch
2 times, most recently
from
e5cb2ef to
bb4823d
Compare
mvorisek
force-pushed
the
testcase_dynamic_model_method
branch
2 times, most recently
from
ef5ebfd to
00ace67
Compare
mvorisek
changed the title
|
Do you know you can run the code locally too, righr? ;) |
mvorisek
force-pushed
the
testcase_dynamic_model_method
branch
from
b435d37 to
7549ae9
Compare
mvorisek
force-pushed
the
testcase_dynamic_model_method
branch
from
7549ae9 to
20596c6
Compare
yes, I know, I located the issue on my project and then hoped I will need less pushes here for the repro :) |
|
now the issue is fixed incl. full & simple functional test - can you please merge and tag a new release? |
src/Type/TypeUtils.php
Outdated
| @@ -145,14 +146,12 @@ public static function getDirectClassNames(Type $type): array | |||
| if ($type instanceof UnionType || $type instanceof IntersectionType) { | |||
| $classNames = []; | |||
| foreach ($type->getTypes() as $innerType) { | |||
| if (!$innerType instanceof TypeWithClassName) { | |||
| continue; | |||
| foreach (static::getDirectClassNames($innerType) as $n) { | |||
| Scope $scope | ||
| ): Type { | ||
| // resolve static type and remove all virtual interfaces from it | ||
| if ($methodCall instanceof StaticCall) { |
There was a problem hiding this comment.
This extension is too complicated. Can you just write a simple one just so we can verify it's actually executed? So we can see the method's return type isn't used but the one returned from an extension?
There was a problem hiding this comment.
this is the simplest impl. supporting unlimited nesting, please merge this test case as is
There was a problem hiding this comment.
The fixed issue can be demonstrated with a much simpler extension. Let's say you're writing an extension for a method that has string return type, the extension can just return new IntegerType() and we can assert that int has been used.
There was a problem hiding this comment.
the issue was limited nesting support of union/intersect
This test does support unlimited nesting - it removes virtual interface and reresolves $this/static. I do not think this two needed operations can be further simplified.
There was a problem hiding this comment.
Alright, had to do it myself :P 26d8fde
I verified the test fails without the change in TypeUtils.
ondrejmirtes
force-pushed
the
testcase_dynamic_model_method
branch
from
81b7c93 to
26d8fde
Compare
|
Thanks. |
fixes phpstan/phpstan#7344
needed for atk4/data#932