bug #4901 Evaluate object destructuring expressions once (fabpot)

This PR was merged into the 3.x branch.

Discussion
----------

Evaluate object destructuring expressions once

This ensures the right-hand expression of an object or mapping destructuring assignment is evaluated exactly once.

All properties are now read from the same resolved value, avoiding repeated side effects and unnecessary work while preserving assignment order and return semantics.

Commits
-------

609376f491 Fix repeated object destructuring evaluation

CHANGELOG

@@ -4,6 +4,8 @@
  * Fix `html_attr` JSON encoding a `Stringable` value in a `data-*` attribute instead of using its string representation
  * Add documentation comments to attach metadata to nodes (experimental)
  * Fix destructuring patterns with no variables, including sequences containing only empty slots, triggering a PHP fatal error instead of a `SyntaxError`
+ * Fix an empty destructuring pattern triggering a PHP fatal error instead of a `SyntaxError`
+ * Fix object and mapping destructuring evaluating the right-hand expression more than once
  * Fix sequence destructuring of iterators throwing a `TypeError`
  * Add `TempestMarkdown` to use `tempest/markdown` as the `markdown_to_html` converter
  * Fix imported macros not resolving their own template-level macro imports

src/Node/Expression/Binary/ObjectDestructuringSetBinary.php

@@ -53,6 +53,7 @@ class ObjectDestructuringSetBinary extends AbstractBinary
     public function compile(Compiler $compiler): void
     {
         $compiler->addDebugInfo($this);
+        $var = '$'.$compiler->getVarName();
         $compiler->raw('[');
         foreach ($this->mappings as $i => $mapping) {
             if ($i) {
@@ -65,7 +66,13 @@ class ObjectDestructuringSetBinary extends AbstractBinary
             if ($i) {
                 $compiler->raw(', ');
             }
-            $compiler->raw('CoreExtension::getAttribute($this->env, $this->source, ')->subcompile($this->getNode('right'))->raw(', ')->repr($mapping['property'])->raw(', [], \\Twig\\Template::ANY_CALL, false, false, ')->repr($compiler->getEnvironment()->hasExtension(SandboxExtension::class))->raw(', ')->repr($this->getNode('right')->getTemplateLine())->raw(')');
+            $compiler->raw('CoreExtension::getAttribute($this->env, $this->source, ');
+            if (0 === $i) {
+                $compiler->raw('('.$var.' = ')->subcompile($this->getNode('right'))->raw(')');
+            } else {
+                $compiler->raw($var);
+            }
+            $compiler->raw(', ')->repr($mapping['property'])->raw(', [], \\Twig\\Template::ANY_CALL, false, false, ')->repr($compiler->getEnvironment()->hasExtension(SandboxExtension::class))->raw(', ')->repr($this->getNode('right')->getTemplateLine())->raw(')');
         }
         $compiler->raw(']');
     }

tests/ExpressionParserTest.php

@@ -303,6 +303,20 @@ class ExpressionParserTest extends TestCase
         $this->assertSame('user_name', $pair['value']->getAttribute('name'));
     }
 
+    public function testObjectDestructuringEvaluatesRightHandExpressionOnce(): void
+    {
+        $calls = 0;
+        $env = new Environment(new ArrayLoader(['template' => '{% do [result_first, result_second] = ({first, second} = next_value()) %}{{ first }} {{ second }} {{ result_first }} {{ result_second }}']));
+        $env->addFunction(new TwigFunction('next_value', static function () use (&$calls): object {
+            ++$calls;
+
+            return (object) ['first' => $calls, 'second' => $calls];
+        }));
+
+        $this->assertSame('1 1 1 1', $env->render('template'));
+        $this->assertSame(1, $calls);
+    }
+
     /**
      * @dataProvider getTestsForString
      */