Support for generics in interface inference

[arnaud-lb]

Thank you for the awesome work on this library. Valinor is super useful and makes it much easier to deal with external data in a strongly typed project.

I am facing an issue since 0.11 added the following restriction:

It is now mandatory to list all possible class-types that can be inferred by the mapper. This change is a step towards the library being able to deliver powerful new features such as compiling a mapper for better performance.

Currently, this change makes interface inference incompatible with generic classes, as Valinor will not accept a return type like class-string<Foo<A>|Foo<B>> (rightfully, as this is not a valid type) or 'Foo<A>'|'Foo<B>'.

To give some context, I'm trying to map a discriminated union, but the discriminator is on the parent node.

The source looks like this:

{
    "type": "string",
    "node": "..."
}

And the model looks like this:

/** @template T */
interface RootInterface {}

class Root implements RootInterface {
    public function __construct(
        /** @var T */
        NodeInterface $node,
    ){}
}

interface NodeInterface {}
class NodeA implements NodeInterface {}
class NodeB implements NodeInterface {}

Due to the position of the discriminator, I can not directly infer the type of the nested node: I must infer the type of the root node.

It seems that supporting a return type that consists of a union of strings in this case would fix the issue, as I could type the mapper like this:

/** @return 'Root<NodeA>'|'Root<NodeB>' */
fn (string $type) => ...

Here is a full example of my use-case: https://gist.github.com/arnaud-lb/535d89874ed719265f66bc19a88b5fb0

[romm]

Hi @arnaud-lb, thank you for reporting this issue; I did not have this usecase in mind when pushing 1b0ff39 indeed.

From my POV, the following should make sense, although it is not supported by the library I certainly could make this work:

/** @return class-string<Root<NodeA>|Root<NodeB>> */
function inferRootType(string $type): string {
    return match ($type) {
        'NodeA' => 'Root<NodeA>',
        'NodeB' => 'Root<NodeB>',
        default => throw new \Exception(),
    };
}

But static analysis tools wont be happy about that, see PHPStan example and Psalm example; this probably makes sense as Root<NodeA> is not considered a class-string, but Root is.

Do you see any other way than using string values like @return 'Root<NodeA>'|'Root<NodeB>' — even if that would mean contributing to PHPStan/Psalm?

BTW big fan of your work on PHPStan generics. 🙂

[arnaud-lb]

Thank you :D

I wish you and this project a happy new year :)

But static analysis tools wont be happy about that, see PHPStan example and Psalm example; this probably makes sense as Root<NodeA> is not considered a class-string, but Root is.

Agreed. I think that the class-string type would lose usefulness if class-string<Root<NodeA>> was valid, as this does not represent a class name string that PHP constructs can understand.

Do you see any other way than using string values like @return 'Root<NodeA>'|'Root<NodeB>' — even if that would mean contributing to PHPStan/Psalm?

Introducing a new generic-class-string<T> type would solve this issue, but it would have limited usefulness.

Adding support for class-string<Root<NodeA>> would also work, if PHPStan/Psalm treated generic class-strings and non-generic ones differently (e.g. generic class-string is not valid in new), and if class-string<Root<NodeA>> was not considered a sub type of class-string. This would make class-string more confusing.

Using string values like @return 'Root<NodeA>'|'Root<NodeB>' would work for me, though.

[romm]

Hi @arnaud-lb, I know it's a very old issue, but I'm trying to clean-up old issues and I think this one deserved a working solution in the library.

Any chance you take a look at #772 and see if that can solve the issue you were having?

[arnaud-lb]

Hi @romm,

Thank you! I can not test on the original code anymore, but I was able to test with the gist I've linked earlier. From what I can see, #772 fixes a part of the issue by allowing @return 'Root<NodeA>'|'Root<NodeB>', however Valinor still refuses a generic interface.

This works: https://gist.github.com/arnaud-lb/679e99fb7f81e7b1a75dcac85430cbb3

This does no work: https://gist.github.com/arnaud-lb/d508ba2b0ad74aed1fc0d2a3e5ace8a0

PHP Fatal error:  Uncaught CuyZ\Valinor\Mapper\Exception\InvalidMappingTypeSignature: Could not parse the type `RootInterface` that should be mapped: there are 1 missing generics for `RootInterface<?>`. in vendor/cuyz/valinor/src/Mapper/TypeTreeMapper.php:28
Stack trace:
#0 test.php(47): CuyZ\Valinor\Mapper\TypeTreeMapper->map()
#1 {main}
  thrown in vendor/cuyz/valinor/src/Mapper/TypeTreeMapper.php on line 28