Skip to content

[DependencyInjection] Deprecate integer keys in "service_locator" config #48686

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 22, 2022
Merged

[DependencyInjection] Deprecate integer keys in "service_locator" config #48686

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions UPGRADE-6.3.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ DependencyInjection
-------------------

* Deprecate `PhpDumper` options `inline_factories_parameter` and `inline_class_loader_parameter`, use `inline_factories` and `inline_class_loader` instead
* Deprecate undefined and numeric keys with `service_locator` config, use string aliases instead

HttpKernel
----------
Expand Down
1 change: 1 addition & 0 deletions src/Symfony/Component/DependencyInjection/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ CHANGELOG
* Deprecate `PhpDumper` options `inline_factories_parameter` and `inline_class_loader_parameter`
* Add `RemoveBuildParametersPass`, which removes parameters starting with a dot during compilation
* Add support for nesting autowiring-related attributes into `#[Autowire(...)]`
* Deprecate undefined and numeric keys with `service_locator` config

6.2
---
Expand Down
8 changes: 7 additions & 1 deletion src/Symfony/Component/DependencyInjection/Loader/Configurator/ContainerConfigurator.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,13 @@ function inline_service(string $class = null): InlineServiceConfigurator
*/
function service_locator(array $values): ServiceLocatorArgument
{
return new ServiceLocatorArgument(AbstractConfigurator::processValue($values, true));
$values = AbstractConfigurator::processValue($values, true);

if (isset($values[0])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can use array_is_list to detect numerically indexed arrays. But for a more complete validation, all key needs to be strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, it's a bit confusing. It's possible to mix strings and sequential integers and non-sequential integers. The only sequential integers from zero (or undefined keys) are deprecated and will be replaced with IDs.

I will try to improve the messages. However, I could use help.

trigger_deprecation('symfony/dependency-injection', '6.3', 'Using integers as keys in a "service_locator()" argument is deprecated. The keys will default to the IDs of the original services in 7.0.');
}

return new ServiceLocatorArgument($values);
}

