Skip to content

Commit

Permalink
feature #16001 [DI] Warn when a definition relies on a deprecated cla…
Browse files Browse the repository at this point in the history
…ss in ContainerBuilder::createService() (nicolas-grekas)

This PR was merged into the 2.8 branch.

Discussion
----------

[DI] Warn when a definition relies on a deprecated class in ContainerBuilder::createService()

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | yes
| BC breaks?    | no
| Deprecations? | yes
| Tests pass?   | yes
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

The new feature is in the DI component and it enlighten a deprecation from Doctrine that we ignored in FrameworkBundle, that is also fixed in this PR.

See https://github.com/symfony/symfony/pull/16001/files?w=1

Commits
-------

ca69fa3 [DI] Warn when a definition relies on a deprecated class in ContainerBuilder::createService()
  • Loading branch information
fabpot committed Oct 6, 2015
2 parents 45a5b00 + ca69fa3 commit a1c7af1
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 14 deletions.
Expand Up @@ -841,22 +841,29 @@ private function registerAnnotationsConfiguration(array $config, ContainerBuilde
{
$loader->load('annotations.xml');

if ('file' === $config['cache']) {
$cacheDir = $container->getParameterBag()->resolveValue($config['file_cache_dir']);
if (!is_dir($cacheDir) && false === @mkdir($cacheDir, 0777, true) && !is_dir($cacheDir)) {
throw new \RuntimeException(sprintf('Could not create cache directory "%s".', $cacheDir));
if ('none' !== $config['cache']) {
if ('file' === $config['cache']) {
$cacheDir = $container->getParameterBag()->resolveValue($config['file_cache_dir']);
if (!is_dir($cacheDir) && false === @mkdir($cacheDir, 0777, true) && !is_dir($cacheDir)) {
throw new \RuntimeException(sprintf('Could not create cache directory "%s".', $cacheDir));
}

$container
->getDefinition('annotations.php_file_cache')
->replaceArgument(0, $cacheDir)
;

// The annotations.file_cache_reader service is deprecated
$container
->getDefinition('annotations.file_cache_reader')
->replaceArgument(1, $cacheDir)
->replaceArgument(2, $config['debug'])
;
}

$container
->getDefinition('annotations.file_cache_reader')
->replaceArgument(1, $cacheDir)
->replaceArgument(2, $config['debug'])
;
$container->setAlias('annotation_reader', 'annotations.file_cache_reader');
} elseif ('none' !== $config['cache']) {
$container
->getDefinition('annotations.cached_reader')
->replaceArgument(1, new Reference($config['cache']))
->replaceArgument(1, new Reference('file' !== $config['cache'] ? $config['cache'] : 'annotations.php_file_cache'))
->replaceArgument(2, $config['debug'])
;
$container->setAlias('annotation_reader', 'annotations.cached_reader');
Expand Down
Expand Up @@ -19,7 +19,12 @@
<argument /><!-- Debug-Flag -->
</service>

<service id="annotations.php_file_cache" class="Doctrine\Common\Cache\PhpFileCache" public="false">
<argument /><!-- Cache-Directory -->
</service>

<service id="annotations.file_cache_reader" class="%annotations.file_cache_reader.class%" public="false">
<deprecated>The "%service_id%" service is deprecated since 2.8 and will be removed in 3.0.</deprecated>
<argument type="service" id="annotations.reader" />
<argument /><!-- Cache-Directory -->
<argument /><!-- Debug Flag -->
Expand Down
Expand Up @@ -320,8 +320,9 @@ public function testAnnotations()
{
$container = $this->createContainerFromFile('full');

$this->assertEquals($container->getParameter('kernel.cache_dir').'/annotations', $container->getDefinition('annotations.file_cache_reader')->getArgument(1));
$this->assertInstanceOf('Doctrine\Common\Annotations\FileCacheReader', $container->get('annotation_reader'));
$this->assertEquals($container->getParameter('kernel.cache_dir').'/annotations', $container->getDefinition('annotations.php_file_cache')->getArgument(0));
$this->assertSame('annotations.cached_reader', (string) $container->getAlias('annotation_reader'));
$this->assertSame('annotations.php_file_cache', (string) $container->getDefinition('annotations.cached_reader')->getArgument(1));
}

public function testFileLinkFormat()
Expand Down
12 changes: 12 additions & 0 deletions src/Symfony/Component/DependencyInjection/ContainerBuilder.php
Expand Up @@ -899,6 +899,14 @@ public function createService(Definition $definition, $id, $tryProxy = true)
}

$service = call_user_func_array($factory, $arguments);

if (!$definition->isDeprecated() && is_array($factory) && is_string($factory[0])) {
$r = new \ReflectionClass($factory[0]);

if (0 < strpos($r->getDocComment(), "\n * @deprecated ")) {
@trigger_error(sprintf('The "%s" service relies on the deprecated "%s" factory class. It should either be deprecated or its factory upgraded.', $id, $r->name), E_USER_DEPRECATED);
}
}
} elseif (null !== $definition->getFactoryMethod(false)) {
if (null !== $definition->getFactoryClass(false)) {
$factory = $parameterBag->resolveValue($definition->getFactoryClass(false));
Expand All @@ -913,6 +921,10 @@ public function createService(Definition $definition, $id, $tryProxy = true)
$r = new \ReflectionClass($parameterBag->resolveValue($definition->getClass()));

$service = null === $r->getConstructor() ? $r->newInstance() : $r->newInstanceArgs($arguments);

if (!$definition->isDeprecated() && 0 < strpos($r->getDocComment(), "\n * @deprecated ")) {
@trigger_error(sprintf('The "%s" service relies on the deprecated "%s" class. It should either be deprecated or its implementation upgraded.', $id, $r->name), E_USER_DEPRECATED);
}
}

if ($tryProxy || !$definition->isLazy()) {
Expand Down

0 comments on commit a1c7af1

Please sign in to comment.