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

fabpot · fabpot · commit 23b672f5af49 · 2017-01-30T07:41:46.000-08:00
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
-------

fb200a0d2f [DI] Enhance logging in compiler passes
diff --git a/Compiler/AutowirePass.php b/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;
diff --git a/Compiler/Compiler.php b/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.
      *
diff --git a/Compiler/InlineServiceDefinitionsPass.php b/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;
diff --git a/Compiler/LoggingFormatter.php b/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
 {
diff --git a/Compiler/RemoveAbstractDefinitionsPass.php b/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));
             }
         }
     }
diff --git a/Compiler/RemovePrivateAliasesPass.php b/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));
         }
     }
 }
diff --git a/Compiler/RemoveUnusedDefinitionsPass.php b/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;
             }
         }
diff --git a/Compiler/ReplaceAliasByActualDefinitionPass.php b/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);
diff --git a/Compiler/ResolveDefinitionTemplatesPass.php b/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
diff --git a/ContainerBuilder.php b/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.
      *

Original file line number	Diff line number	Diff line change
`@@ -150,8 +150,7 @@ private function getMethodsToAutowire(\ReflectionClass $reflectionClass, array $`
`150`	`150`	`}`
`151`	`151`
`152`	`152`	`if ($notFound = array_diff($autowiredMethods, $found)) {`
`153`		`- $compiler = $this->container->getCompiler();`
`154`		`- $compiler->addLogMessage($compiler->getLoggingFormatter()->formatUnusedAutowiringPatterns($this, $this->currentId, $notFound));`
	`153`	`+ $this->container->log($this, sprintf('Autowiring\'s patterns "%s" for service "%s" don\'t match any method.', implode('", "', $notFound), $this->currentId));`
