From 3e80e461a9057d52c88fa7f9808bf1da29e851d4 Mon Sep 17 00:00:00 2001 From: Thomas Calvet Date: Thu, 16 Apr 2020 11:22:06 +0200 Subject: [PATCH] [DependencyInjection] Add a mechanism to deprecate public services to private --- .../Compiler/UnusedTagsPass.php | 1 + .../DependencyInjection/CHANGELOG.md | 1 + .../AliasDeprecatedPublicServicesPass.php | 72 ++++++++++++++++++ .../Compiler/PassConfig.php | 1 + .../AliasDeprecatedPublicServicesPassTest.php | 73 +++++++++++++++++++ .../Tests/ContainerBuilderTest.php | 38 ++++++++++ .../Tests/Dumper/PhpDumperTest.php | 48 ++++++++++++ 7 files changed, 234 insertions(+) create mode 100644 src/Symfony/Component/DependencyInjection/Compiler/AliasDeprecatedPublicServicesPass.php create mode 100644 src/Symfony/Component/DependencyInjection/Tests/Compiler/AliasDeprecatedPublicServicesPassTest.php diff --git a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php index 4c6c5e834e..95ec917acc 100644 --- a/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php +++ b/src/Symfony/Bundle/FrameworkBundle/DependencyInjection/Compiler/UnusedTagsPass.php @@ -34,6 +34,7 @@ class UnusedTagsPass implements CompilerPassInterface 'container.hot_path', 'container.no_preload', 'container.preload', + 'container.private', 'container.reversible', 'container.service_locator', 'container.service_locator_context', diff --git a/src/Symfony/Component/DependencyInjection/CHANGELOG.md b/src/Symfony/Component/DependencyInjection/CHANGELOG.md index f14c71bfbe..62604dd5de 100644 --- a/src/Symfony/Component/DependencyInjection/CHANGELOG.md +++ b/src/Symfony/Component/DependencyInjection/CHANGELOG.md @@ -21,6 +21,7 @@ CHANGELOG * deprecated `Alias::getDeprecationMessage()`, use `Alias::getDeprecation()` instead * deprecated PHP-DSL's `inline()` function, use `service()` instead * added support of PHP8 static return type for withers + * added `AliasDeprecatedPublicServicesPass` to deprecate public services to private 5.0.0 ----- diff --git a/src/Symfony/Component/DependencyInjection/Compiler/AliasDeprecatedPublicServicesPass.php b/src/Symfony/Component/DependencyInjection/Compiler/AliasDeprecatedPublicServicesPass.php new file mode 100644 index 0000000000..802c407662 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Compiler/AliasDeprecatedPublicServicesPass.php @@ -0,0 +1,72 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\Component\DependencyInjection\Compiler; + +use Symfony\Component\DependencyInjection\ContainerBuilder; +use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException; +use Symfony\Component\DependencyInjection\Reference; + +final class AliasDeprecatedPublicServicesPass extends AbstractRecursivePass +{ + private $tagName; + + private $aliases = []; + + public function __construct(string $tagName = 'container.private') + { + $this->tagName = $tagName; + } + + /** + * {@inheritdoc} + */ + protected function processValue($value, bool $isRoot = false) + { + if ($value instanceof Reference && isset($this->aliases[$id = (string) $value])) { + return new Reference($this->aliases[$id], $value->getInvalidBehavior()); + } + + return parent::processValue($value, $isRoot); + } + + /** + * {@inheritdoc} + */ + public function process(ContainerBuilder $container) + { + foreach ($container->findTaggedServiceIds($this->tagName) as $id => $tags) { + if (null === $package = $tags[0]['package'] ?? null) { + throw new InvalidArgumentException(sprintf('The "package" attribute is mandatory for the "%s" tag on the "%s" service.', $this->tagName, $id)); + } + + if (null === $version = $tags[0]['version'] ?? null) { + throw new InvalidArgumentException(sprintf('The "version" attribute is mandatory for the "%s" tag on the "%s" service.', $this->tagName, $id)); + } + + $definition = $container->getDefinition($id); + if (!$definition->isPublic() || $definition->isPrivate()) { + throw new InvalidArgumentException(sprintf('The "%s" service is private: it cannot have the "%s" tag.', $id, $this->tagName)); + } + + $container + ->setAlias($id, $aliasId = '.'.$this->tagName.'.'.$id) + ->setPublic(true) + ->setDeprecated($package, $version, 'Accessing the "%alias_id%" service directly from the container is deprecated, use dependency injection instead.'); + + $container->setDefinition($aliasId, $definition); + + $this->aliases[$id] = $aliasId; + } + + parent::process($container); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php index 03d4a57d52..245c3b539c 100644 --- a/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php +++ b/src/Symfony/Component/DependencyInjection/Compiler/PassConfig.php @@ -94,6 +94,7 @@ class PassConfig new CheckExceptionOnInvalidReferenceBehaviorPass(), new ResolveHotPathPass(), new ResolveNoPreloadPass(), + new AliasDeprecatedPublicServicesPass(), ]]; } diff --git a/src/Symfony/Component/DependencyInjection/Tests/Compiler/AliasDeprecatedPublicServicesPassTest.php b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AliasDeprecatedPublicServicesPassTest.php new file mode 100644 index 0000000000..722cb4f6b7 --- /dev/null +++ b/src/Symfony/Component/DependencyInjection/Tests/Compiler/AliasDeprecatedPublicServicesPassTest.php @@ -0,0 +1,73 @@ +register('foo') + ->setPublic(true) + ->addTag('container.private', ['package' => 'foo/bar', 'version' => '1.2']); + + (new AliasDeprecatedPublicServicesPass())->process($container); + + $this->assertTrue($container->hasAlias('foo')); + + $alias = $container->getAlias('foo'); + + $this->assertSame('.container.private.foo', (string) $alias); + $this->assertTrue($alias->isPublic()); + $this->assertFalse($alias->isPrivate()); + $this->assertSame([ + 'package' => 'foo/bar', + 'version' => '1.2', + 'message' => 'Accessing the "foo" service directly from the container is deprecated, use dependency injection instead.', + ], $alias->getDeprecation('foo')); + } + + /** + * @dataProvider processWithMissingAttributeProvider + */ + public function testProcessWithMissingAttribute(string $attribute, array $attributes) + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage(sprintf('The "%s" attribute is mandatory for the "container.private" tag on the "foo" service.', $attribute)); + + $container = new ContainerBuilder(); + $container + ->register('foo') + ->addTag('container.private', $attributes); + + (new AliasDeprecatedPublicServicesPass())->process($container); + } + + public function processWithMissingAttributeProvider() + { + return [ + ['package', ['version' => '1.2']], + ['version', ['package' => 'foo/bar']], + ]; + } + + public function testProcessWithNonPublicService() + { + $this->expectException(InvalidArgumentException::class); + $this->expectExceptionMessage('The "foo" service is private: it cannot have the "container.private" tag.'); + + $container = new ContainerBuilder(); + $container + ->register('foo') + ->setPublic(false) + ->addTag('container.private', ['package' => 'foo/bar', 'version' => '1.2']); + + (new AliasDeprecatedPublicServicesPass())->process($container); + } +} diff --git a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php index 14cca64920..daf4ed7456 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/ContainerBuilderTest.php @@ -1661,6 +1661,44 @@ class ContainerBuilderTest extends TestCase $this->assertInstanceOf(D::class, $container->get(X::class)); } + + /** + * @group legacy + */ + public function testDirectlyAccessingDeprecatedPublicService() + { + $this->expectDeprecation('Since foo/bar 3.8: Accessing the "Symfony\Component\DependencyInjection\Tests\A" service directly from the container is deprecated, use dependency injection instead.'); + + $container = new ContainerBuilder(); + $container + ->register(A::class) + ->setPublic(true) + ->addTag('container.private', ['package' => 'foo/bar', 'version' => '3.8']); + + $container->compile(); + + $container->get(A::class); + } + + public function testReferencingDeprecatedPublicService() + { + $container = new ContainerBuilder(); + $container + ->register(A::class) + ->setPublic(true) + ->addTag('container.private', ['package' => 'foo/bar', 'version' => '3.8']); + $container + ->register(B::class) + ->setPublic(true) + ->addArgument(new Reference(A::class)); + + $container->compile(); + + // No deprecation should be triggered. + $container->get(B::class); + + $this->addToAssertionCount(1); + } } class FooClass diff --git a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php index 108f5ad443..93000ab82e 100644 --- a/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php +++ b/src/Symfony/Component/DependencyInjection/Tests/Dumper/PhpDumperTest.php @@ -1429,6 +1429,54 @@ class PhpDumperTest extends TestCase $dumper = new PhpDumper($container); $dumper->dump(); } + + /** + * @group legacy + */ + public function testDirectlyAccessingDeprecatedPublicService() + { + $this->expectDeprecation('Since foo/bar 3.8: Accessing the "bar" service directly from the container is deprecated, use dependency injection instead.'); + + $container = new ContainerBuilder(); + $container + ->register('bar', \BarClass::class) + ->setPublic(true) + ->addTag('container.private', ['package' => 'foo/bar', 'version' => '3.8']); + + $container->compile(); + + $dumper = new PhpDumper($container); + eval('?>'.$dumper->dump(['class' => 'Symfony_DI_PhpDumper_Test_Directly_Accessing_Deprecated_Public_Service'])); + + $container = new \Symfony_DI_PhpDumper_Test_Directly_Accessing_Deprecated_Public_Service(); + + $container->get('bar'); + } + + public function testReferencingDeprecatedPublicService() + { + $container = new ContainerBuilder(); + $container + ->register('bar', \BarClass::class) + ->setPublic(true) + ->addTag('container.private', ['package' => 'foo/bar', 'version' => '3.8']); + $container + ->register('bar_user', \BarUserClass::class) + ->setPublic(true) + ->addArgument(new Reference('bar')); + + $container->compile(); + + $dumper = new PhpDumper($container); + eval('?>'.$dumper->dump(['class' => 'Symfony_DI_PhpDumper_Test_Referencing_Deprecated_Public_Service'])); + + $container = new \Symfony_DI_PhpDumper_Test_Referencing_Deprecated_Public_Service(); + + // No deprecation should be triggered. + $container->get('bar_user'); + + $this->addToAssertionCount(1); + } } class Rot13EnvVarProcessor implements EnvVarProcessorInterface