[ExpressionLanguage] Deprecate loose comparisons when using the "in" operator

Author: nicolas-grekas

CHANGELOG.md

@@ -5,6 +5,8 @@ CHANGELOG
 ---
 
  * Add `enum` expression function
+ * Deprecate loose comparisons when using the "in" operator; normalize the array parameter
+   so it only has the expected types or implement loose matching in your own expression function
 
 6.2
 ---

Node/BinaryNode.php

@@ -30,8 +30,8 @@ class BinaryNode extends Node
     private const FUNCTIONS = [
         '**' => 'pow',
         '..' => 'range',
-        'in' => 'in_array',
-        'not in' => '!in_array',
+        'in' => '\\'.self::class.'::inArray',
+        'not in' => '!\\'.self::class.'::inArray',
         'contains' => 'str_contains',
         'starts with' => 'str_starts_with',
         'ends with' => 'str_ends_with',
@@ -101,7 +101,7 @@ public function evaluate(array $functions, array $values)
             $right = $this->nodes['right']->evaluate($functions, $values);
 
             if ('not in' === $operator) {
-                return !\in_array($left, $right);
+                return !self::inArray($left, $right);
             }
             $f = self::FUNCTIONS[$operator];
 
@@ -143,9 +143,9 @@ public function evaluate(array $functions, array $values)
             case '<=':
                 return $left <= $right;
             case 'not in':
-                return !\in_array($left, $right);
+                return !self::inArray($left, $right);
             case 'in':
-                return \in_array($left, $right);
+                return self::inArray($left, $right);
             case '+':
                 return $left + $right;
             case '-':
@@ -176,6 +176,22 @@ public function toArray()
         return ['(', $this->nodes['left'], ' '.$this->attributes['operator'].' ', $this->nodes['right'], ')'];
     }
 
+    /**
+     * @internal to be replaced by an inline strict call to in_array() in version 7.0
+     */
+    public static function inArray($value, array $array): bool
+    {
+        if (false === $key = array_search($value, $array)) {
+            return false;
+        }
+
+        if (!\in_array($value, $array, true)) {
+            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));
+        }
+
+        return true;
+    }
+
     private function evaluateMatches(string $regexp, ?string $str): int
     {
         set_error_handler(function ($t, $m) use ($regexp) {

Tests/ExpressionLanguageTest.php

@@ -269,7 +269,7 @@ public function testOperatorCollisions()
         $expressionLanguage = new ExpressionLanguage();
         $expression = 'foo.not in [bar]';
         $compiled = $expressionLanguage->compile($expression, ['foo', 'bar']);
-        $this->assertSame('in_array($foo->not, [0 => $bar])', $compiled);
+        $this->assertSame('\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray($foo->not, [0 => $bar])', $compiled);
 
         $result = $expressionLanguage->evaluate($expression, ['foo' => (object) ['not' => 'test'], 'bar' => 'test']);
         $this->assertTrue($result);

Tests/Node/BinaryNodeTest.php

@@ -11,6 +11,7 @@
 
 namespace Symfony\Component\ExpressionLanguage\Tests\Node;
 
+use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
 use Symfony\Component\ExpressionLanguage\Compiler;
 use Symfony\Component\ExpressionLanguage\Node\ArrayNode;
 use Symfony\Component\ExpressionLanguage\Node\BinaryNode;
@@ -20,6 +21,8 @@
 
 class BinaryNodeTest extends AbstractNodeTest
 {
+    use ExpectDeprecationTrait;
+
     public function getEvaluateData()
     {
         $array = new ArrayNode();
@@ -113,10 +116,10 @@ public function getCompileData()
             ['pow(5, 2)', new BinaryNode('**', new ConstantNode(5), new ConstantNode(2))],
             ['("a" . "b")', new BinaryNode('~', new ConstantNode('a'), new ConstantNode('b'))],
 
-            ['in_array("a", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('a'), $array)],
-            ['in_array("c", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('c'), $array)],
-            ['!in_array("c", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('c'), $array)],
-            ['!in_array("a", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('a'), $array)],
+            ['\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("a", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('a'), $array)],
+            ['\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("c", [0 => "a", 1 => "b"])', new BinaryNode('in', new ConstantNode('c'), $array)],
+            ['!\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("c", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('c'), $array)],
+            ['!\Symfony\Component\ExpressionLanguage\Node\BinaryNode::inArray("a", [0 => "a", 1 => "b"])', new BinaryNode('not in', new ConstantNode('a'), $array)],
 
             ['range(1, 3)', new BinaryNode('..', new ConstantNode(1), new ConstantNode(3))],
 
@@ -214,4 +217,19 @@ public function testCompileMatchesWithInvalidRegexpAsExpression()
         $node->compile($compiler);
         eval('$regexp = "this is not a regexp"; '.$compiler->getSource().';');
     }
+
+    /**
+     * @group legacy
+     */
+    public function testInOperatorStrictness()
+    {
+        $array = new ArrayNode();
+        $array->addElement(new ConstantNode('a'));
+        $array->addElement(new ConstantNode(true));
+
+        $node = new BinaryNode('in', new ConstantNode('b'), $array);
+
+        $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.');
+        $this->assertTrue($node->evaluate([], []));
+    }
 }

composer.json

@@ -17,6 +17,7 @@
     ],
     "require": {
         "php": ">=8.1",
+        "symfony/deprecation-contracts": "^2.5|^3",
         "symfony/cache": "^5.4|^6.0",
         "symfony/service-contracts": "^2.5|^3"
     },