feature #21396 [DI] Enhance logging in compiler passes (nicolas-grekas)

This PR was merged into the 3.3-dev branch.

Discussion
----------

[DI] Enhance logging in compiler passes

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

We should log more in compiler passes - and this should be better integrated in usual log reports.

For logging more, let's drop LoggingFormatter and add a simple "log" method on ContainerBuilder.
For better integration, let's throw silenced notices - they can be caught by our Debug handler.

Commits
-------

fb200a0 [DI] Enhance logging in compiler passes

src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php

@@ -53,8 +53,6 @@ class UnusedTagsPass implements CompilerPassInterface
 
     public function process(ContainerBuilder $container)
     {
-        $compiler = $container->getCompiler();
-        $formatter = $compiler->getLoggingFormatter();
         $tags = array_unique(array_merge($container->findTags(), $this->whitelist));
 
         foreach ($container->findUnusedTags() as $tag) {
@@ -81,7 +79,7 @@ public function process(ContainerBuilder $container)
                 $message .= sprintf(' Did you mean "%s"?', implode('", "', $candidates));
             }
 
-            $compiler->addLogMessage($formatter->format($this, $message));
+            $container->log($this, $message);
         }
     }
 }

src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php

@@ -106,7 +106,7 @@ public function build(ContainerBuilder $container)
             $container->addCompilerPass(new AddDebugLogProcessorPass(), PassConfig::TYPE_BEFORE_OPTIMIZATION, -32);
             $container->addCompilerPass(new UnusedTagsPass(), PassConfig::TYPE_AFTER_REMOVING);
             $container->addCompilerPass(new ContainerBuilderDebugDumpPass(), PassConfig::TYPE_AFTER_REMOVING);
-            $container->addCompilerPass(new CompilerDebugDumpPass(), PassConfig::TYPE_AFTER_REMOVING);
+            $container->addCompilerPass(new CompilerDebugDumpPass(), PassConfig::TYPE_AFTER_REMOVING, -32);
             $container->addCompilerPass(new ConfigCachePass());
             $container->addCompilerPass(new CacheCollectorPass());
         }

src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/UnusedTagsPassTest.php

@@ -19,18 +19,10 @@ public function testProcess()
     {
         $pass = new UnusedTagsPass();
 
-        $formatter = $this->getMockBuilder('Symfony\Component\DependencyInjection\Compiler\LoggingFormatter')->getMock();
-        $formatter
-            ->expects($this->at(0))
-            ->method('format')
-            ->with($pass, 'Tag "kenrel.event_subscriber" was defined on service(s) "foo", "bar", but was never used. Did you mean "kernel.event_subscriber"?')
-        ;
-
-        $compiler = $this->getMockBuilder('Symfony\Component\DependencyInjection\Compiler\Compiler')->getMock();
-        $compiler->expects($this->once())->method('getLoggingFormatter')->will($this->returnValue($formatter));
-
-        $container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerBuilder')->setMethods(array('findTaggedServiceIds', 'getCompiler', 'findUnusedTags', 'findTags'))->getMock();
-        $container->expects($this->once())->method('getCompiler')->will($this->returnValue($compiler));
+        $container = $this->getMockBuilder('Symfony\Component\DependencyInjection\ContainerBuilder')->setMethods(array('findTaggedServiceIds', 'findUnusedTags', 'findTags', 'log'))->getMock();
+        $container->expects($this->once())
+            ->method('log')
+            ->with($pass, 'Tag "kenrel.event_subscriber" was defined on service(s) "foo", "bar", but was never used. Did you mean "kernel.event_subscriber"?');
         $container->expects($this->once())
             ->method('findTags')
             ->will($this->returnValue(array('kenrel.event_subscriber')));

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

@@ -150,8 +150,7 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $
         }
 
         if ($notFound = array_diff($autowiredMethods, $found)) {
-            $compiler = $this->container->getCompiler();
-            $compiler->addLogMessage($compiler->getLoggingFormatter()->formatUnusedAutowiringPatterns($this, $this->currentId, $notFound));
+            $this->container->log($this, sprintf('Autowiring\'s patterns "%s" for service "%s" don\'t match any method.', implode('", "', $notFound), $this->currentId));
         }
 
         return $methodsToAutowire;

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

@@ -30,7 +30,6 @@ public function __construct()
     {
         $this->passConfig = new PassConfig();
         $this->serviceReferenceGraph = new ServiceReferenceGraph();
-        $this->loggingFormatter = new LoggingFormatter();
     }
 
     /**
@@ -57,9 +56,17 @@ public function getServiceReferenceGraph()
      * Returns the logging formatter which can be used by compilation passes.
      *
      * @return LoggingFormatter
+     *
+     * @deprecated since version 3.3, to be removed in 4.0. Use the ContainerBuilder::log() method instead.
      */
     public function getLoggingFormatter()
     {
+        if (null === $this->loggingFormatter) {
+            @trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the ContainerBuilder::log() method instead.', __METHOD__), E_USER_DEPRECATED);
+
+            $this->loggingFormatter = new LoggingFormatter();
+        }
+
         return $this->loggingFormatter;
     }
 
@@ -92,12 +99,24 @@ public function addPass(CompilerPassInterface $pass, $type = PassConfig::TYPE_BE
      * Adds a log message.
      *
      * @param string $string The log message
+     *
+     * @deprecated since version 3.3, to be removed in 4.0. Use the ContainerBuilder::log() method instead.
      */
     public function addLogMessage($string)
     {
+        @trigger_error(sprintf('The %s() method is deprecated since version 3.3 and will be removed in 4.0. Use the ContainerBuilder::log() method instead.', __METHOD__), E_USER_DEPRECATED);
+
         $this->log[] = $string;
     }
 
+    /**
+     * @final
+     */
+    public function log(CompilerPassInterface $pass, $message)
+    {
+        $this->log[] = get_class($pass).': '.$message;
+    }
+
     /**
      * Returns the log.
      *

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

@@ -43,11 +43,10 @@ protected function processValue($value, $isRoot = false)
             return $value;
         }
         if ($value instanceof Reference && $this->container->hasDefinition($id = (string) $value)) {
-            $compiler = $this->container->getCompiler();
             $definition = $this->container->getDefinition($id);
 
-            if ($this->isInlineableDefinition($id, $definition, $compiler->getServiceReferenceGraph())) {
-                $compiler->addLogMessage($compiler->getLoggingFormatter()->formatInlineService($this, $id, $this->currentId));
+            if ($this->isInlineableDefinition($id, $definition, $this->container->getCompiler()->getServiceReferenceGraph())) {
+                $this->container->log($this, sprintf('Inlined service "%s" to "%s".', $id, $this->currentId));
 
                 if ($definition->isShared()) {
                     return $definition;

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

@@ -11,10 +11,14 @@
 
 namespace Symfony\Component\DependencyInjection\Compiler;
 
+@trigger_error('The '.__NAMESPACE__.'\LoggingFormatter class is deprecated since version 3.3 and will be removed in 4.0. Use the ContainerBuilder::log() method instead.', E_USER_DEPRECATED);
+
 /**
  * Used to format logging messages during the compilation.
  *
  * @author Johannes M. Schmitt <schmittjoh@gmail.com>
+ *
+ * @deprecated since version 3.3, to be removed in 4.0. Use the ContainerBuilder::log() method instead.
  */
 class LoggingFormatter
 {

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

@@ -26,12 +26,11 @@ class RemoveAbstractDefinitionsPass implements CompilerPassInterface
     public function process(ContainerBuilder $container)
     {
         $compiler = $container->getCompiler();
-        $formatter = $compiler->getLoggingFormatter();
 
         foreach ($container->getDefinitions() as $id => $definition) {
             if ($definition->isAbstract()) {
                 $container->removeDefinition($id);
-                $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'abstract'));
+                $container->log($this, sprintf('Removed service "%s"; reason: abstract.', $id));
             }
         }
     }

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

@@ -30,15 +30,14 @@ class RemovePrivateAliasesPass implements CompilerPassInterface
     public function process(ContainerBuilder $container)
     {
         $compiler = $container->getCompiler();
-        $formatter = $compiler->getLoggingFormatter();
 
         foreach ($container->getAliases() as $id => $alias) {
             if ($alias->isPublic()) {
                 continue;
             }
 
             $container->removeAlias($id);
-            $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'private alias'));
+            $container->log($this, sprintf('Removed service "%s"; reason: private alias.', $id));
         }
     }
 }

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

@@ -38,7 +38,6 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)
     public function process(ContainerBuilder $container)
     {
         $compiler = $container->getCompiler();
-        $formatter = $compiler->getLoggingFormatter();
         $graph = $compiler->getServiceReferenceGraph();
 
         $hasChanged = false;
@@ -69,10 +68,10 @@ public function process(ContainerBuilder $container)
                 $container->setDefinition((string) reset($referencingAliases), $definition);
                 $definition->setPublic(true);
                 $container->removeDefinition($id);
-                $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'replaces alias '.reset($referencingAliases)));
+                $container->log($this, sprintf('Removed service "%s"; reason: replaces alias %s.', $id, reset($referencingAliases)));
             } elseif (0 === count($referencingAliases) && false === $isReferenced) {
                 $container->removeDefinition($id);
-                $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'unused'));
+                $container->log($this, sprintf('Removed service "%s"; reason: unused.', $id));
                 $hasChanged = true;
             }
         }

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

@@ -82,8 +82,7 @@ protected function processValue($value, $isRoot = false)
             // Perform the replacement
             $newId = $this->replacements[$referenceId];
             $value = new Reference($newId, $value->getInvalidBehavior());
-            $compiler = $this->container->getCompiler();
-            $compiler->addLogMessage($compiler->getLoggingFormatter()->formatUpdateReference($this, $this->currentId, $referenceId, $newId));
+            $this->container->log($this, sprintf('Changed reference of service "%s" previously pointing to "%s" to "%s".', $this->currentId, $referenceId, $newId));
         }
 
         return parent::processValue($value, $isRoot);

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

@@ -80,8 +80,7 @@ private function doResolveDefinition(ChildDefinition $definition)
             $this->currentId = $id;
         }
 
-        $compiler = $this->container->getCompiler();
-        $compiler->addLogMessage($compiler->getLoggingFormatter()->formatResolveInheritance($this, $this->currentId, $parent));
+        $this->container->log($this, sprintf('Resolving inheritance for "%s" (parent: %s).', $this->currentId, $parent));
         $def = new Definition();
 
         // merge in parent definition

src/Symfony/Component/DependencyInjection/ContainerBuilder.php

@@ -1171,6 +1171,14 @@ public function getNormalizedIds()
         return $normalizedIds;
     }
 
+    /**
+     * @final
+     */
+    public function log(CompilerPassInterface $pass, $message)
+    {
+        $this->getCompiler()->log($pass, $message);
+    }
+
     /**
      * Returns the Service Conditionals.
      *