cc @matej21

👍

I wonder how this commit affects large containers (could it try @fprochazka, @ondrejmirtes, @pilec, …)? In my projects containers are the same, only with minor difference in meta data.

I use setting autowired to off to exclude services from being accidentally injected somewhere where I do not want them. For example I have all instances of Monolog\Logger set to autowired: off because when I ask for a logger in a constructor, I want the DI container to fail and require specifying which concrete logger I want to inject into the service.

I am afraid that this change will lead to injecting of random unwanted services. Or is my scenario somehow solvable after this change?

@ondrejmirtes this can be solved in two ways:

1. option autowired will have three states: off, on and preferred (the default one), so you can disable autowiring in the same way as before
2. option autowired will have two states: on and preferred (the default one?) and you can disable autowiring via excluded classes and they will be configurable via config.neon.

I have absolutely no idea which approach is better.

The 1) is close to current approach, but three states will make it very difficult for programmers to figure out which state they should use.

The 2) is really different, because there is not autowire: off, but it fits perfectly to your scenario. You can disable autowiring for i.e ILogger, for all services with a single line, so it is less error prone.

If people use autowired: no in sense not preferred, the 2) is fine. If they use it in sense „prevent accidental injection“, the 2) is maybe better solution too.

(Now I am not talking about back compatibility, it will remain, just about best practices.)

I've tried it and it doesn't exactly work :) But I don't have much time to dig in.

@dg: Just idea: What abou setAutowired($priority). FALSE = not autowire, TRUE=1=autowired lowest priority, 2+=autowired with ascending priority. It would be backward compatible but sloves what I think you trying to solve.

@MartinMystikJonas I think it is the same as first option here #84 (comment)

@dg Basically yes. But my solution can solve this for deeper inheritance hierarchies when you need to set prefferences when multiple services in inheritance hierarchies matches searched type.

For example:
Service A - IStorage, FileStorage ->setAutowire(3)
Service B - IStorage, FileStorage, MyFileStorage ->setAutowire(2)
Service C - IStorage, DevNullStorage ->setAutowire(1)

getByType('IStorage') -> Service A
getByType('FileStorage') -> Service A
getByType('MyFileStorage') -> Service B
getByType('DevNullStorage') -> Service C

Service A with setAutowired(2) and B & C with setAutowired(1) will have the same result, or not?

You are right. My solution is only useful in even more complex situations and therefore probably not needed.

👍 Just found out I need this.

One thing that I don't like in Nette/DI is overriding of autowired services by another extension. For example I've an extension which provides alternative implementation of Nette/Application/Application. Now what the extension can do to override the default implementation? In my opinion it's wrong for the custom extension to change the original service in any way (changing class/factory, turning of autowiring etc.) - but it currently can't be solved without it.

This PR doesn't solve this of course. I'm mentioning it because it could solve it (by using priorities or something). If you think it's off-topic, just ignore this comment.

👍

👍

Now option autowired can contain name or names of classes/interfaces. It make services autowired only for specified types and preferably.

Because you choose the instance of approach instead of === approach, I don't understand why would you ever need to declare multiple autowire types, i.e. why support bool|array|string and not just bool|string.

But maybe I'm missing something. I had a bit of trouble reading the code.

I tried to implement a completely different solution. Option autowired can be true or false (the same meaning), or class name or list of class names.

In this case, autowiring is limited only to these classes. For example, when class implements three interfaces and I want to autowired it only for one or two of them, I will enumerate these interfaces. So when argument typehint matches of one of enumerated interfaces, service will be used for autowiring.

Therefore when I will specify it's own class name (or use autowired: self), service will be autowired only when argument typehint === specified interface.

At the same time, in addition to limiting the class list, the service becomes for that type preferred. So when there will be multiple (normally) autowired services that meet the specified class/interface, this service with specific option will win.

I think I finally understand it now, the getByType logic can be rewritten in pseudocode as

coalesce(
    definitions.filter(_.class instanceof type).filter(type instance of _.autowired),
    definitions.filter(_.class instanceof type).filter(_.autowired === true)
)

Making the following 8 tests cover the core logic.

test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired('Bar');

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same(NULL, $builder->getByType('IBar'));
    Assert::same(NULL, $builder->getByType('Foo'));
    Assert::same(NULL, $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired('IBar');

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same('bar', $builder->getByType('IBar'));
    Assert::same(NULL, $builder->getByType('Foo'));
    Assert::same(NULL, $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired('Foo');

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same(NULL, $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same(NULL, $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired('IFoo');

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same(NULL, $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same('bar', $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired(['IFoo', 'IBar']);

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same('bar', $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same('bar', $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired(['Foo', 'Bar']);

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same(NULL, $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same(NULL, $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired(['Foo', 'IBar']);

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same('bar', $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same(NULL, $builder->getByType('IFoo'));
});


test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired(['IFoo', 'Bar']);

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same(NULL, $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same('bar', $builder->getByType('IFoo'));
});

Having a class hierarchy graph (including interfaces of course), the setClass() marks one node (green) in graph and setAutowired() marks the second node (orange). The resulting autowired types are all nodes along the path from the setClass() node to the setAutowired() node.

test(function () {
    $builder = new DI\ContainerBuilder;
    $builder->addDefinition('bar')
        ->setClass('Bar')
        ->setAutowired('IFoo');

    Assert::same('bar', $builder->getByType('Bar'));
    Assert::same(NULL, $builder->getByType('IBar'));
    Assert::same('bar', $builder->getByType('Foo'));
    Assert::same('bar', $builder->getByType('IFoo'));
});

😍

@JanTvrdik I'll use your more explicit tests