Templated container

[ro0NL]

Hi,

What do you think about templating the container interface?

Roughly https://psalm.dev/r/41be1515c2, but perhaps using vendor specific prefixes for param/return :)

[mnapoli]

Hi @ro0NL!

ContainerInterface<string, string>

I don't understand how this is related to PSR-11: that looks like a very specific implementation (i.e. specifying a precise return type to get()), which is out of scope from this PSR?

(when I mean "out of scope", I mean it seems that it's a problem that is not related to PSR-11, I don't see how a "normal" container would always return string, or any other specific type for that matter)

[ro0NL]

service containers always return objects specific service locators may always return known types

i'd like to annotate eg. some general purpose container as such

https://psalm.dev/r/3a595c3f59

[mnapoli]

ContainerInterface<class-string<T>, T> ahh that one speaks more to me, thanks!

Is there any downside to adding these annotations?

I'm not speaking for the FIG, and I don't know how easy this would be to add in PSR-11 (process-wise), but docblock changes that have no impact on users (unless the feature is used, as you've shown) might be doable 🤷

[ro0NL]

best case people annotate their code, use a static analyzer, and have red CI pipelines. i dont think it's a downside :)

in that sense im curious to see how it goes when eg. frameworks pass a untyped container, but the consumer expects a typed one.

im not sure how to deal with phpdoc generally spoken. IMHO real types make room for eg. @return K, but currenly @return mixed isnt a real one, so we may need to preserve it ... thus adding vendor specific ones.

(at this point PSR-5 failed a bit, given we could have a single @psr-param for interoperability =))

[ChristophWurst]

At Nextcloud we typed our container interfaces as well. Here's how we did it

• Add a template to the method not the class
• Allow either a string or a class-name as identifier
• If a class name is used, assume the return type is an instance of that class name, otherwise assume mixed

Example: https://psalm.dev/r/bb36621819

This means you do not have to annotate/document anything when the container is used. Psalm is able to deduce this information from the parameter you pass as $id. This container also works with anything that isn't a class, e.g. some parameters (primitives) that you want to have injected.

With the examples above you always have to document what instance of the container you have. But then you can only use it for one single type. This doesn't seem practical.

Original code tested for months: https://github.com/nextcloud/server/blob/8c82dd37b4cd023e3a5b8ee492800134e4544f4a/lib/public/IContainer.php#L70-L85

[ro0NL]

i dont think ContainerInterface has to return T for class-string<T> :)

[ChristophWurst]

Right, that's possibly different to the PSR specs. For us that is always the case, sorry for generalizing.

[Jean85]

Exactly, my same thought: it's a nice assumption, and I use it myself in my apps, but it's not generalizable; hence, it probably doesn't belong in this PSR, but in the implementing libraries (or apps).

[ro0NL]

ServiceContainerInterface extends ContainerInterface

perhaps does bring some value here :)

[mnapoli]

@ChristophWurst really cool, thank you for sharing! That's not generalizable in the PSR indeed, but definitely interesting and useful in actual projects.

[XedinUnknown]

May I ask, why is this not generalizable in this PSR? Simply adding @template TKey of array-key (or of mixed, if you want non-scalar keys, for example) and @template TValue of mixed and related type annotations to the methods e.g. has(TKey $key): bool and get(TKey $key): TValue would do the trick. As far as I understand. Psalm and PHPStan both support templating this way. Users that don't use supporting tools will be unaffected. The rest will be able to depend on specific containers, like ContainerInterface<string, int> would be a container where string keys map to integers. This does not focus on any specific container use-case, but instead allows typing of all relevant container parameters. Which is often very useful, as Generics are generally great to have.

[greg0ire]

@ro0NL would you please consider reopening this?

Consider the following piece of code:

<?php

declare(strict_types=1);

namespace App\Algolia;

use Algolia\AlgoliaSearch\SearchClient;
use App\Market\Market;
use App\Platform\Platform;
use Psr\Container\ContainerInterface;

final class PsrContainerClientCollection implements ClientCollection
{
    public function __construct(private readonly ContainerInterface $clients)
    {
    }

    public function get(Market $market, Platform $platform): SearchClient
    {
        return $this->clients
            ->get($market->identifier())
            ->get($platform->identifier());
    }

    public function has(Market $market, Platform $platform): bool
    {
        if (!$this->clients->get($market->identifier())->has($platform->identifier())) {
            return false;
        }

        return $this->clients
            ->get($market->identifier())
            ->has($platform->identifier());
    }
}

I think it would be handy to be able to let static analysis know that this is a container of containers of SearchClient instances. In fact, it would have helped me avoid an incident caused by a type error not spotted by Symfony because of https://github.com/symfony/symfony/issues/53946, but I digress :see_no_evil:

Don't look for the error though, this is the code after my fix.

[ro0NL]

ive kinda switched to https://github.com/symfony/symfony/blob/7.1/src/Symfony/Contracts/Service/ServiceProviderInterface.php already :sweat_smile:

[greg0ire]

I was saying that I was going to make a PR then but now I see there are already 2 open PRs about this :sweat_smile: