8000 feature #47683 [DependencyInjection] Deprecate numeric parameter name… · symfony/symfony@59da7ea · GitHub
[go: up one dir, main page]
Skip to content

Commit 59da7ea

Browse files
chalasrchalasr
committed
feature #47683 [DependencyInjection] Deprecate numeric parameter names (HeahDude)
This PR was merged into the 6.2 branch. Discussion ---------- [DependencyInjection] Deprecate numeric parameter names | Q | A | ------------- | --- | Branch? | 6.2 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | ~ | License | MIT | Doc PR | TODO While trying to use `'.' !== $key[0]` instead of `str_starts_with($key, '.')` in #47680, I noticed some tests were failing due to the usage of numeric parameter names in the fixtures. This leads to inconsistent behavior since the following code: `$parameterBag->set(10, 10)`, will first cast the name `10` to string because of the method signature, but will then cast back to integer when using it as an array key in https://github.com/symfony/symfony/blob/6.2/src/Symfony/Component/DependencyInjection/ParameterBag/ParameterBag.php#L89. Because Symfony does not use strict types, it can be really tricky. This PR propose to deprecate using such names to be able to properly throw in 7.0. Commits ------- 3e0050a [DependencyInjection] Deprecate numeric parameter names
2 parents 7b212a1 + 3e0050a commit 59da7ea

File tree

10 files changed

+135
-16
lines changed

10 files changed

+135
-16
lines changed

‎UPGRADE-6.2.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ DependencyInjection
1818

1919
* Change the signature of `ContainerAwareInterface::setContainer()` to `setContainer(?ContainerInterface)`
2020
* Deprecate calling `ContainerAwareTrait::setContainer()` without arguments
21+
* Deprecate using numeric parameter names
2122

2223
Form
2324
----

‎src/Symfony/Component/DependencyInjection/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ CHANGELOG
1111
* Allow #[When] to be extended
1212
* Change the signature of `ContainerAwareInterface::setContainer()` to `setContainer(?ContainerInterface)`
1313
* Deprecate calling `ContainerAwareTrait::setContainer()` without arguments
14+
* Deprecate using numeric parameter names
1415

1516
6.1
1617
---

‎src/Symfony/Component/DependencyInjection/ParameterBag/ParameterBag.php

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,12 @@ public function get(string $name): array|bool|string|int|float|\UnitEnum|null
8686

< EDBE /td>
8787
public function set(string $name, array|bool|string|int|float|\UnitEnum|null $value)
8888
{
89+
if (is_numeric($name)) {
90+
trigger_deprecation('symfony/dependency-injection', '6.2', sprintf('Using numeric parameter name "%s" is deprecated and will throw as of 7.0.', $name));
91+
// uncomment the following line in 7.0
92+
// throw new InvalidArgumentException(sprintf('The parameter name "%s" cannot be numeric.', $name));
93+
}
94+
8995
$this->parameters[$name] = $value;
9096
}
9197

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/ini/types.ini

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,23 +9,23 @@
99
no = no
1010
none = none
1111
constant = PHP_VERSION
12-
12 = 12
12+
12_int = 12
1313
12_string = '12'
1414
12_quoted_number = "12"
1515
12_comment = 12 ; comment
1616
12_string_comment = '12' ; comment
1717
12_quoted_number_comment = "12" ; comment
18-
-12 = -12
19-
0 = 0
20-
1 = 1
21-
0b0110 = 0b0110
22-
11112222333344445555 = 1111,2222,3333,4444,5555
23-
0777 = 0777
24-
255 = 0xFF
25-
100.0 = 1e2
26-
-120.0 = -1.2E2
27-
-10100.1 = -10100.1
28-
-10,100.1 = -10,100.1
18+
-12_negative = -12
19+
zero = 0
20+
one = 1
21+
0b0110_byte_string = 0b0110
22+
11112222333344445555_great_number = 1111,2222,3333,4444,5555
23+
0777_number_starting_with_0 = 0777
24+
255_hexadecimal = 0xFF
25+
100.0_exponential = 1e2
26+
-120.0_exponential = -1.2E2
27+
-10100.1_negative_float = -10100.1
28+
-10,100.1_negative_float = -10,100.1
2929
list[] = 1
3030
list[] = 2
3131
map[one] = 1

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/ini/types_legacy.ini

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
[parameters]
2+
true = true
3+
true_comment = true ; comment
4+
false = false
5+
null = null
6+
on = on
7+
off = off
8+
yes = yes
9+
no = no
10+
none = none
11+
constant = PHP_VERSION
12+
12 = 12
13+
12_string = '12'
14+
12_quoted_number = "12"
15+
12_comment = 12 ; comment
16+
12_string_comment = '12' ; comment
17+
12_quoted_number_comment = "12" ; comment
18+
-12 = -12
19+
0 = 0
20+
1 = 1
21+
0b0110 = 0b0110
22+
11112222333344445555 = 1111,2222,3333,4444,5555
23+
0777 = 0777
24+
255 = 0xFF
25+
100.0 = 1e2
26+
-120.0 = -1.2E2
27+
-10100.1 = -10100.1
28+
-10,100.1 = -10,100.1
29+
list[] = 1
30+
list[] = 2
31+
map[one] = 1
32+
map[two] = 2

‎src/Symfony/Component/DependencyInjection/Tests/Fixtures/xml/services2.xml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
55
xsi:schemaLocation="http://symfony.com/schema/dic/services https://symfony.com/schema/dic/services/services-1.0.xsd">
66
<parameters>
7-
<parameter>a string</parameter>
7+
<parameter key="a_string">a string</parameter>
88
<parameter key="foo">bar</parameter>
99
<parameter key="values" type="collection">
1010
<parameter>0</parameter>

