Cannot figure out how to get the template type from extended/implemented classes

[dpi]

This is either a support request or a bug request, I'm not sure yet, but it does affect whether I adopt this library.

Following on my example I shared in #13 (comment)

use Ramsey\Collection\Set;
final class TestClass {
 /**
 * @param Set<string> $c
 */
 public function __construct(public Set $c = new Set('string')) {
 }
}

If you refer to the published code at https://github.com/ramsey/collection, generics is made use of thorougly.

At each level, the library @extends/@implements appropriately with templates: Set > AbstractSet > AbstractCollection > CollectionInterface

My goal is to be able to reflect on method parameters, for example TestClass::$c->add($element). This parameter has a mixed native type, but it has @param T $element in CollectionInterface.

But when I use reflection like this, I get a MixedType:

$reflector = (new ReflectorBuilder())->build();
$test = $reflector->forType(TestClass::class);
$setType = $reflector->forNamedType($test->properties()[0]->type());
foreach ($setType->methods() as $m) {
 if ($m->name() === 'add') {
 $m->parameters[0] === MixedType

I would like to somehow to be able to get the T, which should be a string. Is this possible with good-php/reflection?

[autaut03]

Yes, it should be working as you expect. Replacing type parameters with actual types - is supported.

The reason you're getting mixed is that reflection doesn't take into account docblocks of inherited members - e.g. in this case, because add is also declared in AbstractSet without a correct docblock, it reports mixed as if the parameter wasn't typed at all.

This should be fixable though; PHPStan does this: https://github.com/phpstan/phpstan-src/blob/c985b8509d71062fcee468050c6c0765164b328b/src/PhpDoc/PhpDocInheritanceResolver.php

I'll work on this.

[dpi]

Thanks!

[dpi]

I see if I add

 /**
 * @param T $element
 */`

To \Ramsey\Collection\AbstractSet::add, or even \Ramsey\Collection\Set::add, things look better.

But if I comment out AbstractSet::add, so only the AbstractCollection::add version takes effect, its not working.

No luck either with @method add(T $element) above \Ramsey\Collection\Set.

[autaut03]

AbstractCollection::add version takes effect, its not working.

That one doesn't have a "correct" docblock either :) I'd expect that if you remove both AbstractSet::add and AbstractCollection::add, then it would work as expected.

[dpi]

Oh, I thought AbstractCollection::add had docs, but it doesnt, its CollectionInterface::add.

Whats incorrect about it these?

My understanding is no matter what, inheritance is always implied. If something is omitted in a child class, its definitions always come from above.

[oprypkhantc]

Whats incorrect about it these?

I put it in quotes because it's neither correct nor incorrect - there isn't really any standard or spec on how to handle such cases, so it's hard to tell what's "correct". What I meant is that it's missing the full "signature" of the function, including the @param T tag.

PHPStan (and probably Psalm) does handle it, but with other programming languages that natively support generics, you'd be required to specify all types (including generic ones) of a method again when overwriting it. It avoids a lot of issues and hassles.

To me, that's weird that the docblock is missing in overwritten methods. It's the same as if you could omit native typehints in parameters/return types of overwritten methods - but you can't.

My understanding is no matter what, inheritance is always implied. If something is omitted in a child class, its definitions always come from above.

Correct. Basically how it works right now is it parses all methods (including those from nested parents and interfaces), but always prefers one "on the top", including all of it's types, docs and other things. It doesn't attempt to "inherit" the docblocks or types from other methods.

To counter this, PHPStan does some pretty elaborate remapping, because there are a lot of things to consider here when "merging" definitions of the same method from the entire class tree:

• parameters may have different names in different classes
• children may "widen" parameter types natively
• parents may have incomplete docblocks (describing only one of the parameters, for example, or none of them)
• parents may have different names for generic types (for example TValue in Set, while the same parameter is called T in CollectionInterface. Not the case here, but it may happen)

So it's really not a simple task. Unfortunately we can't just get the docblock from parent if it's missing.

It's also unclear to me what the rules are. For example, if a parent method defines @template T generic for itself, and you don't include a docblock for your overwritten method, does it still "inherit" the method<T>() signature? What if it does include a docblock, but without @template T?

All that said, I'll see what PHPStan does in these situations and try going from there.

[dpi]

Thanks for the explanation, I understand PHPStan and Psalm are helping out a lot.

I created these test cases, to try to explain that methods below get the same template. I'm sure there's a bunch of other behavior and rules that's implied. Would be good to work toward whats an established defacto standard, which both implementations seem to mirror.

Even supporting just class-level @method annotation would help a lot, as it would reduce the need for real PHP code. In any case, class level docs should be supported eventually, with these behaviour changes being a higher priority.

https://phpstan.org/r/23e77ad0-ecb0-4d13-9dee-f47b3029d772

https://psalm.dev/r/14bbcc08fd

[dpi]

Perhaps it is simple as merging docblocks, or the interpreted rules, upwards...

[autaut03]

Thank you for the example.

Perhaps it is simple as merging docblocks, or the interpreted rules, upwards...

Not really. Consider this example: https://phpstan.org/r/6715a095-017c-47b1-80ec-68fcdc78bd79

If we were to just merge docblocks upwards, $arg's type would be T (from A::m()). But in the scope of class B, template type T doesn't exist - only TRenamed. Furthermore, TRenamed might have a different upper bound (e.g. TRenamed of 'bar').

Then let's consider this example: https://phpstan.org/r/50fbd527-fd67-4261-8405-a70ede0599b3

Both classes A and B define a valid docblock for method B, but argument $arg uses the template type from A::m(). Furthermore, even though B overwrites the @param TOtherMethodParam $param type, it still actually uses the type from A::m() definition:

Dumped type: TMethodParam of int (method A::m(), parameter)

And there are a lot of cases like these.

[autaut03]

Another variation of the first example: https://phpstan.org/r/9595c8b1-65b4-4712-b6f3-e9d156ab2284

Again, if we were to just blindly merge the PHPDoc of A::m() upwards, the type of $arg would remain T of string, when in reality it must be literal 'foo'. So it's not just a matter of renaming parameters. We have to consider all "parents" of a method and judge the true "source" of a given type.

But since that could be scattered across multiple classes, and since it could use a template type that is also used in @return tag, it becomes quite complex very quickly.

[autaut03]

@dpi Should be fixed. I don't guarantee it works exactly the same way as PHPStan does - I've already found edge cases where PHPStan is definitely wrong. But it does support your specific example, and hopefully many more similar cases:

$testClassReflection = $this->reflector->forType(TestClass::class);

$setReflection = $this->reflector->forNamedType(
	$testClassReflection->property('c')->type()
);

$type = $setReflection->method('add')->parameter(0);

assert($type === PrimitiveType::string());

Also added some utility methods for fetching a single property, method and parameter :)

[github-actions]

🎉 This issue has been resolved in version 2.2.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