[DependencyInjection] Add autowiring capabilities #15613

dunglas · 2015-08-25T12:53:02Z

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Tests pass?	yes
Fixed tickets	n/a
License	MIT
Doc PR	not yet

This PR adds autowiring capabilities to the Dependency Injection component. It eases service registration by letting the component guessing dependencies to inject and even (under certain conditions) registering them using typehints of the constructor parameters.

The following usages are supported:

Automatic dependency registration

class Foo
{
}

class Bar
{
    public function __construct(Foo $f)
    {
    }
}

services:
    bar:
        class: Bar
        autowire: true

It will register Foo as a private service (autowired.foo) and injects it as the first argument of the bar constructor.
This method works only for typehints corresponding to instantiable classes (interfaces and abstract classes are not supported).

Autocompletion of definition arguments

interface A
{
}

interface B extends A
{
}

class Foo implements B
{
}

class Bar
{
}

class Baz extends Bar
{
}

class LesTilleuls
{
    public function __construct(A $a, Bar $bar)
    {
    }
}

services:
    foo:
        class: Foo

    baz:
        class: Baz

    les_tilleuls:
        class: LesTilleuls
        autowire: true

The autowiring system will find types of all services and completes constructor arguments of the les_tilleuls service definition using typehints.

It works only if there is one service registered for a given type (if there are several services available for the same type and no explicit type definition, a RuntimeException is thrown).

Explicit type definition

interface A
{
}

class A1 implements A
{
}

class A2 implements A
{
}

class B
{
     public function __construct(A $a)
     {
     }
}

services:
    a1:
        class: A1
        types: [ A ]

    a2:
        class: A2

    # Will be autowired with A1
    class b:
        class: B
        autowire: true

    # Not autowired
    class another_b:
        class: B
        arguments: [ @a2 ]
        autowire: true

When a service is explicitly associated with a type, it is always used to fill a definition depending of this type, even if several services have this type. If several services are associated with the same type, the last definition takes the priority.

Of course explicit definitions are still supported.
YAML, XML and PHP loaders have been updated to supports the new type parameter.

hhamon · 2015-08-25T12:59:28Z

src/Symfony/Component/DependencyInjection/Compiler/AutowiringPass.php

