bug #25282 [DI] Register singly-implemented interfaces when doing PSR-4 discovery (nicolas-grekas)

This PR was merged into the 3.4 branch.

Discussion
----------

[DI] Register singly-implemented interfaces when doing PSR-4 discovery

| Q             | A
| ------------- | ---
| Branch?       | 3.4
| Bug fix?      | yes
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

I'm feeling bad for not having this idea before 3.4.0 went out, therefore submitting on 3.4, despite this being a new feature, technically. On a DX pov still, this is a bugfix :) I'll let you accept the argument or not...

So, when doing PSR-4-based service registration, we keep only classes as services.
This systematically leads to the question: "But what about interfaces, shouldn't we type-hint against abstractions and not classes?!"
And the answer has invariably been: "Well, just create an alias!"
Which means doing configuration manually.

I fear that if we leave things as is, we're going to grow a "generation" of devs that will hijack autowiring and abuse hinting for classes instead of interfaces.

BUT, here is the idea implemented by this PR: let's create an alias for every singly-implemented interface we discover while looking for classes!
Plain local, simple, and obvious, isn't it?

Votes pending :)

Commits
-------

fcd4aa7807 [DI] Register singly-implemented interfaces when doing PSR-4 discovery

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

@@ -56,10 +56,25 @@ abstract class FileLoader extends BaseFileLoader
 
         $classes = $this->findClasses($namespace, $resource, $exclude);
         // prepare for deep cloning
-        $prototype = serialize($prototype);
+        $serializedPrototype = serialize($prototype);
+        $interfaces = array();
+        $singlyImplemented = array();
 
         foreach ($classes as $class) {
-            $this->setDefinition($class, unserialize($prototype));
+            if (interface_exists($class, false)) {
+                $interfaces[] = $class;
+            } else {
+                $this->setDefinition($class, unserialize($serializedPrototype));
+                foreach (class_implements($class, false) as $interface) {
+                    $singlyImplemented[$interface] = isset($singlyImplemented[$interface]) ? false : $class;
+                }
+            }
+        }
+        foreach ($interfaces as $interface) {
+            if (!empty($singlyImplemented[$interface])) {
+                $this->container->setAlias($interface, $singlyImplemented[$interface])
+                    ->setPublic(false);
+            }
         }
     }
 
@@ -129,7 +144,7 @@ abstract class FileLoader extends BaseFileLoader
                 throw new InvalidArgumentException(sprintf('Expected to find class "%s" in file "%s" while importing services from resource "%s", but it was not found! Check the namespace prefix used with the resource.', $class, $path, $pattern));
             }
 
-            if ($r->isInstantiable()) {
+            if ($r->isInstantiable() || $r->isInterface()) {
                 $classes[] = $class;
             }
         }

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Foo.php

@@ -2,13 +2,13 @@
 
 namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype;
 
-class Foo
+class Foo implements FooInterface, Sub\BarInterface
 {
     public function __construct($bar = null)
     {
     }
 
-    function setFoo(self $foo)
+    public function setFoo(self $foo)
     {
     }
 }

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/FooInterface.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype;
+
+interface FooInterface
+{
+}

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/Bar.php

@@ -2,6 +2,6 @@
 
 namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub;
 
-class Bar
+class Bar implements BarInterface
 {
 }

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/BarInterface.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub;
+
+interface BarInterface
+{
+}

src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php

@@ -11,10 +11,12 @@
 
 namespace Symfony\Component\DependencyInjection\Tests\Loader;
 
+use Psr\Container\ContainerInterface as PsrContainerInterface;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\Config\Loader\LoaderResolver;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
 use Symfony\Component\DependencyInjection\Loader\FileLoader;
@@ -25,7 +27,10 @@ use Symfony\Component\DependencyInjection\Loader\YamlFileLoader;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\AnotherSub\DeeperBaz;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\OtherDir\Baz;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Foo;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\FooInterface;
 use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\Bar;
+use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\BarInterface;
 
 class FileLoaderTest extends TestCase
 {
@@ -91,6 +96,14 @@ class FileLoaderTest extends TestCase
             array('service_container', Bar::class),
             array_keys($container->getDefinitions())
         );
+        $this->assertEquals(
+            array(
+                PsrContainerInterface::class,
+                ContainerInterface::class,
+                BarInterface::class,
+            ),
+            array_keys($container->getAliases())
+        );
     }
 
     public function testRegisterClassesWithExclude()
@@ -111,6 +124,43 @@ class FileLoaderTest extends TestCase
         $this->assertTrue($container->has(Baz::class));
         $this->assertFalse($container->has(Foo::class));
         $this->assertFalse($container->has(DeeperBaz::class));
+
+        $this->assertEquals(
+            array(
+                PsrContainerInterface::class,
+                ContainerInterface::class,
+                BarInterface::class,
+            ),
+            array_keys($container->getAliases())
+        );
+    }
+
+    public function testNestedRegisterClasses()
+    {
+        $container = new ContainerBuilder();
+        $loader = new TestFileLoader($container, new FileLocator(self::$fixturesPath.'/Fixtures'));
+
+        $prototype = new Definition();
+        $prototype->setPublic(true)->setPrivate(true);
+        $loader->registerClasses($prototype, 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\\', 'Prototype/*');
+
+        $this->assertTrue($container->has(Bar::class));
+        $this->assertTrue($container->has(Baz::class));
+        $this->assertTrue($container->has(Foo::class));
+
+        $this->assertEquals(
+            array(
+                PsrContainerInterface::class,
+                ContainerInterface::class,
+                FooInterface::class,
+            ),
+            array_keys($container->getAliases())
+        );
+
+        $alias = $container->getAlias(FooInterface::class);
+        $this->assertSame(Foo::class, (string) $alias);
+        $this->assertFalse($alias->isPublic());
+        $this->assertFalse($alias->isPrivate());
     }
 
     /**