Make as many services private as possible

nicolas-grekas · nicolas-grekas · commit 1a9e40283bc9 · 2017-09-13T09:59:43.000+02:00
diff --git a/Alias.php b/Alias.php
@@ -15,6 +15,7 @@ class Alias
 {
     private $id;
     private $public;
+    private $private = false;
 
     /**
      * @param string $id     Alias identifier
@@ -40,10 +41,43 @@ public function isPublic()
      * Sets if this Alias is public.
      *
      * @param bool $boolean If this Alias should be public
+     *
+     * @return $this
      */
     public function setPublic($boolean)
     {
         $this->public = (bool) $boolean;
+
+        return $this;
+    }
+
+    /**
+     * Sets if this Alias is private.
+     *
+     * When set, the "private" state has a higher precedence than "public".
+     * In version 3.4, a "private" alias always remains publicly accessible,
+     * but triggers a deprecation notice when accessed from the container,
+     * so that the alias can be made really private in 4.0.
+     *
+     * @param bool $boolean
+     *
+     * @return $this
+     */
+    public function setPrivate($boolean)
+    {
+        $this->private = (bool) $boolean;
+
+        return $this;
+    }
+
+    /**
+     * Whether this alias is private.
+     *
+     * @return bool
+     */
+    public function isPrivate()
+    {
+        return $this->private;
     }
 
     /**
diff --git a/Compiler/CheckDefinitionValidityPass.php b/Compiler/CheckDefinitionValidityPass.php
@@ -39,7 +39,7 @@ public function process(ContainerBuilder $container)
     {
         foreach ($container->getDefinitions() as $id => $definition) {
             // synthetic service is public
-            if ($definition->isSynthetic() && !$definition->isPublic()) {
+            if ($definition->isSynthetic() && (!$definition->isPublic() || $definition->isPrivate())) {
                 throw new RuntimeException(sprintf('A synthetic service ("%s") must be public.', $id));
             }
 
diff --git a/Compiler/DecoratorServicePass.php b/Compiler/DecoratorServicePass.php
@@ -49,6 +49,7 @@ public function process(ContainerBuilder $container)
             if ($container->hasAlias($inner)) {
                 $alias = $container->getAlias($inner);
                 $public = $alias->isPublic();
+                $private = $alias->isPrivate();
                 $container->setAlias($renamedId, new Alias((string) $alias, false));
             } else {
                 $decoratedDefinition = $container->getDefinition($inner);
@@ -57,6 +58,7 @@ public function process(ContainerBuilder $container)
                     $definition->setAutowiringTypes($types);
                 }
                 $public = $decoratedDefinition->isPublic();
+                $private = $decoratedDefinition->isPrivate();
                 $decoratedDefinition->setPublic(false);
                 $decoratedDefinition->setTags(array());
                 if ($decoratedDefinition->getAutowiringTypes(false)) {
@@ -65,7 +67,7 @@ public function process(ContainerBuilder $container)
                 $container->setDefinition($renamedId, $decoratedDefinition);
             }
 
-            $container->setAlias($inner, new Alias($id, $public));
+            $container->setAlias($inner, $id)->setPublic($public && !$private)->setPrivate($private);
         }
     }
 }
diff --git a/Compiler/InlineServiceDefinitionsPass.php b/Compiler/InlineServiceDefinitionsPass.php
@@ -82,7 +82,7 @@ private function isInlineableDefinition($id, Definition $definition, ServiceRefe
             return true;
         }
 
-        if ($definition->isDeprecated() || $definition->isPublic() || $definition->isLazy()) {
+        if ($definition->isDeprecated() || $definition->isPublic() || $definition->isPrivate() || $definition->isLazy()) {
             return false;
         }
 
diff --git a/Compiler/RemovePrivateAliasesPass.php b/Compiler/RemovePrivateAliasesPass.php
@@ -30,7 +30,7 @@ class RemovePrivateAliasesPass implements CompilerPassInterface
     public function process(ContainerBuilder $container)
     {
         foreach ($container->getAliases() as $id => $alias) {
-            if ($alias->isPublic()) {
+            if ($alias->isPublic() || $alias->isPrivate()) {
                 continue;
             }
 
diff --git a/Compiler/RemoveUnusedDefinitionsPass.php b/Compiler/RemoveUnusedDefinitionsPass.php
@@ -41,7 +41,7 @@ public function process(ContainerBuilder $container)
 
         $hasChanged = false;
         foreach ($container->getDefinitions() as $id => $definition) {
-            if ($definition->isPublic()) {
+            if ($definition->isPublic() || $definition->isPrivate()) {
                 continue;
             }
 
@@ -68,7 +68,8 @@ public function process(ContainerBuilder $container)
 
             if (1 === count($referencingAliases) && false === $isReferenced) {
                 $container->setDefinition((string) reset($referencingAliases), $definition);
-                $definition->setPublic(true);
+                $definition->setPrivate(reset($referencingAliases)->isPrivate());
+                $definition->setPublic(!$definition->isPrivate());
                 $container->removeDefinition($id);
                 $container->log($this, sprintf('Removed service "%s"; reason: replaces alias %s.', $id, reset($referencingAliases)));
             } elseif (0 === count($referencingAliases) && false === $isReferenced) {
diff --git a/Compiler/ReplaceAliasByActualDefinitionPass.php b/Compiler/ReplaceAliasByActualDefinitionPass.php
@@ -58,11 +58,12 @@ public function process(ContainerBuilder $container)
             } catch (InvalidArgumentException $e) {
                 throw new InvalidArgumentException(sprintf('Unable to replace alias "%s" with actual definition "%s".', $definitionId, $targetId), null, $e);
             }
-            if ($definition->isPublic()) {
+            if ($definition->isPublic() || $definition->isPrivate()) {
                 continue;
             }
             // Remove private definition and schedule for replacement
-            $definition->setPublic(true);
+            $definition->setPublic(!$target->isPrivate());
+            $definition->setPrivate($target->isPrivate());
             $container->setDefinition($definitionId, $definition);
             $container->removeDefinition($targetId);
             $replacements[$targetId] = $definitionId;
diff --git a/Compiler/ResolveChildDefinitionsPass.php b/Compiler/ResolveChildDefinitionsPass.php
@@ -12,6 +12,7 @@
 namespace Symfony\Component\DependencyInjection\Compiler;
 
 use Symfony\Component\DependencyInjection\ChildDefinition;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Exception\ExceptionInterface;
 use Symfony\Component\DependencyInjection\Exception\RuntimeException;
@@ -25,6 +26,26 @@
  */
 class ResolveChildDefinitionsPass extends AbstractRecursivePass
 {
+    /**
+     * {@inheritdoc}
+     */
+    public function process(ContainerBuilder $container)
+    {
+        parent::process($container);
+
+        foreach ($container->getDefinitions() as $definition) {
+            if ($definition->isPrivate()) {
+                $definition->setPublic(false);
+            }
+        }
+
+        foreach ($container->getAliases() as $alias) {
+            if ($alias->isPrivate()) {
+                $alias->setPublic(false);
+            }
+        }
+    }
+
     protected function processValue($value, $isRoot = false)
     {
         if (!$value instanceof Definition) {
@@ -121,6 +142,8 @@ private function doResolveDefinition(ChildDefinition $definition)
         }
         if (isset($changes['public'])) {
             $def->setPublic($definition->isPublic());
+        } else {
+            $def->setPrivate($definition->isPrivate() || $parentDef->isPrivate());
         }
         if (isset($changes['lazy'])) {
             $def->setLazy($definition->isLazy());
diff --git a/Compiler/ResolveReferencesToAliasesPass.php b/Compiler/ResolveReferencesToAliasesPass.php
@@ -51,7 +51,7 @@ public function process(ContainerBuilder $container)
         foreach ($container->getAliases() as $id => $alias) {
             $aliasId = (string) $alias;
             if ($aliasId !== $defId = $this->getDefinitionId($aliasId)) {
-                $container->setAlias($id, new Alias($defId, $alias->isPublic()));
+                $container->setAlias($id, $defId)->setPublic($alias->isPublic() && !$alias->isPrivate())->setPrivate($alias->isPrivate());
             }
         }
     }
diff --git a/ContainerBuilder.php b/ContainerBuilder.php
@@ -806,6 +806,8 @@ public function setAliases(array $aliases)
      * @param string       $alias The alias to create
      * @param string|Alias $id    The service to alias
      *
+     * @return Alias
+     *
      * @throws InvalidArgumentException if the id is not a string or an Alias
      * @throws InvalidArgumentException if the alias is for itself
      */
@@ -825,7 +827,7 @@ public function setAlias($alias, $id)
 
         unset($this->definitions[$alias]);
 
-        $this->aliasDefinitions[$alias] = $id;
+        return $this->aliasDefinitions[$alias] = $id;
     }
 
     /**
diff --git a/Definition.php b/Definition.php
@@ -35,6 +35,7 @@ class Definition
     private $configurator;
     private $tags = array();
     private $public = true;
+    private $private = false;
     private $synthetic = false;
     private $abstract = false;
     private $lazy = false;
@@ -616,6 +617,35 @@ public function isPublic()
         return $this->public;
     }
 
+    /**
+     * Sets if this service is private.
+     *
+     * When set, the "private" state has a higher precedence than "public".
+     * In version 3.4, a "private" service always remains publicly accessible,
+     * but triggers a deprecation notice when accessed from the container,
+     * so that the service can be made really private in 4.0.
+     *
+     * @param bool $boolean
+     *
+     * @return $this
+     */
+    public function setPrivate($boolean)
+    {
+        $this->private = (bool) $boolean;
+
+        return $this;
+    }
+
+    /**
+     * Whether this service is private.
+     *
+     * @return bool
+     */
+    public function isPrivate()
+    {
+        return $this->private;
+    }
+
     /**
      * Sets the lazy flag of this service.
      *
diff --git a/Dumper/PhpDumper.php b/Dumper/PhpDumper.php
@@ -1040,11 +1040,17 @@ private function addFileMap()
      */
     private function addPrivateServices()
     {
-        if (!$definitions = $this->container->getDefinitions()) {
-            return '';
+        $code = '';
+
+        $aliases = $this->container->getAliases();
+        ksort($aliases);
+        foreach ($aliases as $id => $alias) {
+            if ($alias->isPrivate()) {
+                $code .= '            '.$this->export($id)." => true,\n";
+            }
         }
 
-        $code = '';
+        $definitions = $this->container->getDefinitions();
         ksort($definitions);
         foreach ($definitions as $id => $definition) {
             if (!$definition->isPublic()) {
diff --git a/Tests/Compiler/ResolveChildDefinitionsPassTest.php b/Tests/Compiler/ResolveChildDefinitionsPassTest.php
@@ -397,6 +397,26 @@ public function testSetAutoconfiguredOnServiceIsParent()
         $this->assertFalse($container->getDefinition('child1')->isAutoconfigured());
     }
 
+    public function testPrivateHasHigherPrecedenceThanPublic()
+    {
+        $container = new ContainerBuilder();
+
+        $container->register('foo', 'stdClass')
+            ->setPrivate(true)
+            ->setPublic(true)
+        ;
+
+        $container->setAlias('bar', 'foo')
+            ->setPrivate(false)
+            ->setPublic(false)
+        ;
+
+        $this->process($container);
+
+        $this->assertFalse($container->getDefinition('foo')->isPublic());
+        $this->assertFalse($container->getAlias('bar')->isPublic());
+    }
+
     /**
      * @group legacy
      */
diff --git a/Tests/Dumper/PhpDumperTest.php b/Tests/Dumper/PhpDumperTest.php
@@ -778,6 +778,52 @@ public function testDumpHandlesLiteralClassWithRootNamespace()
         $this->assertInstanceOf('stdClass', $container->get('foo'));
     }
 
+    /**
+     * @group legacy
+     * @expectedDeprecation Requesting the "private" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "private_alias" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "decorated_private" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "decorated_private_alias" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "private_not_inlined" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "private_not_removed" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "private_child" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     * @expectedDeprecation Requesting the "private_parent" private service is deprecated since Symfony 3.2 and won't be supported anymore in Symfony 4.0.
+     */
+    public function testLegacyPrivateServices()
+    {
+        $container = new ContainerBuilder();
+        $loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
+        $loader->load('services_legacy_privates.yml');
+
+        $container->getDefinition('private')->setPrivate(true);
+        $container->getDefinition('private_not_inlined')->setPrivate(true);
+        $container->getDefinition('private_not_removed')->setPrivate(true);
+        $container->getDefinition('decorated_private')->setPrivate(true);
+        $container->getDefinition('private_child')->setPrivate(true);
+        $container->getAlias('decorated_private_alias')->setPrivate(true);
+        $container->getAlias('private_alias')->setPrivate(true);
+
+        $container->compile();
+        $dumper = new PhpDumper($container);
+
+        $this->assertStringEqualsFile(self::$fixturesPath.'/php/services_legacy_privates.php', $dumper->dump(array('class' => 'Symfony_DI_PhpDumper_Test_Legacy_Privates', 'file' => self::$fixturesPath.'/php/services_legacy_privates.php')));
+
+        require self::$fixturesPath.'/php/services_legacy_privates.php';
+
+        $container = new \Symfony_DI_PhpDumper_Test_Legacy_Privates();
+
+        $container->get('private');
+        $container->get('private_alias');
+        $container->get('alias_to_private');
+        $container->get('decorated_private');
+        $container->get('decorated_private_alias');
+        $container->get('private_not_inlined');
+        $container->get('private_not_removed');
+        $container->get('private_child');
+        $container->get('private_parent');
+        $container->get('public_child');
+    }
+
     /**
      * This test checks the trigger of a deprecation note and should not be removed in major releases.
      *
diff --git a/Tests/Fixtures/php/services_legacy_privates.php b/Tests/Fixtures/php/services_legacy_privates.php
diff --git a/Tests/Fixtures/yaml/services_legacy_privates.yml b/Tests/Fixtures/yaml/services_legacy_privates.yml

Original file line number	Diff line number	Diff line change
`@@ -39,7 +39,7 @@ public function process(ContainerBuilder $container)`
`39`	`39`	`{`
`40`	`40`	`foreach ($container->getDefinitions() as $id => $definition) {`
`41`	`41`	`// synthetic service is public`
`42`		`- if ($definition->isSynthetic() && !$definition->isPublic()) {`
	`42`	`+ if ($definition->isSynthetic() && (!$definition->isPublic() \|\| $definition->isPrivate())) {`
`43`	`43`	`throw new RuntimeException(sprintf('A synthetic service ("%s") must be public.', $id));`
`44`	`44`	`}`
`45`	`45`
Original file line number	Diff line number	Diff line change
`@@ -49,6 +49,7 @@ public function process(ContainerBuilder $container)`
`49`	`49`	`if ($container->hasAlias($inner)) {`
`50`	`50`	`$alias = $container->getAlias($inner);`
`51`	`51`	`$public = $alias->isPublic();`
	`52`	`+ $private = $alias->isPrivate();`
`52`	`53`	`$container->setAlias($renamedId, new Alias((string) $alias, false));`
`53`	`54`	`} else {`
`54`	`55`	`$decoratedDefinition = $container->getDefinition($inner);`
`@@ -57,6 +58,7 @@ public function process(ContainerBuilder $container)`
`57`	`58`	`$definition->setAutowiringTypes($types);`
`58`	`59`	`}`
`59`	`60`	`$public = $decoratedDefinition->isPublic();`
	`61`	`+ $private = $decoratedDefinition->isPrivate();`
`60`	`62`	`$decoratedDefinition->setPublic(false);`
`61`	`63`	`$decoratedDefinition->setTags(array());`
`62`	`64`	`if ($decoratedDefinition->getAutowiringTypes(false)) {`
`@@ -65,7 +67,7 @@ public function process(ContainerBuilder $container)`
`65`	`67`	`$container->setDefinition($renamedId, $decoratedDefinition);`
`66`	`68`	`}`
`67`	`69`
`68`		`- $container->setAlias($inner, new Alias($id, $public));`
	`70`	`+ $container->setAlias($inner, $id)->setPublic($public && !$private)->setPrivate($private);`
`69`	`71`	`}`
`70`	`72`	`}`
`71`	`73`	`}`
Original file line number	Diff line number	Diff line change
`@@ -82,7 +82,7 @@ private function isInlineableDefinition($id, Definition $definition, ServiceRefe`
`82`	`82`	`return true;`
`83`	`83`	`}`
`84`	`84`
`85`		`- if ($definition->isDeprecated() \|\| $definition->isPublic() \|\| $definition->isLazy()) {`
	`85`	`+ if ($definition->isDeprecated() \|\| $definition->isPublic() \|\| $definition->isPrivate() \|\| $definition->isLazy()) {`
`86`	`86`	`return false;`
`87`	`87`	`}`
`88`	`88`
Original file line number	Diff line number	Diff line change
`@@ -30,7 +30,7 @@ class RemovePrivateAliasesPass implements CompilerPassInterface`
`30`	`30`	`public function process(ContainerBuilder $container)`
`31`	`31`	`{`
`32`	`32`	`foreach ($container->getAliases() as $id => $alias) {`
`33`		`- if ($alias->isPublic()) {`
	`33`	`+ if ($alias->isPublic() \|\| $alias->isPrivate()) {`
`34`	`34`	`continue;`
`35`	`35`	`}`
`36`	`36`
Original file line number	Diff line number	Diff line change
`@@ -51,7 +51,7 @@ public function process(ContainerBuilder $container)`
`51`	`51`	`foreach ($container->getAliases() as $id => $alias) {`
`52`	`52`	`$aliasId = (string) $alias;`
`53`	`53`	`if ($aliasId !== $defId = $this->getDefinitionId($aliasId)) {`
`54`		`- $container->setAlias($id, new Alias($defId, $alias->isPublic()));`
	`54`	`+ $container->setAlias($id, $defId)->setPublic($alias->isPublic() && !$alias->isPrivate())->setPrivate($alias->isPrivate());`
`55`	`55`	`}`
`56`	`56`	`}`
`57`	`57`	`}`
Original file line number	Diff line number	Diff line change
`@@ -806,6 +806,8 @@ public function setAliases(array $aliases)`
`806`	`806`	`* @param string $alias The alias to create`
`807`	`807`	`* @param string\|Alias $id The service to alias`
`808`	`808`	`*`
	`809`	`+ * @return Alias`
	`810`	`+ *`
`809`	`811`	`* @throws InvalidArgumentException if the id is not a string or an Alias`
`810`	`812`	`* @throws InvalidArgumentException if the alias is for itself`
`811`	`813`	`*/`
`@@ -825,7 +827,7 @@ public function setAlias($alias, $id)`
`825`	`827`
`826`	`828`	`unset($this->definitions[$alias]);`
`827`	`829`
`828`		`- $this->aliasDefinitions[$alias] = $id;`
	`830`	`+ return $this->aliasDefinitions[$alias] = $id;`
`829`	`831`	`}`
`830`	`832`
`831`	`833`	`/**`