+ */
+class AutowiringPass implements CompilerPassInterface
+{
+    private $container;


why storing the container? Can't you just pass it explicitly to your private methods?

As this class already has a lot of states, it's just to avoid passing the container as argument of other methods.
I can rewrite this class without states but not sure it has any interest (and other passes also have states).

dunglas · 2015-08-26T08:16:53Z

There is a problem with invalid definitions intended to be removed later by removing passes. It's why tests of FrameworkBundle fail: annotations.cached_reader is not complete, AutowiringPass tries to complete it but fails an throws a RuntimeException.

A solution is to explicitly mark with a tag definitions to autowire or to not autowire.

IMO marking definitions to not autowire is more user friendly (it's an edge case) but it would be a (small) BC break. Bundles using black magic similar to what is done in FrameworkBundle will require to be updated.
Alternatively, definitions to autowire can be marked explicitly. It will be BC but less user friendly (the final goal is to reduce the amount of config to write).

Another possibility is to silently restore the initial definition if the autowiring fail. I think it would be less user friendly (no way to detect if the autowiring failed or not) but this approach is both BC and doesn't require to add specific tags.

I suggest to require to mark services to autowire in 2.8 (BC) and to change the behavior in 3.0 by requiring to mark the service to not autowire (as it is just a tag, it's still possible to have a definition working for both 2.8 and 3.0 by adding the not-autowired tag, this tag will simply be ignored in 2.8).

What do you think about that @stof @fabpot?

javiereguiluz · 2015-08-26T08:16:55Z

src/Symfony/Component/DependencyInjection/Tests/Compiler/AutowiringPassTest.php

+    {
+        $container = new ContainerBuilder();
+
+        $container->register('coop_tilleuls', __NAMESPACE__.'\LesTilleuls');


I hope you understand this comment right: I think that using the real name of your company in the tests is "not right". I'd prefer to keep using synthetic names for everything (acme, foo, bar, etc.)

linaori · 2015-08-27T06:48:35Z

This introduces a lot of magic and complexity for developers.

What if someone introduces another service with the same class, will everything (silently) break?
How will people notice errors when they use $this->get() in an action that's hardly ever called?
What kind of impact will this have on stability? People will start relying on "magic"

I can certainly see benefits for this such as request_stack, session*, security.*, router etc. But most cases I encounter, that's just one of several dependencies injected. Does it mean it will still have to be verbose?

If I understand it right, it will guess the dependency based on the class name. What if someone typehints an interface and decorates the dependency, will it create a reference to the decorated or decorator? Both are in the DIC and the public: false won't prevent injection.

I personally like verbosity. Writing down a service definition is a minute of work, but you won't have to really bother with it anymore. I do a lot of refactoring because development never stops. One thing I always do, is check if a service is used somewhere, which works better than checking against an interface. This will be virtually impossible because you lost control of where it's injected.

It's a nice feature that certainly has potential, but at the moment I see far more downsides than benefits. Maybe you can open my eyes!

dunglas · 2015-08-27T09:02:16Z

This introduces a lot of magic and complexity for developers.

It maybe introduces some "magic" but no complexity at all: this feature is fully optional and developers that don't like it can define their services as they have always done. IMO this feature simplify the use of the DI in the context of RAD (I'll get back to it).

What if someone introduces another service with the same class, will everything (silently) break?

I'm not sure to understand exactly what you mean:

class A {}
class B { function __construct(A $a) {} }
class C { function __construct(A $a) {} }

services:
   b:
       class: B
    c:
        class: C

It works without breaking anything.

abstract class A {}
class B extends A {}
class C extends A {}
class Foo { function __construct(A $a) {} }

services:
    foo:
        class: Foo

This is not possible to guess what to inject so it throws an explicit RuntimeException with the following message: Unable to autowire argument of type "A" for the service "foo".

There is two solutions here:

Using the classical (explicit) definition:

   services:
       b:
           class: B
       c:
           class: C
       foo:
           class: Foo
           arguments: [ @b ]

It's the same old config available from the beginning of Symfony.

An alternative is to define a default service to use for a given type (it will always be injected for that type unless an explicit service is passed in argument).

   services:
       b:
           class: B
           types: [ A ]
       c:
           class: C
       foo:
           class: Foo

This is useful when a service must be injected 90% of the time (it can be autowired) but in some definitions another implementation (decorator, debug or performance optimized version...) is needed.

How will people notice errors when they use $this->get() in an action that's hardly ever called?

Errors are raised at compile time (when $container->compile() is called: when doing a cache:clear in the full stack framework). It doesn't matter if the service is used or not.

What kind of impact will this have on stability? People will start relying on "magic"

Basically, the autowiring system just complete service definitions during a compiler pass. After that, it's the same old system that is used. The impact in term of stability is minor and if an error occurs, it will occur during the container compilation and will be handled by the developper very early in the development process.

I can certainly see benefits for this such as request_stack, session_, security._, router etc. But most cases I encounter, that's just one of several dependencies injected. Does it mean it will still have to be verbose?

I'm not sure I understand what you mean. Can you give an example?

If I understand it right, it will guess the dependency based on the class name. What if someone typehints an interface and decorates the dependency, will it create a reference to the decorated or decorator? Both are in the DIC and the public: false won't prevent injection.

The system is smarter than just using the class name. I've posted some examples using interfaces in the PR description.

To sum up:

If the definition is explicit (the actual way of registering service): nothing change, the current behavior applies
If they are missing arguments in a service definition:
- the system guesses all types of existing services definition (including interface implementations and inherited types) and tries to fill the incomplete definition automatically with a service of the type required by the typehint (as explained previously, it works only if there is one implementation or if a types has been set explicitly)
- If the typehint is a class name, no existing service of this type exists and this class name is instantiable: a private service is automatically registered and injected

I personally like verbosity. [...]

As previously said, the current behavior is not deprecated and can still be used. It's a layer on top of what is already existing that is fully optional.
This is a high level RAD feature. It should be compared to existing similar features such as configuration annotations and conventions based features (controllers and entities auto-detection when they are in a specific folder...).

Autowiring is a feature present in many DI systems (Spring, Laravel DI, PHP DI...). It's main advantage is to drastically reduce the configuration code needed to use DI (in many case, it allows to write no configuration all) and ease refactoring (when you add or remove a dependency to a class, you don't need to dive into all your service definitions to update the configuration, this is automatic).
It helps when using controller as a service with the convenience of the DIC but without requiring it and reduce the amount of configuration code needed when using alternative patterns such as ADR (it's originally in the intent of implementing a generic and convenient ADR implementation for Symfony that I wrote this code).

Personally, I always avoid using such things (autowiring, configuration annotations, even the DIC) for a large scale enterprise where maintainability matter and I know the code base will be huge. I prefer to be explicit and have chunks of code as independent and framework agnostic as possible.

However I also often work on smaller projects where the goals are different: development as speed as possible with less line of code as possible. For such projects (prototypes, some micro services, weekend projects and some non-commercial projects...) I rely as much as possible on every feature that ease the devolvement and the refactoring including conventions, annotations and all that kind of "magic" (when you know how it works, there is not much magic). Autowiring is for that kind of projects.

I hope it answers to your concerns. The summary:

It is fully optional and built on top of existing features
If should not break anything
It's especially intended for RAD (at least I designed it for that use)

javiereguiluz · 2015-08-27T09:28:15Z

@dunglas I agree 100% with the idea of this PR and I thank you for working on it.

Regarding the "magic fear" expressed by @iltar, I have one question:

Nowadays, when I don't configure a service properly, I see a nice error. Then I add some (verbose) config and that's it.
With this autowire feature, if I don't configure that same service, everything will still work and I won't see the previous error message. The "magic" has silently fixed my wrong config.

My question is: for those people who don't want this feature, how can they know that their configuration is wrong but it was "silently" fixed by the autowiring feature?

gnugat · 2015-08-27T09:30:54Z

@ITAR: autowiring only solves the simplest use cases, for the rest you can still configure explicitly the services.
Also this isn't a brand new feature: laravel (framework), PHP DI (library) and JmsDiExtraBubdle (sf bundle) provide it. My point is that it's already proven itself.

Now Symfony already has some history with auto wiring, but it didn't make it in the end: https://groups.google.com/forum/m/#!topic/symfony-devs/77GxWI8F8lU

wouterj · 2015-08-27T09:31:32Z

As this is mainly based on typehinting against implementations, which is a big no-go for services. I'm afraid this is actually promoting to typehint against an implementation instead of the interface, resulting in almost useless services...

dunglas · 2015-08-27T09:43:17Z

@javiereguiluz It's similar to what I've suggested here #15613 (comment). What to you think of the tag based approach?
@wouterj it works fine with interfaces too (and I mostly use it that way): see "Autocompletion of definition arguments" and "Explicit type definition " examples in the PR description.

linaori · 2015-08-27T09:46:16Z

The qiestion @javiereguiluz is asking, is one of the things that concerns me as well, hence:

What if someone introduces another service with the same class, will everything (silently) break?

class A {}

class FooBar {
    public function __construct(A $a);
}

services:
    a.foo:
        class: A

    a.bar:
        class: A

    foobar:
        class: FooBar

I can certainly see benefits for this such as request_stack, session, security., router etc. But most cases I encounter, that's just one of several dependencies injected. Does it mean it will still have to be verbose?

I'm not sure I understand what you mean. Can you give an example?

class A {
    public function __construct(RequestStack $rs, AB $a, BB $b);
}

services:
    ab.henk:
        class: AB
    ab.jan:
        class: AB
    bb:
        class: BB
    a.foo:
        class A:
        arguments:
            # how do I wire the request stack in here?
            - `@ab.henk` # cannot be autowired, 2x AB defined
            # BB is autowired?

I agree with what WouterJ said. This is also what happens with the JMSDiExtraBundle, which is the exact reason I avoid it. Maybe this could be a RapidDevelopmentBundle though? It seems like very pluggable functionality, similar to the lazy: option.

Nicofuma · 2015-08-27T09:54:08Z

I have only one question, similar to the one raised by @javiereguiluz : for some reasons, in a project I don't want to use auto-wiring at all, I want to completely defines all my services. But, sometimes, I failed and I forget some things. How can I completely disable auto-wiring in order to see my mistakes?

dunglas · 2015-08-27T09:57:01Z

@iltar it's exactly the example I've given:

class A {}

class FooBar {
    public function __construct(A $a);
}

services:
    a.foo:
        class: A

    a.bar:
        class: A

    foobar:
        class: FooBar

You'll get the following error (at compile time): RuntimeException. Unable to autowire argument of type "A" for the service "foobar".

For such cases:

if you want the autowiring you must define a default implementation for the given type (using the new types keyword)
or fallback to the classical explicit definition

For your second example, you can of course fallback to the explicit definition, but you can also use the following syntax (your snippet edited):

services:
    ab.henk:
        class: AB
    ab.jan:
        class: AB
    bb:
        class: BB
    a.foo:
        class A:
        arguments:
            - '' # As it is empty, the autowiring will be triggered for request stack. Looks better in XML (I prefer XML...): <argument/>
            - `@ab.henk` # cannot be autowired, 2x AB defined => explicit definition is OK
            # Yes, BB will be autowired

dunglas · 2015-08-27T10:02:14Z

@Nicofuma in #15613 (comment) I've proposed the following:

in 2.8, explicitly mark definitions to autowire with a autowire tag (BC) and do nothing if the tag is not present
in 3.0 (and can safely be added to 2.8 too) mark definitions to not autowire with the no-autowire tag

It's not implemented yet and feedback will be very appreciated :)
We can also add a flag to the container to globally disable the autowiring (but the tag system will still be necessary).

linaori · 2015-08-27T10:02:21Z

@dunglas I start liking XML as well (for opensource). But the recommendation and easy going way for your app is still yml. This means you still have to register the argument (or omit it).

services:
    ab.henk:
        class: AB
    ab.jan:
        class: AB
    bb:
        class: BB
    a.foo:
        class A:
        arguments:
            - '' # As it is empty, the autowiring will be triggered for request stack. Looks better in XML (I prefer XML...): <argument/>
            - `@ab.henk` # cannot be autowired, 2x AB defined => explicit definition is OK
            # Yes, BB will be autowired

What if I would want to add a 4th argument or my extension/compiler pass calls addArgument(), a 4th argument would be required. This effectively means that the definition has 3 arguments and the autowiring tries to resolve the 4th.

The biggest problem is that if this is enabled, it will lead to weird constructions in service definitions and headaches for the developers. It doesn't feel intuitive at all to me. I can already see questions coming to #symfony.

wouterj · 2015-08-27T10:03:59Z

@dunglas that error is exactly what @javiereguiluz and @iltar mean.

assume you had this config:

services:
    a.foo:
        class: A

    foobar:
        class: FooBar

Everything works nice! Now, one of your coworkers (or yourself) add a new service somewhere in a config file:

services:
    a.bar: { class: A }

Whoa, foobar service throws an exception now and the application breaks! (only because I added an, apperently totally unrelated, a.bar service).

Nicofuma · 2015-08-27T10:04:59Z

The tag could solve some issue. But I think we still need a global flag (I wouldn't have to add a tag to each of my xxx services)

dunglas · 2015-10-02T06:44:24Z

Rebased, it looks OK now.

fabpot · 2015-10-02T07:10:42Z

src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php

+
+        if (isset($service['autowiring_types'])) {
+            if (!is_array($service['autowiring_types'])) {
+                throw new InvalidArgumentException(sprintf('Parameter "autowiring_types" must be an array for service "%s" in %s. Check your YAML syntax.', $id, $file));


As having only one type is quite common, I would allow a single string to be passed here.

stof · 2015-10-02T09:54:33Z

src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

+
+        $arguments = $definition->getArguments();
+        foreach ($constructor->getParameters() as $index => $parameter) {
+            if (!$typeHint = $parameter->getClass()) {


the case of non-existent classes still need to be handled

dunglas · 2015-10-02T10:09:11Z

@stof should be OK now

aitboudad · 2015-10-02T11:21:58Z

src/Symfony/Component/DependencyInjection/Compiler/AutowirePass.php

+            throw new RuntimeException(sprintf('Unable to autowire argument of type "%s" for the service "%s".', $typeHint->name, $id));
+        }
+
+        $argumentId = sprintf('autowired.%s', $typeHint->name);


This will register a service with an id like 'autowired.AppBundle\Foo' which is not consistent with Service Naming Conventions

You're right but it is only intended for internal services name (they should not be used directly) so it should not be a big deal.

From a debug point of view, autowired.AppBundle\Foo is more explicit than autowired.app_bundle_foo or autowired.app_bundle.foo.

IMO it's better to keep it as is but I can change it if everyone think it's better to follow conventions here.

fabpot · 2015-10-03T06:39:23Z

Thank you @dunglas.

This PR was squashed before being merged into the 2.8 branch (closes #6032). Discussion ---------- [DependencyInjection] Autowiring doc | Q | A | ------------- | --- | Doc fix? | no | New docs? | yes symfony/symfony#15613 | Applies to | 2.8, 3.0 | Fixed tickets | n/a Commits ------- 995bd4f [DependencyInjection] Autowiring doc

carsonbot added the Status: Needs Review label Aug 25, 2015

dunglas added the DependencyInjection label Aug 25, 2015

hhamon reviewed Aug 25, 2015
View reviewed changes

javiereguiluz reviewed Aug 26, 2015
View reviewed changes

dunglas added 10 commits October 1, 2015 16:11

Flag to enable/disable autowiring. no_autowiring tag.

afe009a

Fix some tests

7ab5b08

Fix @meyerbaptiste comments

a7ca3f2

Move the autowiring parameter to a setter

57c494c

Fix SecurityBundle test

e642aa8

Optin. Removed config flag.

d00c7e3

Fix CS

55bb42c

[DependencyInjection] Rename types to autowiring_types

7806487

[DependencyInjection] Add the new 'autowire' syntax.

585616d

Fix tests.

c717c69

dunglas force-pushed the autowiring branch from c2d2ecd to c717c69 Compare October 1, 2015 14:20

fabpot reviewed Oct 2, 2015
View reviewed changes

Fix @fabpot and @stof comments.

3b7f553

stof reviewed Oct 2, 2015
View reviewed changes

Typehint against a non-existing class

936a16a

aitboudad reviewed Oct 2, 2015
View reviewed changes

fabpot closed this in 478ad54 Oct 3, 2015

dunglas deleted the autowiring branch October 3, 2015 09:27

fabpot mentioned this pull request Nov 16, 2015

Release v2.8.0-BETA1 #16550

Merged

dunglas mentioned this pull request Dec 16, 2015

[DependencyInjection] Autowiring doc symfony/symfony-docs#6032

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DependencyInjection] Add autowiring capabilities #15613

[DependencyInjection] Add autowiring capabilities #15613

dunglas commented Aug 25, 2015

hhamon Aug 25, 2015

dunglas Aug 25, 2015

dunglas commented Aug 26, 2015

javiereguiluz Aug 26, 2015

linaori commented Aug 27, 2015

dunglas commented Aug 27, 2015

javiereguiluz commented Aug 27, 2015

gnugat commented Aug 27, 2015

wouterj commented Aug 27, 2015

dunglas commented Aug 27, 2015

linaori commented Aug 27, 2015

Nicofuma commented Aug 27, 2015

dunglas commented Aug 27, 2015

dunglas commented Aug 27, 2015

linaori commented Aug 27, 2015

wouterj commented Aug 27, 2015

Nicofuma commented Aug 27, 2015

dunglas commented Oct 2, 2015

fabpot Oct 2, 2015

stof Oct 2, 2015

dunglas commented Oct 2, 2015

aitboudad Oct 2, 2015

dunglas Oct 2, 2015

fabpot commented Oct 3, 2015

[DependencyInjection] Add autowiring capabilities #15613

[DependencyInjection] Add autowiring capabilities #15613

Conversation

dunglas commented Aug 25, 2015

Automatic dependency registration

Autocompletion of definition arguments

Explicit type definition

hhamon Aug 25, 2015

Choose a reason for hiding this comment

dunglas Aug 25, 2015

Choose a reason for hiding this comment

dunglas commented Aug 26, 2015

javiereguiluz Aug 26, 2015

Choose a reason for hiding this comment

linaori commented Aug 27, 2015

dunglas commented Aug 27, 2015

javiereguiluz commented Aug 27, 2015

gnugat commented Aug 27, 2015

wouterj commented Aug 27, 2015

dunglas commented Aug 27, 2015

linaori commented Aug 27, 2015

Nicofuma commented Aug 27, 2015

dunglas commented Aug 27, 2015

dunglas commented Aug 27, 2015

linaori commented Aug 27, 2015

wouterj commented Aug 27, 2015

Nicofuma commented Aug 27, 2015

dunglas commented Oct 2, 2015

fabpot Oct 2, 2015

Choose a reason for hiding this comment

stof Oct 2, 2015

Choose a reason for hiding this comment

dunglas commented Oct 2, 2015

aitboudad Oct 2, 2015

Choose a reason for hiding this comment

dunglas Oct 2, 2015

Choose a reason for hiding this comment

fabpot commented Oct 3, 2015