[ExpressionLanguage] Fix null-safe chaining

HypeMC · fabpot · commit a39c25cfbf98 · 2022-05-27T08:38:45.000+02:00
diff --git a/src/Symfony/Component/ExpressionLanguage/Node/GetAttrNode.php b/src/Symfony/Component/ExpressionLanguage/Node/GetAttrNode.php
@@ -24,6 +24,8 @@ class GetAttrNode extends Node
     public const METHOD_CALL = 2;
     public const ARRAY_CALL = 3;
 
+    private bool $isShortCircuited = false;
+
     public function __construct(Node $node, Node $attribute, ArrayNode $arguments, int $type)
     {
         parent::__construct(
@@ -70,9 +72,16 @@ public function evaluate(array $functions, array $values)
         switch ($this->attributes['type']) {
             case self::PROPERTY_CALL:
                 $obj = $this->nodes['node']->evaluate($functions, $values);
+
                 if (null === $obj && $this->nodes['attribute']->isNullSafe) {
+                    $this->isShortCircuited = true;
+
+                    return null;
+                }
+                if (null === $obj && $this->isShortCircuited()) {
                     return null;
                 }
+
                 if (!\is_object($obj)) {
                     throw new \RuntimeException(sprintf('Unable to get property "%s" of non-object "%s".', $this->nodes['attribute']->dump(), $this->nodes['node']->dump()));
                 }
@@ -83,9 +92,16 @@ public function evaluate(array $functions, array $values)
 
             case self::METHOD_CALL:
                 $obj = $this->nodes['node']->evaluate($functions, $values);
+
                 if (null === $obj && $this->nodes['attribute']->isNullSafe) {
+                    $this->isShortCircuited = true;
+
+                    return null;
+                }
+                if (null === $obj && $this->isShortCircuited()) {
                     return null;
                 }
+
                 if (!\is_object($obj)) {
                     throw new \RuntimeException(sprintf('Unable to call method "%s" of non-object "%s".', $this->nodes['attribute']->dump(), $this->nodes['node']->dump()));
                 }
@@ -97,6 +113,11 @@ public function evaluate(array $functions, array $values)
 
             case self::ARRAY_CALL:
                 $array = $this->nodes['node']->evaluate($functions, $values);
+
+                if (null === $array && $this->isShortCircuited()) {
+                    return null;
+                }
+
                 if (!\is_array($array) && !$array instanceof \ArrayAccess) {
                     throw new \RuntimeException(sprintf('Unable to get an item of non-array "%s".', $this->nodes['node']->dump()));
                 }
@@ -105,6 +126,13 @@ public function evaluate(array $functions, array $values)
         }
     }
 
+    private function isShortCircuited(): bool
+    {
+        return $this->isShortCircuited
+            || ($this->nodes['node'] instanceof self && $this->nodes['node']->isShortCircuited())
+        ;
+    }
+
     public function toArray()
     {
         switch ($this->attributes['type']) {
diff --git a/src/Symfony/Component/ExpressionLanguage/Tests/ExpressionLanguageTest.php b/src/Symfony/Component/ExpressionLanguage/Tests/ExpressionLanguageTest.php
@@ -249,13 +249,13 @@ public function testNullSafeEvaluate($expression, $foo)
     /**
      * @dataProvider provideNullSafe
      */
-    public function testNullsafeCompile($expression, $foo)
+    public function testNullSafeCompile($expression, $foo)
     {
         $expressionLanguage = new ExpressionLanguage();
         $this->assertNull(eval(sprintf('return %s;', $expressionLanguage->compile($expression, ['foo' => 'foo']))));
     }
 
-    public function provideNullsafe()
+    public function provideNullSafe()
     {
         $foo = new class() extends \stdClass {
             public function bar()
@@ -272,6 +272,47 @@ public function bar()
         yield ['foo["bar"]?.baz()', ['bar' => null]];
         yield ['foo.bar()?.baz', $foo];
         yield ['foo.bar()?.baz()', $foo];
+
+        yield ['foo?.bar.baz', null];
+        yield ['foo?.bar["baz"]', null];
+        yield ['foo?.bar["baz"]["qux"]', null];
+        yield ['foo?.bar["baz"]["qux"].quux', null];
+        yield ['foo?.bar["baz"]["qux"].quux()', null];
+        yield ['foo?.bar().baz', null];
+        yield ['foo?.bar()["baz"]', null];
+        yield ['foo?.bar()["baz"]["qux"]', null];
+        yield ['foo?.bar()["baz"]["qux"].quux', null];
+        yield ['foo?.bar()["baz"]["qux"].quux()', null];
+    }
+
+    /**
+     * @dataProvider provideInvalidNullSafe
+     */
+    public function testNullSafeEvaluateFails($expression, $foo, $message)
+    {
+        $expressionLanguage = new ExpressionLanguage();
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionMessage($message);
+        $expressionLanguage->evaluate($expression, ['foo' => $foo]);
+    }
+
+    /**
+     * @dataProvider provideInvalidNullSafe
+     */
+    public function testNullSafeCompileFails($expression, $foo)
+    {
+        $expressionLanguage = new ExpressionLanguage();
+
+        $this->expectWarning();
+        eval(sprintf('return %s;', $expressionLanguage->compile($expression, ['foo' => 'foo'])));
+    }
+
+    public function provideInvalidNullSafe()
+    {
+        yield ['foo?.bar.baz', (object) ['bar' => null], 'Unable to get property "baz" of non-object "foo.bar".'];
+        yield ['foo?.bar["baz"]', (object) ['bar' => null], 'Unable to get an item of non-array "foo.bar".'];
+        yield ['foo?.bar["baz"].qux.quux', (object) ['bar' => ['baz' => null]], 'Unable to get property "qux" of non-object "foo.bar["baz"]".'];
     }
 
     /**
