From f51fe4ac4182a5e841a565639ff12c4ee2629088 Mon Sep 17 00:00:00 2001 From: Florian Pfitzer Date: Wed, 29 Oct 2014 10:15:52 +0100 Subject: [PATCH 1/3] [FrameworkBundle] [DependencyInjection] added logging of unused tags during container compilation --- .../Compiler/UnusedTagsPass.php | 86 +++++++++++++++++++ .../FrameworkBundle/FrameworkBundle.php | 2 + .../DependencyInjection/ContainerBuilder.php | 19 ++++ .../Tests/ContainerBuilderTest.php | 12 +++ 4 files changed, 119 insertions(+) create mode 100644 src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php new file mode 100644 index 0000000000..262017f2a4 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php @@ -0,0 +1,86 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface; +use Symfony\Component\DependencyInjection\ContainerBuilder; + +/** + * Find all service tags which are defined, but not used and yield a warning log message. + * + * @author Florian Pfitzer + */ +class UnusedTagsPass implements CompilerPassInterface +{ + /** + * whitelisted tags + * + * @var array + */ + protected $whitelist = array( + "console.command", + "data_collector", + "form.type", + "form.type_extension", + "form.type_guesser", + "kernel.cache_clearer", + "kernel.cache_warmer", + "kernel.event_listener", + "kernel.event_subscriber", + "kernel.fragment_renderer", + "monolog.logger", + "routing.loader", + "security.remember_me_aware", + "security.voter", + "serializer.encoder", + "templating.helper", + "translation.dumper", + "translation.extractor", + "translation.loader", + "twig.extension", + "twig.loader", + "validator.constraint_validator", + "validator.initializer", + ); + + public function process(ContainerBuilder $container) + { + $compiler = $container->getCompiler(); + $formatter = $compiler->getLoggingFormatter(); + $tags = $container->findTags(); + + $unusedTags = $container->findUnusedTags(); + foreach ($unusedTags as $tag) { + // skip whitelisted tags + if (in_array($tag, $this->whitelist)) { + continue; + } + // check for typos + $candidates = array(); + foreach ($tags as $definedTag) { + if ($definedTag === $tag) { + continue; + } + if (false !== strpos($definedTag, $tag) || levenshtein($tag, $definedTag) <= strlen($tag) / 3) { + $candidates[] = $definedTag; + } + } + + $services = array_keys($container->findTaggedServiceIds($tag)); + $message = sprintf('Tag "%s" was defined on the service(s) %s, but was never used.', $tag, implode(',', $services)); + if (!empty($candidates)) { + $message .= sprintf(' Did you mean "%s"?', implode('", "', $candidates)); + } + $compiler->addLogMessage($formatter->format($this, $message)); + } + } +} diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 7b1c77f225..1cdf83df5d 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -28,6 +28,7 @@ use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\CompilerDebugDum use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\TranslationExtractorPass; use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\TranslationDumperPass; use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\SerializerPass; +use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\UnusedTagsPass; use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\ConfigCachePass; use Symfony\Component\Debug\ErrorHandler; use Symfony\Component\DependencyInjection\ContainerBuilder; @@ -89,6 +90,7 @@ class FrameworkBundle extends Bundle $container->addCompilerPass(new TranslationDumperPass()); $container->addCompilerPass(new FragmentRendererPass(), PassConfig::TYPE_AFTER_REMOVING); $container->addCompilerPass(new SerializerPass()); + $container->addCompilerPass(new UnusedTagsPass(), PassConfig::TYPE_AFTER_REMOVING); if ($container->getParameter('kernel.debug')) { $container->addCompilerPass(new ContainerBuilderDebugDumpPass(), PassConfig::TYPE_AFTER_REMOVING); diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index 52405413af..ca8a136d25 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -90,6 +90,11 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ private $expressionLanguageProviders = array(); + /** + * @var array with tag names used by findTaggedServiceIds + */ + private $usedTags = array(); + /** * Sets the track resources flag. * @@ -1064,6 +1069,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface */ public function findTaggedServiceIds($name) { + $this->usedTags[] = $name; $tags = array(); foreach ($this->getDefinitions() as $id => $definition) { if ($definition->hasTag($name)) { @@ -1089,6 +1095,19 @@ class ContainerBuilder extends Container implements TaggedContainerInterface return array_unique($tags); } + /** + * Returns all tags not queried by findTaggedServiceIds + * + * @return array An array of tags + */ + public function findUnusedTags() + { + $tags = array_values(array_diff($this->findTags(), $this->usedTags)); + $tags = array_unique($tags); + + return $tags; + } + public function addExpressionLanguageProvider(ExpressionFunctionProviderInterface $provider) { $this->expressionLanguageProviders[] = $provider; diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 511b29c939..047a7cf334 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -560,6 +560,18 @@ class ContainerBuilderTest extends \PHPUnit_Framework_TestCase $this->assertEquals(array(), $builder->findTaggedServiceIds('foobar'), '->findTaggedServiceIds() returns an empty array if there is annotated services'); } + public function testFindUnusedTags() + { + $builder = new ContainerBuilder(); + $builder + ->register('foo', 'Bar\FooClass') + ->addTag('kernel.event_listener', array('foo' => 'foo')) + ->addTag('kenrel.event_listener', array('bar' => 'bar')) + ; + $builder->findTaggedServiceIds('kernel.event_listener'); + $this->assertEquals(array('kenrel.event_listener'), $builder->findUnusedTags(), '->findUnusedTags() returns an array with unused tags'); + } + /** * @covers Symfony\Component\DependencyInjection\ContainerBuilder::findDefinition */ From d3271e1fae8614d240578d0e3a723a280c790298 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 28 Sep 2015 12:16:05 +0200 Subject: [PATCH 2/3] missing tags in whitelist --- .../DependencyInjection/Compiler/UnusedTagsPass.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php index 262017f2a4..61722072e4 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php @@ -28,6 +28,7 @@ class UnusedTagsPass implements CompilerPassInterface */ protected $whitelist = array( "console.command", + "config_cache.resource_checker", "data_collector", "form.type", "form.type_extension", @@ -38,10 +39,13 @@ class UnusedTagsPass implements CompilerPassInterface "kernel.event_subscriber", "kernel.fragment_renderer", "monolog.logger", + "routing.expression_language_provider", "routing.loader", + "security.expression_language_provider", "security.remember_me_aware", "security.voter", "serializer.encoder", + "serializer.normalizer", "templating.helper", "translation.dumper", "translation.extractor", From 95c9f500e9427c147586835ffba8f60b77f4262d Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Mon, 28 Sep 2015 13:00:47 +0200 Subject: [PATCH 3/3] added some tests --- .../Compiler/UnusedTagsPass.php | 71 +++++++++---------- .../FrameworkBundle/FrameworkBundle.php | 2 +- .../Compiler/UnusedTagsPassTest.php | 52 ++++++++++++++ .../Compiler/LoggingFormatter.php | 2 +- .../DependencyInjection/ContainerBuilder.php | 11 ++- 5 files changed, 92 insertions(+), 46 deletions(-) create mode 100644 src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/UnusedTagsPassTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php index 61722072e4..ed852fd041 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php @@ -21,69 +21,66 @@ use Symfony\Component\DependencyInjection\ContainerBuilder; */ class UnusedTagsPass implements CompilerPassInterface { - /** - * whitelisted tags - * - * @var array - */ - protected $whitelist = array( - "console.command", - "config_cache.resource_checker", - "data_collector", - "form.type", - "form.type_extension", - "form.type_guesser", - "kernel.cache_clearer", - "kernel.cache_warmer", - "kernel.event_listener", - "kernel.event_subscriber", - "kernel.fragment_renderer", - "monolog.logger", - "routing.expression_language_provider", - "routing.loader", - "security.expression_language_provider", - "security.remember_me_aware", - "security.voter", - "serializer.encoder", - "serializer.normalizer", - "templating.helper", - "translation.dumper", - "translation.extractor", - "translation.loader", - "twig.extension", - "twig.loader", - "validator.constraint_validator", - "validator.initializer", + private $whitelist = array( + 'console.command', + 'config_cache.resource_checker', + 'data_collector', + 'form.type', + 'form.type_extension', + 'form.type_guesser', + 'kernel.cache_clearer', + 'kernel.cache_warmer', + 'kernel.event_listener', + 'kernel.event_subscriber', + 'kernel.fragment_renderer', + 'monolog.logger', + 'routing.expression_language_provider', + 'routing.loader', + 'security.expression_language_provider', + 'security.remember_me_aware', + 'security.voter', + 'serializer.encoder', + 'serializer.normalizer', + 'templating.helper', + 'translation.dumper', + 'translation.extractor', + 'translation.loader', + 'twig.extension', + 'twig.loader', + 'validator.constraint_validator', + 'validator.initializer', ); public function process(ContainerBuilder $container) { $compiler = $container->getCompiler(); $formatter = $compiler->getLoggingFormatter(); - $tags = $container->findTags(); + $tags = array_unique(array_merge($container->findTags(), $this->whitelist)); - $unusedTags = $container->findUnusedTags(); - foreach ($unusedTags as $tag) { + foreach ($container->findUnusedTags() as $tag) { // skip whitelisted tags if (in_array($tag, $this->whitelist)) { continue; } + // check for typos $candidates = array(); foreach ($tags as $definedTag) { if ($definedTag === $tag) { continue; } + if (false !== strpos($definedTag, $tag) || levenshtein($tag, $definedTag) <= strlen($tag) / 3) { $candidates[] = $definedTag; } } $services = array_keys($container->findTaggedServiceIds($tag)); - $message = sprintf('Tag "%s" was defined on the service(s) %s, but was never used.', $tag, implode(',', $services)); + $message = sprintf('Tag "%s" was defined on service(s) "%s", but was never used.', $tag, implode('", "', $services)); if (!empty($candidates)) { $message .= sprintf(' Did you mean "%s"?', implode('", "', $candidates)); } + $compiler->addLogMessage($formatter->format($this, $message)); } } diff --git a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php index 1cdf83df5d..3a96f71e11 100644 --- a/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php +++ b/src/Symfony/Bundle/FrameworkBundle/FrameworkBundle.php @@ -90,9 +90,9 @@ class FrameworkBundle extends Bundle $container->addCompilerPass(new TranslationDumperPass()); $container->addCompilerPass(new FragmentRendererPass(), PassConfig::TYPE_AFTER_REMOVING); $container->addCompilerPass(new SerializerPass()); - $container->addCompilerPass(new UnusedTagsPass(), PassConfig::TYPE_AFTER_REMOVING); if ($container->getParameter('kernel.debug')) { + $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 ConfigCachePass()); diff --git a/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/UnusedTagsPassTest.php b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/UnusedTagsPassTest.php new file mode 100644 index 0000000000..f354007bb9 --- /dev/null +++ b/src/Symfony/Bundle/FrameworkBundle/Tests/DependencyInjection/Compiler/UnusedTagsPassTest.php @@ -0,0 +1,52 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Bundle\FrameworkBundle\Tests\DependencyInjection\Compiler; + +use Symfony\Bundle\FrameworkBundle\DependencyInjection\Compiler\UnusedTagsPass; + +class UnusedTagsPassTest extends \PHPUnit_Framework_TestCase +{ + public function testProcess() + { + $pass = new UnusedTagsPass(); + + $formatter = $this->getMock('Symfony\Component\DependencyInjection\Compiler\LoggingFormatter'); + $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->getMock('Symfony\Component\DependencyInjection\Compiler\Compiler'); + $compiler->expects($this->once())->method('getLoggingFormatter')->will($this->returnValue($formatter)); + + $container = $this->getMock('Symfony\Component\DependencyInjection\ContainerBuilder', + array('findTaggedServiceIds', 'getCompiler', 'findUnusedTags', 'findTags') + ); + $container->expects($this->once())->method('getCompiler')->will($this->returnValue($compiler)); + $container->expects($this->once()) + ->method('findTags') + ->will($this->returnValue(array('kenrel.event_subscriber'))); + $container->expects($this->once()) + ->method('findUnusedTags') + ->will($this->returnValue(array('kenrel.event_subscriber', 'form.type'))); + $container->expects($this->once()) + ->method('findTaggedServiceIds') + ->with('kenrel.event_subscriber') + ->will($this->returnValue(array( + 'foo' => array(), + 'bar' => array(), + ))); + + $pass->process($container); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Compiler/LoggingFormatter.php b/src/Symfony/Component/DependencyInjection/Compiler/LoggingFormatter.php index 6bd6161ceb..db208fa0d6 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/LoggingFormatter.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/LoggingFormatter.php @@ -20,7 +20,7 @@ class LoggingFormatter { public function formatRemoveService(CompilerPassInterface $pass, $id, $reason) { - return $this->format($pass, sprintf('Removed service "%s"; reason: %s', $id, $reason)); + return $this->format($pass, sprintf('Removed service "%s"; reason: %s.', $id, $reason)); } public function formatInlineService(CompilerPassInterface $pass, $id, $target) diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php index ca8a136d25..e3a6ea744c 100644 --- a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php +++ b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php @@ -91,7 +91,7 @@ class ContainerBuilder extends Container implements TaggedContainerInterface private $expressionLanguageProviders = array(); /** - * @var array with tag names used by findTaggedServiceIds + * @var string[] with tag names used by findTaggedServiceIds */ private $usedTags = array(); @@ -1096,16 +1096,13 @@ class ContainerBuilder extends Container implements TaggedContainerInterface } /** - * Returns all tags not queried by findTaggedServiceIds + * Returns all tags not queried by findTaggedServiceIds. * - * @return array An array of tags + * @return string[] An array of tags */ public function findUnusedTags() { - $tags = array_values(array_diff($this->findTags(), $this->usedTags)); - $tags = array_unique($tags); - - return $tags; + return array_values(array_diff($this->findTags(), $this->usedTags)); } public function addExpressionLanguageProvider(ExpressionFunctionProviderInterface $provider)