minor #27845 [DI] Improve exception messages by hiding the hidden ids they contain (nicolas-grekas)

This PR was merged into the 4.2-dev branch.

Discussion
----------

[DI] Improve exception messages by hiding the hidden ids they contain

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

This PR improves error messages containing hidden ids, especially the ones mentioning service locators.

Right now, when a service subscriber is incomplete, we get:
> The service ".service_locator.G69Xsbl.App\Controller\MyRouter" has a dependency on a non-existent service "Symfony\Component\Config\Loader\LoaderInterface".

With this PR we get this instead:
> The service "routing.loader" in the container provided to "App\Controller\MyRouter" has a dependency on a non-existent service "Symfony\Component\Config\Loader\LoaderInterface".

When no locators are involved, the hidden service is swallowed:
> The service "App\Controller\MyRouter" has a dependency on a non-existent service "Symfony\Component\Config\Loader\LoaderInterface".

This PR also improves runtime exceptions thrown in service locators.

Before:
> Cannot autowire service ".service_locator.Z1jvVrN": it references interface "Symfony\Component\Config\Loader\LoaderInterface" but no such service exists. You should maybe alias this interface to one of these existing services: [...].

After:
> Cannot autowire service "routing.loader" required by "App\Controller\MyRouter": it references interface "Symfony\Component\Config\Loader\LoaderInterface" but no such service exists. You should maybe alias this interface to one of these existing services: [...].

TODO:
- [x] add tests.

Commits
-------

d2b4901a43 [DI] Improve exception messages by hiding the hidden ids they contain

src/Symfony/Component/DependencyInjection/Compiler/CheckExceptionOnInvalidReferenceBehaviorPass.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\DependencyInjection\Compiler;
 
 use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\ContainerInterface;
 use Symfony\Component\DependencyInjection\Reference;
 
@@ -22,15 +23,66 @@ use Symfony\Component\DependencyInjection\Reference;
  */
 class CheckExceptionOnInvalidReferenceBehaviorPass extends AbstractRecursivePass
 {
+    private $serviceLocatorContextIds = array();
+
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        $this->serviceLocatorContextIds = array();
+        foreach ($container->findTaggedServiceIds('container.service_locator_context') as $id => $tags) {
+            $this->serviceLocatorContextIds[$id] = $tags[0]['id'];
+            $container->getDefinition($id)->clearTag('container.service_locator_context');
+        }
+
+        try {
+            return parent::process($container);
+        } finally {
+            $this->serviceLocatorContextIds = array();
+        }
+    }
+
     protected function processValue($value, $isRoot = false)
     {
         if (!$value instanceof Reference) {
             return parent::processValue($value, $isRoot);
         }
-        if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE >= $value->getInvalidBehavior() && !$this->container->has($id = (string) $value)) {
+        if (ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE < $value->getInvalidBehavior() || $this->container->has($id = (string) $value)) {
-            throw new ServiceNotFoundException($id, $this->currentId);
+            return $value;
         }
 
-        return $value;
+        $currentId = $this->currentId;
+        $graph = $this->container->getCompiler()->getServiceReferenceGraph();
+
+        if (isset($this->serviceLocatorContextIds[$currentId])) {
+            $currentId = $this->serviceLocatorContextIds[$currentId];
+            $locator = $this->container->getDefinition($this->currentId)->getFactory()[0];
+
+            foreach ($locator->getArgument(0) as $k => $v) {
+                if ($v->getValues()[0] === $value) {
+                    if ($k !== $id) {
+                        $currentId = $k.'" in the container provided to "'.$currentId;
+                    }
+                    throw new ServiceNotFoundException($id, $currentId);
+                }
+            }
+        }
+
+        if ('.' === $currentId[0] && $graph->hasNode($currentId)) {
+            foreach ($graph->getNode($currentId)->getInEdges() as $edge) {
+                if (!$edge->getValue() instanceof Reference || ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE < $edge->getValue()->getInvalidBehavior()) {
+                    continue;
+                }
+                $sourceId = $edge->getSourceNode()->getId();
+
+                if ('.' !== $sourceId[0]) {
+                    $currentId = $sourceId;
+                    break;
+                }
+            }
+        }
+
+        throw new ServiceNotFoundException($id, $currentId);
     }
 }

src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php

@@ -83,6 +83,7 @@ class PassConfig
             new RemoveAbstractDefinitionsPass(),
             new RemoveUnusedDefinitionsPass(),
             new InlineServiceDefinitionsPass(new AnalyzeServiceReferencesPass()),
+            new AnalyzeServiceReferencesPass(),
             new DefinitionErrorExceptionPass(),
             new CheckExceptionOnInvalidReferenceBehaviorPass(),
             new ResolveHotPathPass(),

src/Symfony/Component/DependencyInjection/Compiler/ServiceLocatorTagPass.php

@@ -107,6 +107,7 @@ final class ServiceLocatorTagPass extends AbstractRecursivePass
             $container->register($id .= '.'.$callerId, ServiceLocator::class)
                 ->setPublic(false)
                 ->setFactory(array(new Reference($locatorId), 'withContext'))
