bug #33350 [DI] scope singly-implemented interfaces detection by file…

… (daniel-iwaniec, nicolas-grekas)

This PR was merged into the 4.4 branch.

Discussion
----------

[DI] scope singly-implemented interfaces detection by file

| Q             | A
| ------------- | ---
| Branch?       | 4.4
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | yes
| Deprecations? | no
| Tests pass?   | yes
| License       | MIT

[DependencyInjection] fixed handling singly implemented interfaces when importing multiple resources

for example:
```yaml
App\Adapter\:
    resource: '../src/Adapter/*'
App\Port\:
    resource: '../src/Port/*'
```

this configuration wont create service for interface (in other words singly implemented interface wont be autowired) and this chage fixes it

**Also** this will prevent false positives - for example if I had one implementation in \App\Port namespace and another in \App\Adapter then interface service would still be registered

but that could potentially break exisitng code not aware of this bug

Commits
-------

c1f3970 [DI] add FileLoader::registerAliasesForSinglyImplementedInterfaces()
bec3890 [DI] scope singly-implemented interfaces detection by file

UPGRADE-4.4.md

@@ -16,41 +16,37 @@ Debug
 DependencyInjection
 -------------------
 
+ * Made singly-implemented interfaces detection be scoped by file
  * Deprecated support for short factories and short configurators in Yaml
 
    Before:
    ```yaml
    services:
-     my_service:
-       factory: factory_service:method
+       my_service:
+           factory: factory_service:method
    ```
 
    After:
    ```yaml
    services:
-     my_service:
-       factory: ['@factory_service', method]
+       my_service:
+           factory: ['@factory_service', method]
    ```
+
  * Deprecated `tagged` in favor of `tagged_iterator`
 
    Before:
    ```yaml
    services:
-       App\Handler:
-           tags: ['app.handler']
-
        App\HandlerCollection:
-           arguments: [!tagged app.handler]
+           arguments: [!tagged my_tag]
    ```
 
    After:
    ```yaml
    services:
-       App\Handler:
-       tags: ['app.handler']
-
-   App\HandlerCollection:
-       arguments: [!tagged_iterator app.handler]
+       App\HandlerCollection:
+           arguments: [!tagged_iterator my_tag]
    ```
 
  * Passing an instance of `Symfony\Component\DependencyInjection\Parameter` as class name to `Symfony\Component\DependencyInjection\Definition` is deprecated.
@@ -145,6 +141,7 @@ HttpKernel
 
    As many bundles must be compatible with a range of Symfony versions, the current 
    directory convention is not deprecated yet, but it will be in the future.
+
  * Deprecated the second and third argument of `KernelInterface::locateResource`
  * Deprecated the second and third argument of `FileLocator::__construct`
  * Deprecated loading resources from `%kernel.root_dir%/Resources` and `%kernel.root_dir%` as
@@ -283,37 +280,37 @@ TwigBundle
    Before (`templates/bundles/TwigBundle/Exception/error.jsonld.twig`):
    ```twig
    { 
-     "@id": "https://example.com",
-     "@type": "error",
-     "@context": {
-         "title": "{{ status_text }}",
-         "code": {{ status_code }},
-         "message": "{{ exception.message }}"
-     }
+       "@id": "https://example.com",
+       "@type": "error",
+       "@context": {
+           "title": "{{ status_text }}",
+           "code": {{ status_code }},
+           "message": "{{ exception.message }}"
+       }
    }
    ```
 
    After (`App\ErrorRenderer\JsonLdErrorRenderer`):
    ```php
    class JsonLdErrorRenderer implements ErrorRendererInterface
    {
-     public static function getFormat(): string
-     {
-         return 'jsonld';
-     }
+       public static function getFormat(): string
+       {
+           return 'jsonld';
+       }
    
-     public function render(FlattenException $exception): string
-     {
-         return json_encode([
-             '@id' => 'https://example.com',
-             '@type' => 'error',
-             '@context' => [
-                 'title' => $exception->getTitle(),
-                 'code' => $exception->getStatusCode(),
-                 'message' => $exception->getMessage(),
-             ],
-         ]);
-     }
+       public function render(FlattenException $exception): string
+       {
+           return json_encode([
+               '@id' => 'https://example.com',
+               '@type' => 'error',
+               '@context' => [
+                   'title' => $exception->getTitle(),
+                   'code' => $exception->getStatusCode(),
+                   'message' => $exception->getMessage(),
+               ],
+           ]);
+       }
    }
    ```
 

src/Symfony/Component/DependencyInjection/CHANGELOG.md

