[DependencyInjection] Throw better exception when missing use-statement

[Nyholm]

Q	A
Branch?	3.4
Bug fix?	yes
New feature?	no
Deprecations?	no
Tickets
License	MIT
Doc PR

Im trying to address this issue: https://stackoverflow.com/questions/49958116/class-appbundle-controller-homecontroller-used-for-service-appbundle-controll

So here is the scenario:

namespace App\Foo;
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;

abstract class MyBaseController extends AbstractController{}

namespace App\Bar;

// Note that "use App\Foo\MyBaseController" is missing
use App\MyService;

class StartpageController extends MyBaseController{
    public function action(MyService $service)
    {
        // ...
    }
}

Since StartpageController is autowired and tagged with controller.service_arguments we will run it in RegisterControllerArgumentLocatorsPass.

If my cache folder is empty, I will get an error saying something like:

Class "App\Bar\StartpageController" used for service "App\Bar\StartpageController" cannot be found.

That does not make any sense.

This PR is trying to load App\Bar\StartpageController to make PHP discover that we are missing something. Now I will get a much better error:

Attempted to load class "MyBaseController" from namespace "App\Bar". Did you forget a "use" statement for "App\Foo\MyBaseController"?

---

If for some reason this is not the issue and PHP does not throw this error, we just continue with the normal exception.

[OskarStark]

Class "App\Bar\StartpageController" used for service "App\Bar\StartpageController" cannot be found.

Is this really the message or is it:

- Class "App\Bar\StartpageController" used for service "App\Bar\StartpageController" cannot be found.
+ Class "App\Bar\MyBaseController" used for service "App\Bar\StartpageController" cannot be found.

[Nyholm]

Yes, it is really correct. I did not make a typo.

That is why the error message is confusing and a change is needed.

[nicolas-grekas]

I think the attached patch is currently a hack that works around the real issue: the core issue is that getReflectionClass() caches missing classes when $throw is false. This means the return value of the function is call-order dependent.

Here is a better fix:

--- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
+++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -376,7 +376,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
             if ($throw) {
                 throw $e;
             }
-            $classReflector = false;
+            $classReflector = null;
         }
 
         if ($this->trackResources) {
@@ -392,7 +392,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface
             $this->classReflectors[$class] = $classReflector;
         }
 
-        return $classReflector ?: null;
+        return $classReflector;
     }

This will display this:

Which is correct.

It's not as good as the one you're achieving with the class_exists(), but that's another issue: the ErrorHandler should be able to do these suggestions on ReflectionException too, not only on fatal errors. - This last part would be for 4.4.

Makes sense?

[nicolas-grekas]

See #34282

[nicolas-grekas]

Thanks for raising the point!

[Nyholm]

Thank you for improving the PR.