+                ->addTag('container.service_locator_context', array('id' => $callerId))
                 ->addArgument($callerId)
                 ->addArgument(new Reference('service_container'));
         }

src/Symfony/Component/DependencyInjection/ServiceLocator.php

@@ -12,6 +12,7 @@
 namespace Symfony\Component\DependencyInjection;
 
 use Psr\Container\ContainerInterface as PsrContainerInterface;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 use Symfony\Component\DependencyInjection\Exception\ServiceCircularReferenceException;
 use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
 
@@ -62,6 +63,22 @@ class ServiceLocator implements PsrContainerInterface
         $this->loading[$id] = $id;
         try {
             return $this->factories[$id]();
+        } catch (RuntimeException $e) {
+            if (!$this->externalId) {
+                throw $e;
+            }
+            $what = sprintf('service "%s" required by "%s"', $id, $this->externalId);
+            $message = preg_replace('/service "\.service_locator\.[^"]++"/', $what, $e->getMessage());
+
+            if ($e->getMessage() === $message) {
+                $message = sprintf('Cannot resolve %s: %s', $what, $message);
+            }
+
+            $r = new \ReflectionProperty($e, 'message');
+            $r->setAccessible(true);
+            $r->setValue($e, $message);
+
+            throw $e;
         } finally {
             unset($this->loading[$id]);
         }

src/Symfony/Component/DependencyInjection/Tests/Compiler/CheckExceptionOnInvalidReferenceBehaviorPassTest.php

@@ -14,7 +14,10 @@ namespace Symfony\Component\DependencyInjection\Tests\Compiler;
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\DependencyInjection\Argument\BoundArgument;
 use Symfony\Component\DependencyInjection\Definition;
+use Symfony\Component\DependencyInjection\Compiler\AnalyzeServiceReferencesPass;
 use Symfony\Component\DependencyInjection\Compiler\CheckExceptionOnInvalidReferenceBehaviorPass;
+use Symfony\Component\DependencyInjection\Compiler\InlineServiceDefinitionsPass;
+use Symfony\Component\DependencyInjection\Compiler\ServiceLocatorTagPass;
 use Symfony\Component\DependencyInjection\Reference;
 use Symfony\Component\DependencyInjection\ContainerBuilder;
 
@@ -82,6 +85,36 @@ class CheckExceptionOnInvalidReferenceBehaviorPassTest extends TestCase
         $this->addToAssertionCount(1);
     }
 
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
+     * @expectedExceptionMessage The service "foo" in the container provided to "bar" has a dependency on a non-existent service "baz".
+     */
+    public function testWithErroredServiceLocator()
+    {
+        $container = new ContainerBuilder();
+
+        ServiceLocatorTagPass::register($container, array('foo' => new Reference('baz')), 'bar');
+
+        (new AnalyzeServiceReferencesPass())->process($container);
+        (new InlineServiceDefinitionsPass())->process($container);
+        $this->process($container);
+    }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException
+     * @expectedExceptionMessage The service "bar" has a dependency on a non-existent service "foo".
+     */
+    public function testWithErroredHiddenService()
+    {
+        $container = new ContainerBuilder();
+
+        ServiceLocatorTagPass::register($container, array('foo' => new Reference('foo')), 'bar');
+
+        (new AnalyzeServiceReferencesPass())->process($container);
+        (new InlineServiceDefinitionsPass())->process($container);
+        $this->process($container);
+    }
+
     private function process(ContainerBuilder $container)
     {
         $pass = new CheckExceptionOnInvalidReferenceBehaviorPass();

src/Symfony/Component/DependencyInjection/Tests/ServiceLocatorTest.php

@@ -13,6 +13,7 @@ namespace Symfony\Component\DependencyInjection\Tests;
 
 use PHPUnit\Framework\TestCase;
 use Symfony\Component\DependencyInjection\Container;
+use Symfony\Component\DependencyInjection\Exception\RuntimeException;
 use Symfony\Component\DependencyInjection\ServiceLocator;
 use Symfony\Component\DependencyInjection\ServiceSubscriberInterface;
 
@@ -126,6 +127,20 @@ class ServiceLocatorTest extends TestCase
         $this->assertSame('baz', $locator('bar'));
         $this->assertNull($locator('dummy'), '->__invoke() should return null on invalid service');
     }
+
+    /**
+     * @expectedException \Symfony\Component\DependencyInjection\Exception\RuntimeException
+     * @expectedExceptionMessage Invalid service "foo" required by "external-id".
+     */
+    public function testRuntimeException()
+    {
+        $locator = new ServiceLocator(array(
+            'foo' => function () { throw new RuntimeException('Invalid service ".service_locator.abcdef".'); },
+        ));
+
+        $locator = $locator->withContext('external-id', new Container());
+        $locator->get('foo');
+    }
 }
 
 class SomeServiceSubscriber implements ServiceSubscriberinterface