‎src/Symfony/Component/DependencyInjection/Tests/Loader/FileLoaderTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ public function testImportWithGlobPattern()
5959

6060
$actual = $container->getParameterBag()->all();
6161
$expected = [
62-
'a string',
62+
'a_string' => 'a string',
6363
'foo' => 'bar',
6464
'values' => [
6565
0,

‎src/Symfony/Component/DependencyInjection/Tests/Loader/IniFileLoaderTest.php

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,56 @@ public function testTypeConversionsWithNativePhp($key, $value, $supported)
5959
}
6060

6161
public function getTypeConversions()
62+
{
63+
return [
64+
['true_comment', true, true],
65+
['true', true, true],
66+
['false', false, true],
67+
['on', true, true],
68+
['off', false, true],
69+
['yes', true, true],
70+
['no', false, true],
71+
['none', false, true],
72+
['null', null, true],
73+
['constant', \PHP_VERSION, true],
74+
['12_int', 12, true],
75+
['12_string', '12', true],
76+
['12_quoted_number', 12, false], // INI_SCANNER_RAW removes the double quotes
77+
['12_comment', 12, true],
78+
['12_string_comment', '12', true],
79+
['12_quoted_number_comment', 12, false], // INI_SCANNER_RAW removes the double quotes
80+
['-12_negative', -12, true],
81+
['one', 1, true],
82+
['zero', 0, true],
83+
['0b0110_byte_string', bindec('0b0110'), false], // not supported by INI_SCANNER_TYPED
84+
['11112222333344445555_great_number', '1111,2222,3333,4444,5555', true],
85+
['0777_number_starting_with_0', 0777, false], // not supported by INI_SCANNER_TYPED
86+
['255_hexadecimal', 0xFF, false], // not supported by INI_SCANNER_TYPED
87+
['100.0_exponential', 1e2, false], // not supported by INI_SCANNER_TYPED
88+
['-120.0_exponential', -1.2E2, false], // not supported by INI_SCANNER_TYPED
89+
['-10100.1_negative_float', -10100.1, false], // not supported by INI_SCANNER_TYPED
90+
['-10,100.1_negative_float', '-10,100.1', true],
91+
['list', [1, 2], true],
92+
['map', ['one' => 1, 'two' => 2], true],
93+
];
94+
}
95+
96+
/**
97+
* @group legacy
98+
*
99+
* @dataProvider getLegacyTypeConversions
100+
*/
101+
public function testLegacyTypeConversionsWithNativePhp($key, $value, $supported)
102+
{
103+
if (!$supported) {
104+
$this->markTestSkipped(sprintf('Converting the value "%s" to "%s" is not supported by the IniFileLoader.', $key, $value));
105+
}
106+
107+
$expected = parse_ini_file(__DIR__.'/../Fixtures/ini/types_legacy.ini', true, \INI_SCANNER_TYPED);
108+
$this->assertSame($value, $expected['parameters'][$key], '->load() converts values to PHP types');
109+
}
110+
111+
public function getLegacyTypeConversions()
62112
{
63113
return [
64114
['true_comment', true, true],

‎src/Symfony/Component/DependencyInjection/Tests/Loader/XmlFileLoaderTest.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -123,7 +123,7 @@ public function testLoadParameters()
123123

124124
$actual = $container->getParameterBag()->all();
125125
$expected = [
126-
'a string',
126+
'a_string' => 'a string',
127127
'foo' => 'bar',
128128
'values' => [
129129
0,
@@ -159,7 +159,7 @@ public function testLoadImports()
159159

160160
$actual = $container->getParameterBag()->all();
161161
$expected = [
162-
'a string',
162+
'a_string' => 'a string',
163163
'foo' => 'bar',
164164
'values' => [
165165
0,

‎src/Symfony/Component/DependencyInjection/Tests/ParameterBag/ParameterBagTest.php

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,35 @@ public function testGetSet()
6666
}
6767
}
6868

69+
/**
70+
* @group legacy
71+
* Test it will throw in 7.0
72+
*/
73+
public function testGetSetNumericName()
74+
{
75+
$bag = new ParameterBag(['foo']);
76+
$bag->set(1001, 'foo');
77+
$this->assertEquals('foo', $bag->get(1001), '->set() sets the value of a new parameter');
78+
79+
$bag->set(10.0, 'foo');
80+
$this->assertEquals('foo', $bag->get(10), '->set() sets the value of a new parameter');
81+
82+
$bag->set(0b0110, 'foo');
83+
$this->assertEquals('foo', $bag->get(0b0110), '->set() sets the value of a new parameter');
84+
85+
$bag->set('0', 'baz');
86+
$this->assertEquals('baz', $bag->get(0), '->set() overrides previously set parameter');
87+
88+
$this->assertTrue($bag->has(0));
89+
$this->assertTrue($bag->has(1001));
90+
$this->assertTrue($bag->has(10));
91+
$this->assertTrue($bag->has(0b0110));
92+
93+
foreach (array_keys($bag->all()) as $key) {
94+
$this->assertIsInt($key, 'Numeric string keys are cast to integers');
95+
}
96+
}
97+
6998
/**
7099
* @dataProvider provideGetThrowParameterNotFoundExceptionData
71100
*/

0 commit comments

Comments
 (0)
0