`155`	`154`	`}`
`156`	`155`
`157`	`156`	`return $methodsToAutowire;`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,6 @@ public function __construct()`
`30`	`30`	`{`
`31`	`31`	`$this->passConfig = new PassConfig();`
`32`	`32`	`$this->serviceReferenceGraph = new ServiceReferenceGraph();`
`33`		`- $this->loggingFormatter = new LoggingFormatter();`
`34`	`33`	`}`
`35`	`34`
`36`	`35`	`/**`
`@@ -57,9 +56,17 @@ public function getServiceReferenceGraph()`
`57`	`56`	`* Returns the logging formatter which can be used by compilation passes.`
`58`	`57`	`*`
`59`	`58`	`* @return LoggingFormatter`
	`59`	`+ *`
	`60`	`+ * @deprecated since version 3.3, to be removed in 4.0. Use the ContainerBuilder::log() method instead.`
`60`	`61`	`*/`
`61`	`62`	`public function getLoggingFormatter()`
`62`	`63`	`{`
	`64`	`+ if (null === $this->loggingFormatter) {`
	`65`	`+ @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);`
	`66`	`+`
	`67`	`+ $this->loggingFormatter = new LoggingFormatter();`
	`68`	`+ }`
	`69`	`+`
`63`	`70`	`return $this->loggingFormatter;`
`64`	`71`	`}`
`65`	`72`
`@@ -92,12 +99,24 @@ public function addPass(CompilerPassInterface $pass, $type = PassConfig::TYPE_BE`
`92`	`99`	`* Adds a log message.`
`93`	`100`	`*`
`94`	`101`	`* @param string $string The log message`
	`102`	`+ *`
	`103`	`+ * @deprecated since version 3.3, to be removed in 4.0. Use the ContainerBuilder::log() method instead.`
`95`	`104`	`*/`
`96`	`105`	`public function addLogMessage($string)`
`97`	`106`	`{`
	`107`	`+ @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);`
	`108`	`+`
`98`	`109`	`$this->log[] = $string;`
`99`	`110`	`}`
`100`	`111`
	`112`	`+ /**`
	`113`	`+ * @final`
	`114`	`+ */`
	`115`	`+ public function log(CompilerPassInterface $pass, $message)`
	`116`	`+ {`
	`117`	`+ $this->log[] = get_class($pass).': '.$message;`
	`118`	`+ }`
	`119`	`+`
`101`	`120`	`/**`
`102`	`121`	`* Returns the log.`
`103`	`122`	`*`
Original file line number	Diff line number	Diff line change
`@@ -11,10 +11,14 @@`
`11`	`11`
`12`	`12`	`namespace Symfony\Component\DependencyInjection\Compiler;`
`13`	`13`
	`14`	`+@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);`
	`15`	`+`
`14`	`16`	`/**`
`15`	`17`	`* Used to format logging messages during the compilation.`
`16`	`18`	`*`
`17`	`19`	`* @author Johannes M. Schmitt <[email protected]>`
	`20`	`+ *`
	`21`	`+ * @deprecated since version 3.3, to be removed in 4.0. Use the ContainerBuilder::log() method instead.`
`18`	`22`	`*/`
`19`	`23`	`class LoggingFormatter`
`20`	`24`	`{`
Original file line number	Diff line number	Diff line change
`@@ -26,12 +26,11 @@ class RemoveAbstractDefinitionsPass implements CompilerPassInterface`
`26`	`26`	`public function process(ContainerBuilder $container)`
`27`	`27`	`{`
`28`	`28`	`$compiler = $container->getCompiler();`
`29`		`- $formatter = $compiler->getLoggingFormatter();`
`30`	`29`
`31`	`30`	`foreach ($container->getDefinitions() as $id => $definition) {`
`32`	`31`	`if ($definition->isAbstract()) {`
`33`	`32`	`$container->removeDefinition($id);`
`34`		`- $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'abstract'));`
	`33`	`+ $container->log($this, sprintf('Removed service "%s"; reason: abstract.', $id));`
`35`	`34`	`}`
`36`	`35`	`}`
`37`	`36`	`}`
Original file line number	Diff line number	Diff line change
`@@ -30,15 +30,14 @@ class RemovePrivateAliasesPass implements CompilerPassInterface`
`30`	`30`	`public function process(ContainerBuilder $container)`
`31`	`31`	`{`
`32`	`32`	`$compiler = $container->getCompiler();`
`33`		`- $formatter = $compiler->getLoggingFormatter();`
`34`	`33`
`35`	`34`	`foreach ($container->getAliases() as $id => $alias) {`
`36`	`35`	`if ($alias->isPublic()) {`
`37`	`36`	`continue;`
`38`	`37`	`}`
`39`	`38`
`40`	`39`	`$container->removeAlias($id);`
`41`		`- $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'private alias'));`
	`40`	`+ $container->log($this, sprintf('Removed service "%s"; reason: private alias.', $id));`
`42`	`41`	`}`
`43`	`42`	`}`
`44`	`43`	`}`
Original file line number	Diff line number	Diff line change
`@@ -38,7 +38,6 @@ public function setRepeatedPass(RepeatedPass $repeatedPass)`
`38`	`38`	`public function process(ContainerBuilder $container)`
`39`	`39`	`{`
`40`	`40`	`$compiler = $container->getCompiler();`
`41`		`- $formatter = $compiler->getLoggingFormatter();`
`42`	`41`	`$graph = $compiler->getServiceReferenceGraph();`
`43`	`42`
`44`	`43`	`$hasChanged = false;`
`@@ -69,10 +68,10 @@ public function process(ContainerBuilder $container)`
`69`	`68`	`$container->setDefinition((string) reset($referencingAliases), $definition);`
`70`	`69`	`$definition->setPublic(true);`
`71`	`70`	`$container->removeDefinition($id);`
`72`		`- $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'replaces alias '.reset($referencingAliases)));`
	`71`	`+ $container->log($this, sprintf('Removed service "%s"; reason: replaces alias %s.', $id, reset($referencingAliases)));`
`73`	`72`	`} elseif (0 === count($referencingAliases) && false === $isReferenced) {`
`74`	`73`	`$container->removeDefinition($id);`
`75`		`- $compiler->addLogMessage($formatter->formatRemoveService($this, $id, 'unused'));`
	`74`	`+ $container->log($this, sprintf('Removed service "%s"; reason: unused.', $id));`
`76`	`75`	`$hasChanged = true;`
`77`	`76`	`}`
`78`	`77`	`}`
Original file line number	Diff line number	Diff line change
`@@ -82,8 +82,7 @@ protected function processValue($value, $isRoot = false)`
`82`	`82`	`// Perform the replacement`
`83`	`83`	`$newId = $this->replacements[$referenceId];`
`84`	`84`	`$value = new Reference($newId, $value->getInvalidBehavior());`
`85`		`- $compiler = $this->container->getCompiler();`
`86`		`- $compiler->addLogMessage($compiler->getLoggingFormatter()->formatUpdateReference($this, $this->currentId, $referenceId, $newId));`
	`85`	`+ $this->container->log($this, sprintf('Changed reference of service "%s" previously pointing to "%s" to "%s".', $this->currentId, $referenceId, $newId));`
`87`	`86`	`}`
`88`	`87`
`89`	`88`	`return parent::processValue($value, $isRoot);`
Original file line number	Diff line number	Diff line change
`@@ -80,8 +80,7 @@ private function doResolveDefinition(ChildDefinition $definition)`
`80`	`80`	`$this->currentId = $id;`
`81`	`81`	`}`
`82`	`82`
`83`		`- $compiler = $this->container->getCompiler();`
`84`		`- $compiler->addLogMessage($compiler->getLoggingFormatter()->formatResolveInheritance($this, $this->currentId, $parent));`
	`83`	`+ $this->container->log($this, sprintf('Resolving inheritance for "%s" (parent: %s).', $this->currentId, $parent));`
`85`	`84`	`$def = new Definition();`
`86`	`85`
`87`	`86`	`// merge in parent definition`
Original file line number	Diff line number	Diff line change
`@@ -1171,6 +1171,14 @@ public function getNormalizedIds()`
`1171`	`1171`	`return $normalizedIds;`
`1172`	`1172`	`}`
`1173`	`1173`
	`1174`	`+ /**`
	`1175`	`+ * @final`
	`1176`	`+ */`
	`1177`	`+ public function log(CompilerPassInterface $pass, $message)`
	`1178`	`+ {`
	`1179`	`+ $this->getCompiler()->log($pass, $message);`
	`1180`	`+ }`
	`1181`	`+`
`1174`	`1182`	`/**`
`1175`	`1183`	`* Returns the Service Conditionals.`
`1176`	`1184`	`*`