8000 feature #49064 [ExpressionLanguage] Deprecate loose comparisons when … · symfony/symfony@28cce83 · GitHub
[go: up one dir, main page]
Skip to content

Commit 28cce83

Browse files
nicolas-grekasnicolas-grekas
committed
feature #49064 [ExpressionLanguage] Deprecate loose comparisons when using the "in" operator (nicolas-grekas)
This PR was merged into the 6.3 branch. Discussion ---------- [ExpressionLanguage] Deprecate loose comparisons when using the "in" operator | Q | A | ------------- | --- | Branch? | 6.3 | Bug fix? | no | New feature? | no | Deprecations? | yes | Tickets | - | License | MIT | Doc PR | - Twig already uses strict comparisons for its "in" operator. Using loose comparisons can be a foot-gun, since eg `"foo"` is currently "in" `["bar", true]`. I propose to trigger a deprecation in v6.3 and make the comparison strict in v7.0. The deprecation is not really actionable but I don't have a better idea that doesn't involve adding a new operator, and I can't find something that'd be better than "in". I propose to merge as is and see later on with feedback from the community if we need more. Commits ------- 032ee26 [ExpressionLanguage] Deprecate loose comparisons when using the "in" operator
2 parents 290d5e5 + 032ee26 commit 28cce83

File tree

5 files changed

+47
-10
lines changed

5 files changed

+47
-10
lines changed

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

+2
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@ CHANGELOG
55
---
66

77
* Add `enum` expression function
8+
* Deprecate loose comparisons when using the "in" operator; normalize the array parameter
9+
so it only has the expected types or implement loose matching in your own expression function
810

911
6.2
1012
---

‎src/Symfony/Component/ExpressionLanguage/Node/BinaryNode.php

+21-5
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ class BinaryNode extends Node
3030
private const FUNCTIONS = [
3131
'**' => 'pow',
3232
'..' => 'range',
33-
'in' => 'in_array',
34-
'not in' => '!in_array',
33+
'in' => '\\'.self::class.'::inArray',
34+
'not in' => '!\\'.self::class.'::inArray',
3535
'contains' => 'str_contains',
3636
'starts with' => 'str_starts_with',
3737
'ends with' => 'str_ends_with',
@@ -101,7 +101,7 @@ public function evaluate(array $functions, array $values)
101101
$right = $this->nodes['right']->evaluate($functions, $values);
102102

103103
if ('not in' === $operator) {
104-
return !\in_array($left, $right);
104+
return !self::inArray($left, $right);
105105
}
106106
$f = self::FUNCTIONS[$operator];
107107

@@ -143,9 +143,9 @@ public function evaluate(array $functions, array $values)
143143
case '<=':
144144
return $left <= $right;
145145
case 'not in':
146-
return !\in_array($left, $right);
146+
return !self::inArray($left, $right);
147147
case 'in':
148-
return \in_array($left, $right);
148+
return self::inArray($left, $right);
149149
case '+':
150150
return $left + $right;
151151
case '-':
@@ -176,6 +176,22 @@ public function toArray()
176176
return ['(', $this->nodes['left'], ' '.$this->attributes['operator'].' ', $this->nodes['right'], ')'];
177177
}
178178

179+
/**
180+
* @internal to be replaced by an inline strict call to in_array() in version 7.0
181+
*/
182+
public static function inArray($value, array $array): bool
183+
{
184+
if (false === $key = array_search($value, $array)) {
185+
return false;
186+
}
187+
188+
if (!\in_array($value, $array, true)) {
189+
trigger_deprecation('symfony/expression-language', '6.3', 'The "in" operator will use strict comparisons in Symfony 7.0. Loose match found with key "%s" for value %s. Normalize the array parameter so it only has the expected types or implement loose matching in your own expression function.', $key, json_encode($value));
190+
}
191+
192+
return true;
193+
}
194+
179195
private function evaluateMatches(string $regexp, ?string $str): int
180196
{
181197
set_error_handler(function ($t, $m) use ($regexp) {

‎src/Symfony/Component/ExpressionLanguage/Tests/ExpressionLanguageTest.php

+1-1
Original file line numberDiff line numberDiff line change
@@ -269,7 +269,7 @@ public function testOperatorCollisions()
269269
$expressionLanguage = new ExpressionLanguage();
270270
$expression = 'foo.not in [bar]';
271271
$compiled = $expressionLanguage->compile($expression, ['foo', 'bar']);
272-
$this->assertSame('in_array($foo->not, [0 => $bar])', $compiled);
272+
$this->assertSame('\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray($foo->not, [0 => $bar])', $compiled);
273273

274274
$result = $expressionLanguage->evaluate($expression, ['foo' => (object) ['not' => 'test'], 'bar' => 'test']);
275275
$this->assertTrue($result);

‎src/Symfony/Component/ExpressionLanguage/Tests/Node/BinaryNodeTest.php

+22-4
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
namespace Symfony\Component\ExpressionLanguage\Tests\Node;
1313

14+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
1415
use Symfony\Component\ExpressionLanguage\Compiler;
1516
use Symfony\Component\ExpressionLanguage\Node\ArrayNode;
1617
use Symfony\Component\ExpressionLanguage\Node\BinaryNode;
@@ -20,6 +21,8 @@
2021

2122
class BinaryNodeTest extends AbstractNodeTest
2223
{
24+
use ExpectDeprecationTrait;
25+
2326
public function getEvaluateData()
2427
{
2528
$array = new ArrayNode();
@@ -113,10 +116,10 @@ public function getCompileData()
113116
['pow(5, 2)', new BinaryNode('**', new ConstantNode(5), new ConstantNode(2))],
114117
['("a" . "b")', new BinaryNode('~', new F438 ConstantNode('a'), new ConstantNode('b'))],
115118

116-
['in_array("a", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('a'), $array)],
117-
['in_array("c", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('c'), $array)],
118-
['!in_array("c", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('c'), $array)],
119-
['!in_array("a", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('a'), $array)],
119+
['\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("a", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('a'), $array)],
120+
['\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("c", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('c'), $array)],
121+
['!\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("c", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('c'), $array)],
122+
['!\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("a", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('a'), $array)],
120123

121124
['range(1, 3)', new BinaryNode('..', new ConstantNode(1), new ConstantNode(3))],
122125

@@ -214,4 +217,19 @@ public function testCompileMatchesWithInvalidRegexpAsExpression()
214217
$node->compile($compiler);
215218
eval('$regexp = "this is not a regexp"; '.$compiler->getSource().';');
216219
}
220+
221+
/**
222+
* @group legacy
223+
*/
224+
public function testInOperatorStrictness()
225+
{
226+
$array = new ArrayNode();
227+
$array->addElement(new ConstantNode('a'));
228+
$array->addElement(new ConstantNode(true));
229+
230+
$node = new BinaryNode('in', new ConstantNode('b'), $array);
231+
232+
$this->expectDeprecation('Since symfony/expression-language 6.3: The "in" operator will use strict comparisons in Symfony 7.0. Loose match found with key "1" for value "b". Normalize the array parameter so it only has the expected types or implement loose matching in your own expression function.');
233+
$this->assertTrue($node->evaluate([], []));
234+
}
217235
}

‎src/Symfony/Component/ExpressionLanguage/composer.json

+1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
],
1818
"require": {
1919
"php": ">=8.1",
20+
"symfony/deprecation-contracts": "^2.5|^3",
2021
"symfony/cache": "^5.4|^6.0",
2122
"symfony/service-contracts": "^2.5|^3"
2223
},

0 commit comments

Comments
 (0)
0