@@ -10,6 +10,7 @@ CHANGELOG
  * deprecated `tagged` in favor of `tagged_iterator`
  * deprecated passing an instance of `Symfony\Component\DependencyInjection\Parameter` as class name to `Symfony\Component\DependencyInjection\Definition`
  * added support for binding iterable and tagged services
+ * made singly-implemented interfaces detection be scoped by file
 
 4.3.0
 -----

src/Symfony/Component/DependencyInjection/Loader/Configurator/ServicesConfigurator.php

@@ -139,4 +139,9 @@ final public function __invoke(string $id, string $class = null): ServiceConfigu
     {
         return $this->set($id, $class);
     }
+
+    public function __destruct()
+    {
+        $this->loader->registerAliasesForSinglyImplementedInterfaces();
+    }
 }

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

@@ -29,6 +29,8 @@ abstract class FileLoader extends BaseFileLoader
     protected $container;
     protected $isLoadingInstanceof = false;
     protected $instanceof = [];
+    protected $interfaces = [];
+    protected $singlyImplemented = [];
 
     public function __construct(ContainerBuilder $container, FileLocatorInterface $locator)
     {
@@ -57,12 +59,10 @@ public function registerClasses(Definition $prototype, $namespace, $resource, $e
         $classes = $this->findClasses($namespace, $resource, (array) $exclude);
         // prepare for deep cloning
         $serializedPrototype = serialize($prototype);
-        $interfaces = [];
-        $singlyImplemented = [];
 
         foreach ($classes as $class => $errorMessage) {
             if (interface_exists($class, false)) {
-                $interfaces[] = $class;
+                $this->interfaces[] = $class;
             } else {
                 $this->setDefinition($class, $definition = unserialize($serializedPrototype));
                 if (null !== $errorMessage) {
@@ -71,16 +71,21 @@ public function registerClasses(Definition $prototype, $namespace, $resource, $e
                     continue;
                 }
                 foreach (class_implements($class, false) as $interface) {
-                    $singlyImplemented[$interface] = isset($singlyImplemented[$interface]) ? false : $class;
+                    $this->singlyImplemented[$interface] = isset($this->singlyImplemented[$interface]) ? false : $class;
                 }
             }
         }
-        foreach ($interfaces as $interface) {
-            if (!empty($singlyImplemented[$interface])) {
-                $this->container->setAlias($interface, $singlyImplemented[$interface])
-                    ->setPublic(false);
+    }
+
+    public function registerAliasesForSinglyImplementedInterfaces()
+    {
+        foreach ($this->interfaces as $interface) {
+            if (!empty($this->singlyImplemented[$interface]) && !$this->container->hasAlias($interface)) {
+                $this->container->setAlias($interface, $this->singlyImplemented[$interface])->setPublic(false);
             }
         }
+
+        $this->interfaces = $this->singlyImplemented = [];
     }
 
     /**

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

@@ -41,10 +41,15 @@ public function load($resource, $type = null)
             return include $path;
         }, $this, ProtectedPhpFileLoader::class);
 
-        $callback = $load($path);
+        try {
+            $callback = $load($path);
 
-        if (\is_object($callback) && \is_callable($callback)) {
-            $callback(new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource), $this->container, $this);
+            if (\is_object($callback) && \is_callable($callback)) {
+                $callback(new ContainerConfigurator($this->container, $this, $this->instanceof, $path, $resource), $this->container, $this);
+            }
+        } finally {
+            $this->instanceof = [];
+            $this->registerAliasesForSinglyImplementedInterfaces();
         }
     }
 

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

@@ -66,6 +66,7 @@ public function load($resource, $type = null)
             $this->parseDefinitions($xml, $path, $defaults);
         } finally {
             $this->instanceof = [];
+            $this->registerAliasesForSinglyImplementedInterfaces();
         }
     }
 

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

@@ -150,6 +150,7 @@ public function load($resource, $type = null)
             $this->parseDefinitions($content, $path);
         } finally {
             $this->instanceof = [];
+            $this->registerAliasesForSinglyImplementedInterfaces();
         }
     }
 

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Adapter/Adapter.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter;
+
+use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface;
+
+class Adapter implements PortInterface
+{
+}

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/AnotherAdapter/Adapter.php

@@ -0,0 +1,9 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter;
+
+use Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface;
+
+class Adapter implements PortInterface
+{
+}

src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/SinglyImplementedInterface/Port/PortInterface.php

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

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype.php

@@ -9,7 +9,7 @@
         ->tag('baz');
     $di->load(Prototype::class.'\\', '../Prototype')
         ->autoconfigure()
-        ->exclude('../Prototype/{OtherDir,BadClasses}')
+        ->exclude('../Prototype/{OtherDir,BadClasses,SinglyImplementedInterface}')
         ->factory('f')
         ->deprecate('%service_id%')
         ->args([0])

src/Symfony/Component/DependencyInjection/Tests/Fixtures/config/prototype_array.php

@@ -9,7 +9,7 @@
         ->tag('baz');
     $di->load(Prototype::class.'\\', '../Prototype')
         ->autoconfigure()
-        ->exclude(['../Prototype/OtherDir', '../Prototype/BadClasses'])
+        ->exclude(['../Prototype/OtherDir', '../Prototype/BadClasses', '../Prototype/SinglyImplementedInterface'])
         ->factory('f')
         ->deprecate('%service_id%')
         ->args([0])

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources.xml

@@ -0,0 +1,16 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <defaults autowire="true" />
+
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\"
+               resource="../Prototype/SinglyImplementedInterface/Adapter/*" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\"
+               resource="../Prototype/SinglyImplementedInterface/AnotherAdapter/*" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\"
+               resource="../Prototype/SinglyImplementedInterface/Port/*" />
+  </services>
+</container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <defaults autowire="true" />
+
+    <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface"
+             alias="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\Adapter" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\"
+               resource="../Prototype/SinglyImplementedInterface/Port/*" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\"
+               resource="../Prototype/SinglyImplementedInterface/Adapter/*" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\"
+               resource="../Prototype/SinglyImplementedInterface/AnotherAdapter/*" />
+  </services>
+</container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.xml

@@ -0,0 +1,18 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <defaults autowire="true" />
+
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\"
+               resource="../Prototype/SinglyImplementedInterface/Port/*" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\"
+               resource="../Prototype/SinglyImplementedInterface/Adapter/*" />
+    <service id="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface"
+             alias="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\Adapter" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\"
+               resource="../Prototype/SinglyImplementedInterface/AnotherAdapter/*" />
+  </services>
+</container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype.xml

@@ -1,6 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
   <services>
-      <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\" resource="../Prototype/*" exclude="../Prototype/{OtherDir,BadClasses}" />
+      <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\" resource="../Prototype/*" exclude="../Prototype/{OtherDir,BadClasses,SinglyImplementedInterface}" />
   </services>
 </container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services_prototype_array.xml

@@ -4,6 +4,7 @@
       <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\" resource="../Prototype/*">
           <exclude>../Prototype/OtherDir</exclude>
           <exclude>../Prototype/BadClasses</exclude>
+          <exclude>../Prototype/SinglyImplementedInterface</exclude>
       </prototype>
   </services>
 </container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/singly_implemented_interface_in_multiple_resources.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="utf-8"?>
+
+<container xmlns="http://symfony.com/schema/dic/services"
+    xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
+  <services>
+    <defaults autowire="true" />
+
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\"
+               resource="../Prototype/SinglyImplementedInterface/Port/*" />
+    <prototype namespace="Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\"
+               resource="../Prototype/SinglyImplementedInterface/Adapter/*" />
+  </services>
+</container>

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources.yml

@@ -0,0 +1,12 @@
+services:
+    _defaults:
+        autowire: true
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\:
+        resource: ../Prototype/SinglyImplementedInterface/Adapter/*
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\:
+        resource: ../Prototype/SinglyImplementedInterface/AnotherAdapter/*
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\:
+        resource: ../Prototype/SinglyImplementedInterface/Port/*

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias.yml

@@ -0,0 +1,15 @@
+services:
+    _defaults:
+        autowire: true
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface:
+        alias: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\Adapter
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\:
+        resource: ../Prototype/SinglyImplementedInterface/Port/*
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\:
+        resource: ../Prototype/SinglyImplementedInterface/Adapter/*
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\:
+        resource: ../Prototype/SinglyImplementedInterface/AnotherAdapter/*

src/Symfony/Component/DependencyInjection/Tests/Fixtures/yaml/not_singly_implemented_interface_in_multiple_resources_with_previously_registered_alias2.yml

@@ -0,0 +1,15 @@
+services:
+    _defaults:
+        autowire: true
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\:
+        resource: ../Prototype/SinglyImplementedInterface/Port/*
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\:
+        resource: ../Prototype/SinglyImplementedInterface/Adapter/*
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Port\PortInterface:
+        alias: Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\Adapter\Adapter
+
+    Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\SinglyImplementedInterface\AnotherAdapter\:
+        resource: ../Prototype/SinglyImplementedInterface/AnotherAdapter/*