bug #22681 Fixing a bug where abstract classes were wired with the prototype loader (weaverryan)

fabpot · fabpot · commit b33f1dfe07a3 · 2017-05-11T10:11:08.000-07:00
This PR was merged into the 3.3-dev branch. Discussion ---------- Fixing a bug where abstract classes were wired with the prototype loader | Q | A | ------------- | --- | Branch? | master | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | n/a The prototype/PSR-4 loader currently tries to wire abstract classes. The problem is if, for example, you have, for example: ```php abstract class BaseCommand extends Command { } ``` If this is registered as a service, and you have `autoconfigure`, then the console `Application` will try to use this a command. Was there some reason abstract classes were originally allowed to be registered as services with the PSR4/prototype loader? I don't know if there is a real use-case for registering abstract classes. If you wanted to use that service as a parent service... then you'll probably be configuring it yourself anyways. We could also fix this by changing all tags compiler passes to skip classes that are abstract... *if* there is a use-case for Abstract classes being auto-registered. Cheers! Commits ------- 5326bab Fixing a bug where abstract classes were wired
diff --git a/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php b/src/Symfony/Component/DependencyInjection/Loader/FileLoader.php
@@ -108,7 +108,8 @@ private function findClasses($namespace, $pattern)
             if (!$r = $this->container->getReflectionClass($class)) {
                 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->isInterface() && !$r->isTrait()) {
+
+            if (!$r->isInterface() && !$r->isTrait() && !$r->isAbstract()) {
                 $classes[] = $class;
             }
         }
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/NoLoadAbstractBar.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/NoLoadAbstractBar.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub;
+
+abstract class NoLoadAbstractBar
+{
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/NoLoadBarInterface.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/NoLoadBarInterface.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub;
+
+interface NoLoadBarInterface
+{
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/NoLoadBarTrait.php b/src/Symfony/Component/DependencyInjection/Tests/Fixtures/Prototype/Sub/NoLoadBarTrait.php
@@ -0,0 +1,7 @@
+<?php
+
+namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub;
+
+trait NoLoadBarTrait
+{
+}
diff --git a/src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php b/src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php
@@ -84,7 +84,10 @@ public function testRegisterClasses()
 
         $loader->registerClasses(new Definition(), 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\\', 'Prototype/%sub_dir%/*');
 
-        $this->assertTrue($container->has(Bar::class));
+        $this->assertEquals(
+            array('service_container', Bar::class),
+            array_keys($container->getDefinitions())
+        );
     }
 
     /**

Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,8 @@ private function findClasses($namespace, $pattern)`
`108`	`108`	`if (!$r = $this->container->getReflectionClass($class)) {`
`109`	`109`	`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));`
`110`	`110`	`}`
`111`		`- if (!$r->isInterface() && !$r->isTrait()) {`
	`111`	`+`
	`112`	`+ if (!$r->isInterface() && !$r->isTrait() && !$r->isAbstract()) {`
`112`	`113`	`$classes[] = $class;`
`113`	`114`	`}`
`114`	`115`	`}`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,7 @@ @@
 +<?php
++
 +namespace Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub;
++
 +abstract class NoLoadAbstractBar
 +{
 +}
Original file line number	Diff line number	Diff line change
`@@ -84,7 +84,10 @@ public function testRegisterClasses()`
`84`	`84`
`85`	`85`	`$loader->registerClasses(new Definition(), 'Symfony\Component\DependencyInjection\Tests\Fixtures\Prototype\Sub\\', 'Prototype/%sub_dir%/*');`
`86`	`86`
`87`		`- $this->assertTrue($container->has(Bar::class));`
	`87`	`+ $this->assertEquals(`
	`88`	`+ array('service_container', Bar::class),`
	`89`	`+ array_keys($container->getDefinitions())`
	`90`	`+ );`
`88`	`91`	`}`
`89`	`92`
`90`	`93`	`/**`