PSR-11 implementation and auto-wiring

[moufmouf]

Hey guys!

PSR-11 editor here :)

It was just brought to my attention by @harikt here (laravel/framework#21220) that the PSR-11 implementation of Laravel container in 5.5 requires that the services are explicitly bound in the container.

It is clearly documented here: https://laravel.com/docs/5.5/container#psr-11

It's a bit of a shame because it means that we cannot use the PSR-11 side of Laravel container for auto-wiring. And Laravel container is sooo good at auto-wiring.... :)

Looking back at why this decision was taken, I ran into this PR: laravel/framework#19822

@thecrypticace says:

Does the spec explicitly prevent has() from returning true for autowired objects? Ex: I didn't bind an object to the container but it can still be built by it.

So to be clear: PSR-11 gives the container a great deal of freedom regarding what you consider to be part of the container or not. The current implementation is totally legit, but has returning true for autowired object is also legit if you decide that any instantiable class is part of the container.

We have had a lengthy discussion on this topic here: container-interop/container-interop#37 (comment)
(note: container-interop/container-interop was the draft of PSR-11)

IMHO, it would make a great deal of sense to change the behaviour of the Laravel container so that the get method can also return auto-wired objects. The current implementation is not wrong, but it is definitely limiting the power of what can be done with the container and there is no good reason to do that.

[RemiCollin]

Thanks for your in-depth explanation. I ran into this issue when I wanted to abstract Illuminate\Container to a PSR-11 implementation, and I ended up with this :

if($container instanceof \Illuminate\Container\Container) {
    return $container->make($abstract);
}

return $container->get($abstract);

Which kind of defeat the purpose...

I think it would be awesome if the get() method should just redirect to the container's make() method if no binding have been explicitly declared.

[harikt]

I understand that there can be people who will complain that the service is not registered in the container. And it should be delegated to another container.

So here is what I propose.

The container can turn on the feature for autowiring.

$container->autoResolvePsr11();

Now on the container side get and has can look if autoResolve is turned on and try to see if it is instantiable.

    public function autoResolvePsr11()
    {
        $this->autoResolvePsr11 = true;
    }

    public function get($id)
    {
        if ($this->has($id)) {
            return $this->resolve($id);
        }

        throw new EntryNotFoundException();
    }

    public function has($id)
    {
        if ($this->bound($id)) {
             return true;
        }

        if ($this->autoResolvePsr11) {
            if (in_array($id, $this->cachedKeys)) {
                return true;
            }
            if ($this->isInstantiable($id)) {
                $this->cachedKeys[] = $id;
                return true;
            }
        }

        return false;
    }

    private function isInstantiable($id)
    {
        if (class_exists($id)) {
            return true;
        }

        try {
            $reflectionClass = new \ReflectionClass($id);
            return $reflectionClass->isInstantiable();
        } catch (\ReflectionException $e) {
            return false;
        }
    }

Some parts of the code is taken from an implementation over : https://github.com/dms-org/web.laravel/blob/582d970393656a011ffcee4f6716ae79c5e57813/src/Ioc/LaravelIocContainer.php#L157-L199

Thank you.

UPDATE : Code is updated a bit removing certain functions.

[RemiCollin]

@moufmouf : what other autowiring capable PSR-11 containers do in that case ?

[moufmouf]

Good question, I'm not a big autowiring fan so I haven't tracked that closely.

Let's ask @mnapoli. His PHP-DI package is PSR-11 compliant and does auto-wiring (+ he is PSR-11 co-editor so he knows what he does :) )

[mnapoli]

Hi, in PHP-DI has() will return true if the entry is defined explicitly or it's autowireable, i.e.:

• it's a valid class name
• it's instantiable (i.e. not an abstract class/interface)

See the code here: https://github.com/PHP-DI/PHP-DI/blob/master/src/Definition/ObjectDefinition.php#L222

That means that get() can still fail afterwards, for example if a constructor parameter cannot be autowired, but that's OK: that means there's an explicit error message as to why the class cannot be instantiated (which is better than an error saying the entry doesn't exist).

[deleugpn]

Sorry for my interpretation of the first implementation. I didn't like it either, but I thought it was what the PSR-11 implied.

[RemiCollin]

Well, I guess we gonna have to live with it as PR was rejected.. :(

[brayniverse]

Not necessarily. I think it's worth persisting with this and hopefully we can all agree on an implementation.

[deleugpn]

I just proposed a different path for auto-wire to try and make amends for my first interpretation. laravel/framework#21335

[deleugpn]

I proposed another solution on laravel/framework#25870 if anybody want to chime in.