From 1775bba68fbe8d414698e8f45dbba07251dcf3e4 Mon Sep 17 00:00:00 2001 From: Nicolas Grekas Date: Sun, 19 Nov 2017 21:58:42 +0100 Subject: [PATCH] [DI] Skip hot_path tag for deprecated services as their class might also be --- .../Compiler/ResolveHotPathPass.php | 6 +++--- .../DependencyInjection/Dumper/PhpDumper.php | 21 ++++++++++++------- .../Tests/Compiler/ResolveHotPathPassTest.php | 14 +++++++++---- 3 files changed, 26 insertions(+), 15 deletions(-) diff --git a/src/Symfony/Component/DependencyInjection/Compiler/ResolveHotPathPass.php b/src/Symfony/Component/DependencyInjection/Compiler/ResolveHotPathPass.php index e9147f52421b8..38e96d06f1a76 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/ResolveHotPathPass.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/ResolveHotPathPass.php @@ -52,12 +52,12 @@ protected function processValue($value, $isRoot = false) if ($value instanceof ArgumentInterface) { return $value; } - if ($value instanceof Definition && $isRoot && (isset($this->resolvedIds[$this->currentId]) || !$value->hasTag($this->tagName))) { - return $value; + if ($value instanceof Definition && $isRoot && (isset($this->resolvedIds[$this->currentId]) || !$value->hasTag($this->tagName) || $value->isDeprecated())) { + return $value->isDeprecated() ? $value->clearTag($this->tagName) : $value; } if ($value instanceof Reference && ContainerBuilder::IGNORE_ON_UNINITIALIZED_REFERENCE !== $value->getInvalidBehavior() && $this->container->has($id = (string) $value)) { $definition = $this->container->findDefinition($id); - if (!$definition->hasTag($this->tagName)) { + if (!$definition->hasTag($this->tagName) && !$definition->isDeprecated()) { $this->resolvedIds[$id] = true; $definition->addTag($this->tagName); parent::processValue($definition, false); diff --git a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php index db1f98615b818..6d41d7095d353 100644 --- a/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php +++ b/src/Symfony/Component/DependencyInjection/Dumper/PhpDumper.php @@ -271,11 +271,11 @@ private function addServiceLocalTempVariables($cId, Definition $definition, arra array_unshift($inlinedDefinitions, $definition); - $collectLineage = $this->inlineRequires && !($this->hotPathTag && $definition->hasTag($this->hotPathTag)); + $collectLineage = $this->inlineRequires && !$this->isHotPath($definition); $isNonLazyShared = !$this->getProxyDumper()->isProxyCandidate($definition) && $definition->isShared(); $lineage = $calls = $behavior = array(); foreach ($inlinedDefinitions as $iDefinition) { - if ($collectLineage && $class = is_array($factory = $iDefinition->getFactory()) && is_string($factory[0]) ? $factory[0] : $iDefinition->getClass()) { + if ($collectLineage && !$iDefinition->isDeprecated() && $class = is_array($factory = $iDefinition->getFactory()) && is_string($factory[0]) ? $factory[0] : $iDefinition->getClass()) { $this->collectLineage($class, $lineage); } $this->getServiceCallsFromArguments($iDefinition->getArguments(), $calls, $behavior, $isNonLazyShared); @@ -763,7 +763,7 @@ private function addService($id, Definition $definition, &$file = null) $lazyInitialization = ''; } - $asFile = $this->asFiles && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag)); + $asFile = $this->asFiles && $definition->isShared() && !$this->isHotPath($definition); $methodName = $this->generateMethodName($id); if ($asFile) { $file = $methodName.'.php'; @@ -829,7 +829,7 @@ private function addServices() $definitions = $this->container->getDefinitions(); ksort($definitions); foreach ($definitions as $id => $definition) { - if ($definition->isSynthetic() || ($this->asFiles && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag)))) { + if ($definition->isSynthetic() || ($this->asFiles && $definition->isShared() && !$this->isHotPath($definition))) { continue; } if ($definition->isPublic()) { @@ -847,7 +847,7 @@ private function generateServiceFiles() $definitions = $this->container->getDefinitions(); ksort($definitions); foreach ($definitions as $id => $definition) { - if (!$definition->isSynthetic() && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag))) { + if (!$definition->isSynthetic() && $definition->isShared() && !$this->isHotPath($definition)) { $code = $this->addService($id, $definition, $file); yield $file => $code; } @@ -1120,7 +1120,7 @@ private function addMethodMap() $definitions = $this->container->getDefinitions(); ksort($definitions); foreach ($definitions as $id => $definition) { - if (!$definition->isSynthetic() && (!$this->asFiles || !$definition->isShared() || ($this->hotPathTag && $definition->hasTag($this->hotPathTag)))) { + if (!$definition->isSynthetic() && (!$this->asFiles || !$definition->isShared() || $this->isHotPath($definition))) { $code .= ' '.$this->export($id).' => '.$this->export($this->generateMethodName($id)).",\n"; } } @@ -1139,7 +1139,7 @@ private function addFileMap() $definitions = $this->container->getDefinitions(); ksort($definitions); foreach ($definitions as $id => $definition) { - if (!$definition->isSynthetic() && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag))) { + if (!$definition->isSynthetic() && $definition->isShared() && !$this->isHotPath($definition)) { $code .= sprintf(" %s => __DIR__.'/%s.php',\n", $this->export($id), $this->generateMethodName($id)); } } @@ -1856,7 +1856,7 @@ private function getServiceCall($id, Reference $reference = null) if ($definition->isShared()) { $code = sprintf('$this->services[\'%s\'] = %s', $id, $code); } - } elseif ($this->asFiles && $definition->isShared() && !($this->hotPathTag && $definition->hasTag($this->hotPathTag))) { + } elseif ($this->asFiles && $definition->isShared() && !$this->isHotPath($definition)) { $code = sprintf("\$this->load(__DIR__.'/%s.php')", $this->generateMethodName($id)); } else { $code = sprintf('$this->%s()', $this->generateMethodName($id)); @@ -1988,6 +1988,11 @@ private function getExpressionLanguage() return $this->expressionLanguage; } + private function isHotPath(Definition $definition) + { + return $this->hotPathTag && $definition->hasTag($this->hotPathTag) && !$definition->isDeprecated(); + } + private function export($value) { if (null !== $this->targetDirRegex && is_string($value) && preg_match($this->targetDirRegex, $value, $matches, PREG_OFFSET_CAPTURE)) { diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveHotPathPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveHotPathPassTest.php index 33176159cc945..d2ba590047caa 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveHotPathPassTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/ResolveHotPathPassTest.php @@ -34,11 +34,15 @@ public function testProcess() ; $container->register('lazy'); - $container->register('bar'); - $container->register('bar')->addArgument(new Reference('buz')); - $container->register('baz')->addArgument(new Reference('lazy')); - $container->register('baz')->addArgument(new Reference('lazy')); + $container->register('bar') + ->addArgument(new Reference('buz')) + ->addArgument(new Reference('deprec_ref_notag')); + $container->register('baz') + ->addArgument(new Reference('lazy')) + ->addArgument(new Reference('lazy')); $container->register('buz'); + $container->register('deprec_with_tag')->setDeprecated()->addTag('container.hot_path'); + $container->register('deprec_ref_notag')->setDeprecated(); (new ResolveHotPathPass())->process($container); @@ -47,5 +51,7 @@ public function testProcess() $this->assertTrue($container->getDefinition('buz')->hasTag('container.hot_path')); $this->assertFalse($container->getDefinition('baz')->hasTag('container.hot_path')); $this->assertFalse($container->getDefinition('service_container')->hasTag('container.hot_path')); + $this->assertFalse($container->getDefinition('deprec_with_tag')->hasTag('container.hot_path')); + $this->assertFalse($container->getDefinition('deprec_ref_notag')->hasTag('container.hot_path')); } }