Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
made the DI config validation more strict to catch errors early
  • Loading branch information
schmittjoh authored and fabpot committed Jan 24, 2011
1 parent 4408cbc commit f29a5f7
Show file tree
Hide file tree
Showing 7 changed files with 86 additions and 11 deletions.
@@ -0,0 +1,62 @@
<?php

namespace Symfony\Component\DependencyInjection\Compiler;

use Symfony\Component\DependencyInjection\ContainerInterface;
use Symfony\Component\DependencyInjection\ContainerBuilder;

/**
* This pass validates each definition individually only taking the information
* into account which is contained in the definition itself.
*
* Later passes can rely on the following, and specifically do not need to
* perform these checks themself:
*
* - non synthetic services always have a class set
* - synthetic services are always public
* - synthetic services are always of non-prototype scope
*
* @author Johannes M. Schmitt <schmittjoh@gmail.com>
*/
class CheckDefinitionValidityPass implements CompilerPassInterface
{
public function process(ContainerBuilder $container)
{
foreach ($container->getDefinitions() as $id => $definition) {
// synthetic service is public
if ($definition->isSynthetic() && !$definition->isPublic()) {
throw new \RuntimeException(sprintf(
'A synthetic service ("%s") must be public.',
$id
));
}

// synthetic service has non-prototype scope
if ($definition->isSynthetic() && ContainerInterface::SCOPE_PROTOTYPE === $definition->getScope()) {
throw new \RuntimeException(sprintf(
'A synthetic service ("%s") cannot be of scope "prototype".',
$id
));
}

// non-synthetic service has class
if (!$definition->isSynthetic() && !$definition->getClass()) {
if ($definition->getFactoryService()) {
throw new \RuntimeException(sprintf(
'Please add the class to service "%s" even if it is constructed '
.'by a factory since we might need to add method calls based on '
.'interface injection, or other compile-time checks.',
$id
));
}

throw new \RuntimeException(sprintf(
'The definition for "%s" has no class. If you intend to inject '
.'this service dynamically at runtime, please mark it as synthetic=true, '
.'otherwise specify a class to get rid of this error.',
$id
));
}
}
}
}
Expand Up @@ -44,6 +44,10 @@ public function process(ContainerBuilder $container)
}

foreach ($container->getDefinitions() as $id => $definition) {
if ($definition->isSynthetic()) {
continue;
}

$this->currentId = $id;
$this->currentScope = $scope = $definition->getScope();

Expand Down
Expand Up @@ -43,6 +43,7 @@ public function __construct()

$this->optimizationPasses = array(
new ResolveParameterPlaceHoldersPass(),
new CheckDefinitionValidityPass(),
new ResolveReferencesToAliasesPass(),
new ResolveInterfaceInjectorsPass(),
new ResolveInvalidReferencesPass(),
Expand Down
Expand Up @@ -26,6 +26,10 @@ class ResolveInterfaceInjectorsPass implements CompilerPassInterface
public function process(ContainerBuilder $container)
{
foreach ($container->getDefinitions() as $definition) {
if ($definition->isSynthetic()) {
continue;
}

$loaded = false;
foreach ($container->getInterfaceInjectors() as $injector) {
if (null !== $definition->getFactoryService()) {
Expand All @@ -38,7 +42,7 @@ public function process(ContainerBuilder $container)
require_once $definition->getFile();
}

if (null !== $definition->getClass() && $injector->supports($definition->getClass())) {
if ($injector->supports($definition->getClass())) {
$injector->processDefinition($definition);
}
}
Expand Down
Expand Up @@ -40,6 +40,10 @@ public function process(ContainerBuilder $container)
{
$this->container = $container;
foreach ($container->getDefinitions() as $definition) {
if ($definition->isSynthetic()) {
continue;
}

$definition->setArguments(
$this->processArguments($definition->getArguments())
);
Expand Down
Expand Up @@ -61,8 +61,8 @@ protected function processArguments(array $arguments)

protected function getDefinitionId($id)
{
if ($this->container->hasAlias($id)) {
return $this->getDefinitionId((string) $this->container->getAlias($id));
while ($this->container->hasAlias($id)) {
$id = (string) $this->container->getAlias($id);
}

return $id;
Expand Down
Expand Up @@ -32,18 +32,18 @@ public function testProcessRemovesAndInlinesRecursively()
$container = new ContainerBuilder();

$a = $container
->register('a')
->register('a', '\stdClass')
->addArgument(new Reference('c'))
;

$b = $container
->register('b')
->register('b', '\stdClass')
->addArgument(new Reference('c'))
->setPublic(false)
;

$c = $container
->register('c')
->register('c', '\stdClass')
->setPublic(false)
;

Expand All @@ -61,14 +61,14 @@ public function testProcessInlinesReferencesToAliases()
$container = new ContainerBuilder();

$a = $container
->register('a')
->register('a', '\stdClass')
->addArgument(new Reference('b'))
;

$container->setAlias('b', new Alias('c', false));

$c = $container
->register('c')
->register('c', '\stdClass')
->setPublic(false)
;

Expand All @@ -86,19 +86,19 @@ public function testProcessInlinesWhenThereAreMultipleReferencesButFromTheSameDe
$container = new ContainerBuilder();

$container
->register('a')
->register('a', '\stdClass')
->addArgument(new Reference('b'))
->addMethodCall('setC', array(new Reference('c')))
;

$container
->register('b')
->register('b', '\stdClass')
->addArgument(new Reference('c'))
->setPublic(false)
;

$container
->register('c')
->register('c', '\stdClass')
->setPublic(false)
;

Expand Down

0 comments on commit f29a5f7

Please sign in to comment.