Support for template of template

[greg0ire]

doctrine/persistence defines a generic ClassMetadata interface, and its generic ClassMetadataFactory. Templating ClassMetadataFactory::getMetadataFor() does not seem possible:

https://psalm.dev/r/0ff07df695

I would expect no errors.

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/0ff07df695

<?php

/** @template TCM of object */
interface ClassMetadata {};

/**
 * @template T of ClassMetadata
 */
interface ClassMetadataFactory
{
    /**
     * @template T2 of object
     * @psalm-param class-string<T2> $className
     * @psalm-return T<T2>
     */
    public function getMetadataFor($className);
}

Psalm output (using commit 836587e):

ERROR: UndefinedDocblockClass - 14:22 - Docblock-defined class, interface or enum named T does not exist

[orklah]

Not sure I know enough of ClassMetadata structure, but isn't https://psalm.dev/r/4b0ca83540 good enough?

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/4b0ca83540

<?php

/** @template TCM of object */
interface ClassMetadata {};


interface ClassMetadataFactory
{
    /**
     * @template T2 of object
     * @psalm-param class-string<T2> $className
     * @psalm-return ClassMetadata<T2>
     */
    public function getMetadataFor($className);
}

Psalm output (using commit 836587e):

No issues!

[greg0ire]

Sadly no. My end goal is to let Psalm understand that the ClassMetadataFactory::getMetadataFor returns ORM ClassMetadata, which has a lot of fields not defined in the ClassMetadata defined in Persistence.

If I have an entity called user I want to get an ORM\ClassMetadata<User>, if I have a document called User, I want to get an ODM\ClassMetadata<User>

[orklah]

Well, given that ORM\ClassMetadata is a child of ODM\ClassMetadata, I'd say the contract allows for ORM\ClassMetadata to be returned. I'd say it's up to the caller of the contract to check what kind of ClassMetadata it received.

Do you mind explaining the difference between an entity and a document?

[greg0ire]

Well, given that ORM\ClassMetadata is a child of ODM\ClassMetadata

It isn't the ORM and the ODM are 2 different projects, both depend persistence. The former users entities, the later documents.

From https://www.doctrine-project.org/projects/doctrine-phpcr-odm/en/latest/reference/introduction.html

As PHPCR allows NoSQL like data storage, we speak of Documents rather than Entities to stress the fact that there need not be a rigid database model.

I'd say it's up to the caller of the contract to check what kind of ClassMetadata it received.

That's currently what happens but as long as in the ORM, you expect to be dealing with ORM ClassMetadata. It would be great not to have to do all these assertions.

[orklah]

Sorry, I confused ODM for persistence. I understand better. How does persistence knows which one it's dealing with? Ideally, when creating the ClassMetadata, we would give it the type correct type as a template

[greg0ire]

My plan would be to use @template-extends AbstractClassMetadataFactory<ORMClassMetadata> in the ORM ClassMetadataFactory. I tried it when working on doctrine/orm#8699 this evening, but got many more errors I did not understand. My goal was to get rid of the three @method annotations show here: https://github.com/doctrine/orm/blob/2.8.x/lib/Doctrine/ORM/Mapping/ClassMetadataFactory.php#L58 , but that will be a story for tomorrow 😴

[orklah]

Oh right, so the end goal would have been something like https://psalm.dev/r/725182085a, but then the error about the template not recognized pops up, right?

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/725182085a

<?php
namespace Persistence{
    /** @template TCM of object */
    interface ClassMetadata {};

    /**
     * @template Type of ClassMetadata
     */
    interface ClassMetadataFactory
    {
        /**
         * @template T2 of object
         * @psalm-param class-string<T2> $className
         * @psalm-return Type<T2>
         */
        public function getMetadataFor($className);
    }
}

namespace ORM{
    /** 
     * @template TCM of object
     * @psalm-extends\Persistence\ClassMetadata<TCM>
     */
    interface ClassMetadata extends \Persistence\ClassMetadata{};
}

namespace ODM{
    /** 
     * @template TCM of object
     * @psalm-extends\Persistence\ClassMetadata<TCM>
     */
    interface ClassMetadata extends \Persistence\ClassMetadata{};
}

Psalm output (using commit 836587e):

ERROR: UndefinedDocblockClass - 14:26 - Docblock-defined class, interface or enum named Type does not exist

[greg0ire]

Yes exactly!

[weirdan]

Doesn't ORM have its own factory interface? E.g. https://psalm.dev/r/61c14d2a53

[psalm-github-bot]

I found these snippets:

https://psalm.dev/r/61c14d2a53

<?php
namespace Persistence {
    /** @template TCM of object */
    interface ClassMetadata {}
    
    interface AbstractClassMetadataFactory {
        /**
         * @template TT of object
         * @param class-string<TT> $class
         * @return ClassMetadata<TT>
         */
        public function getMetadataFor(string $class): ClassMetadata;
    }
}

namespace ORM {
    use Persistence\ClassMetadata as PersistenceMetadata;
    use Persistence\AbstractClassMetadataFactory as PersistenceMetadataFactory;
    
    /** 
     * @template TCM of object
     * @extends PersistenceMetadata<TCM>
     */
    interface ClassMetadata extends PersistenceMetadata {}
    
    interface ClassMetadataFactory extends PersistenceMetadataFactory {
        /**
         * @template TT of object
         * @param class-string<TT> $class
         * @return ClassMetadata<TT>
         */
        public function getMetadataFor(string $class): ClassMetadata;
    }
}

namespace { 
    function f(ORM\ClassMetadataFactory $factory): ORM\ClassMetadata {
        $r = $factory->getMetadataFor(Exception::class);
        /** @psalm-trace $r */;
        return $r;
    }
}

Psalm output (using commit d82f028):

INFO: Trace - 39:31 - $r: ORM\ClassMetadata<Exception>

[greg0ire]

@weirdan it's not an interface, it's a class, and I think it would be more elegant to have @template-extends PersistenceMetadataFactory<ClassMetadata> here, wouldn't it?

[weirdan]

it's not an interface, it's a class,

That seems irrelevant, it'll work the same way with a class.

I think it would be more elegant to have @template-extends PersistenceMetadataFactory<ClassMetadata>

Why though? Is it to avoid having to specify a docblock for ORM\ClassMetadataFactory::getMetadataFor()?

[greg0ire]

That seems irrelevant, it'll work the same way with a class.

It is irrelevant, but I'd thought I would let you know in case you were thinking of some other interface in ORM.

Why though? Is it to avoid having to specify a docblock for ORM\ClassMetadataFactory::getMetadataFor()?

Yes. Note that this method is defined in the abstract class, in persistence, and not overridden in the ORM. I manage to circumvent that by using @method, but then you have #5786 that prevents me from continuing to do so.

[muglug]

That’s unsafe in a number of ways, I think. It’s a no from me.

[muglug]

Unless this works in Hack, which I doubt.

[muglug]

From Hack:

Naming[2047] T is a type parameter. Type parameters cannot take type arguments (e.g. T<int> isn't allowed) [1]

static.hack:5:77
    3 | interface ClassMetadataFactory<T as ClassMetadata>
    4 | {
[1] 5 |     public function getMetadataFor<T2 as object>(classname<T2> $className): T<T2>;
    6 | }

[greg0ire]

Ok, fair enough.