/**
Expand Down
5 changes: 5 additions & 0 deletions src/Symfony/Component/DependencyInjection/Loader/XmlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -526,6 +526,11 @@ private function getArgumentsAsPhp(\DOMElement $node, string $name, string $file
break;
case 'service_locator':
$arg = $this->getArgumentsAsPhp($arg, $name, $file);

if (isset($arg[0])) {
trigger_deprecation('symfony/dependency-injection', '6.3', 'Skipping "key" argument or using integers as values in a "service_locator" tag is deprecated. The keys will default to the IDs of the original services in 7.0.');
}

$arguments[$key] = new ServiceLocatorArgument($arg);
break;
case 'tagged':
Expand Down
4 changes: 4 additions & 0 deletions src/Symfony/Component/DependencyInjection/Loader/YamlFileLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -814,6 +814,10 @@ private function resolveServices(mixed $value, string $file, bool $isParameter =

$argument = $this->resolveServices($argument, $file, $isParameter);

if (isset($argument[0])) {
trigger_deprecation('symfony/dependency-injection', '6.3', 'Using integers as keys in a "!service_locator" tag is deprecated. The keys will default to the IDs of the original services in 7.0.');
}

return new ServiceLocatorArgument($argument);
}
if (\in_array($value->getTag(), ['tagged', 'tagged_iterator', 'tagged_locator'], true)) {
Expand Down
29 changes: 29 additions & 0 deletions ...nent/DependencyInjection/Tests/Fixtures/config/services_with_service_locator_argument.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace Symfony\Component\DependencyInjection\Loader\Configurator;

return function (ContainerConfigurator $configurator) {
$services = $configurator->services()->defaults()->public();

$services->set('foo_service', \stdClass::class);

$services->set('bar_service', \stdClass::class);

$services->set('locator_dependent_service_indexed', \ArrayObject::class)
->args([service_locator([
'foo' => service('foo_service'),
'bar' => service('bar_service'),
])]);

$services->set('locator_dependent_service_not_indexed', \ArrayObject::class)
->args([service_locator([
service('foo_service'),
service('bar_service'),
])]);

$services->set('locator_dependent_service_mixed', \ArrayObject::class)
->args([service_locator([
'foo' => service('foo_service'),
service('bar_service'),
])]);
};
29 changes: 29 additions & 0 deletions ...mponent/DependencyInjection/Tests/Fixtures/xml/services_with_service_locator_argument.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?xml version="1.0" encoding="utf-8"?>
<container xmlns="http://symfony.com/schema/dic/services" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
<services>
<service id="foo_service" class="stdClass"/>

<service id="bar_service" class="stdClass"/>

<service id="locator_dependent_service_indexed" class="ArrayObject">
<argument type="service_locator">
<argument key="foo" type="service" id="foo_service"/>
<argument key="bar" type="service" id="bar_service"/>
</argument>
</service>

<service id="locator_dependent_service_not_indexed" class="ArrayObject">
<argument type="service_locator">
<argument type="service" id="foo_service"/>
<argument type="service" id="bar_service"/>
</argument>
</service>

<service id="locator_dependent_service_mixed" class="ArrayObject">
<argument type="service_locator">
<argument key="foo" type="service" id="foo_service"/>
<argument type="service" id="bar_service"/>
</argument>
</service>
</services>
</container>
28 changes: 28 additions & 0 deletions ...ponent/DependencyInjection/Tests/Fixtures/yaml/services_with_service_locator_argument.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@

services:
foo_service:
class: stdClass

bar_service:
class: stdClass

locator_dependent_service_indexed:
class: ArrayObject
arguments:
- !service_locator
'foo': '@foo_service'
'bar': '@bar_service'

locator_dependent_service_not_indexed:
class: ArrayObject
arguments:
- !service_locator
- '@foo_service'
- '@bar_service'

locator_dependent_service_mixed:
class: ArrayObject
arguments:
- !service_locator
'foo': '@foo_service'
'0': '@bar_service'
26 changes: 26 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/PhpFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,23 @@
require_once __DIR__.'/../Fixtures/includes/AcmeExtension.php';

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\Config\Builder\ConfigBuilderGenerator;
use Symfony\Component\Config\FileLocator;
use Symfony\Component\DependencyInjection\Argument\ServiceLocatorArgument;
use Symfony\Component\DependencyInjection\ContainerBuilder;
use Symfony\Component\DependencyInjection\Dumper\PhpDumper;
use Symfony\Component\DependencyInjection\Dumper\YamlDumper;
use Symfony\Component\DependencyInjection\Exception\InvalidArgumentException;
use Symfony\Component\DependencyInjection\Loader\PhpFileLoader;
use Symfony\Component\DependencyInjection\Reference;
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooClassWithEnumAttribute;
use Symfony\Component\DependencyInjection\Tests\Fixtures\FooUnitEnum;

class PhpFileLoaderTest extends TestCase
{
use ExpectDeprecationTrait;

public function testSupports()
{
$loader = new PhpFileLoader(new ContainerBuilder(), new FileLocator());
Expand Down Expand Up @@ -200,4 +205,25 @@ public function testWhenEnv()

$loader->load($fixtures.'/config/when_env.php');
}

/**
* @group legacy
*/
public function testServiceWithServiceLocatorArgument()
{
$this->expectDeprecation('Since symfony/dependency-injection 6.3: Using integers as keys in a "service_locator()" argument is deprecated. The keys will default to the IDs of the original services in 7.0.');

$fixtures = realpath(__DIR__.'/../Fixtures');
$loader = new PhpFileLoader($container = new ContainerBuilder(), new FileLocator());
$loader->load($fixtures.'/config/services_with_service_locator_argument.php');

$values = ['foo' => new Reference('foo_service'), 'bar' => new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_indexed')->getArguments());

$values = [new Reference('foo_service'), new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_not_indexed')->getArguments());

$values = ['foo' => new Reference('foo_service'), 0 => new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_mixed')->getArguments());
}
}
24 changes: 24 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\DependencyInjection\Tests\Loader;

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\Config\Exception\FileLocatorFileNotFoundException;
use Symfony\Component\Config\Exception\LoaderLoadException;
use Symfony\Component\Config\FileLocator;
Expand Down Expand Up @@ -47,6 +48,8 @@

class XmlFileLoaderTest extends TestCase
{
use ExpectDeprecationTrait;

protected static $fixturesPath;

public static function setUpBeforeClass(): void
Expand Down Expand Up @@ -421,6 +424,27 @@ public function testParseTaggedArgumentsWithIndexBy()
$this->assertEquals(new ServiceLocatorArgument($taggedIterator3), $container->getDefinition('foo3_tagged_locator')->getArgument(0));
}

/**
* @group legacy
*/
public function testServiceWithServiceLocatorArgument()
{
$this->expectDeprecation('Since symfony/dependency-injection 6.3: Skipping "key" argument or using integers as values in a "service_locator" tag is deprecated. The keys will default to the IDs of the original services in 7.0.');

$container = new ContainerBuilder();
$loader = new XmlFileLoader($container, new FileLocator(self::$fixturesPath.'/xml'));
$loader->load('services_with_service_locator_argument.xml');

$values = ['foo' => new Reference('foo_service'), 'bar' => new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_indexed')->getArguments());

$values = [new Reference('foo_service'), new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_not_indexed')->getArguments());

$values = ['foo' => new Reference('foo_service'), 0 => new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_mixed')->getArguments());
}

public function testParseServiceClosure()
{
$container = new ContainerBuilder();
Expand Down
24 changes: 24 additions & 0 deletions src/Symfony/Component/DependencyInjection/Tests/Loader/YamlFileLoaderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
namespace Symfony\Component\DependencyInjection\Tests\Loader;

use PHPUnit\Framework\TestCase;
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
use Symfony\Component\Config\Exception\FileLocatorFileNotFoundException;
use Symfony\Component\Config\Exception\LoaderLoadException;
use Symfony\Component\Config\FileLocator;
Expand Down Expand Up @@ -45,6 +46,8 @@

class YamlFileLoaderTest extends TestCase
{
use ExpectDeprecationTrait;

protected static $fixturesPath;

public static function setUpBeforeClass(): void
Expand Down Expand Up @@ -408,6 +411,27 @@ public function testTaggedArgumentsWithIndex()
$this->assertEquals(new ServiceLocatorArgument($taggedIterator), $container->getDefinition('bar_service_tagged_locator')->getArgument(0));
}

/**
* @group legacy
*/
public function testServiceWithServiceLocatorArgument()
{
$this->expectDeprecation('Since symfony/dependency-injection 6.3: Using integers as keys in a "!service_locator" tag is deprecated. The keys will default to the IDs of the original services in 7.0.');

$container = new ContainerBuilder();
$loader = new YamlFileLoader($container, new FileLocator(self::$fixturesPath.'/yaml'));
$loader->load('services_with_service_locator_argument.yml');

$values = ['foo' => new Reference('foo_service'), 'bar' => new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_indexed')->getArguments());

$values = [new Reference('foo_service'), new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_not_indexed')->getArguments());

$values = ['foo' => new Reference('foo_service'), 0 => new Reference('bar_service')];
$this->assertEquals([new ServiceLocatorArgument($values)], $container->getDefinition('locator_dependent_service_mixed')->getArguments());
}

public function testParseServiceClosure()
{
$container = new ContainerBuilder();
Expand Down