bug #4898 Reject destructuring patterns containing no variables (fabpot)

This PR was squashed before being merged into the 3.x branch.

Discussion
----------

Reject destructuring patterns containing no variables

This rejects sequence destructuring patterns containing only empty slots, such as `[,]` and `[,,]`.

These patterns previously compiled to an empty PHP list assignment and caused an uncatchable fatal error. They now produce a Twig `SyntaxError` with the template source and line.

Commits
-------

a3a318face Reject destructuring patterns containing no variables

CHANGELOG

@@ -3,7 +3,7 @@
  * Add the `HtmlExtension::htmlAttrValue()` method to resolve a single HTML attribute value the way the `html_attr` function renders it
  * 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 an empty destructuring pattern triggering a PHP fatal error instead of a `SyntaxError`
+ * Fix destructuring patterns with no variables, including sequences containing only empty slots, triggering a PHP fatal error instead of a `SyntaxError`
  * 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/ExpressionParser/Infix/AssignmentExpressionParser.php

@@ -19,6 +19,7 @@ use Twig\Node\Expression\Binary\AbstractBinary;
 use Twig\Node\Expression\Binary\ObjectDestructuringSetBinary;
 use Twig\Node\Expression\Binary\SequenceDestructuringSetBinary;
 use Twig\Node\Expression\Binary\SetBinary;
+use Twig\Node\Expression\EmptyExpression;
 use Twig\Node\Expression\Variable\AssignContextVariable;
 use Twig\Node\Expression\Variable\ContextVariable;
 use Twig\Parser;
@@ -50,16 +51,19 @@ class AssignmentExpressionParser extends BinaryOperatorExpressionParser
         };
 
         if ($left instanceof ArrayExpression) {
-            if (!$left->getKeyValuePairs()) {
+            $pairs = $left->getKeyValuePairs();
+            $isSequence = $left->isSequence();
+            if (!$pairs || ($isSequence && !array_filter($pairs, static fn (array $pair): bool => !$pair['value'] instanceof EmptyExpression))) {
                 throw new SyntaxError('Cannot destructure to an empty list of variables.', $token->getLine(), $parser->getStream()->getSourceContext());
             }
-            foreach ($left->getKeyValuePairs() as $i => $pair) {
+
+            foreach ($pairs as $i => $pair) {
                 if ($pair['value'] instanceof ContextVariable && !$pair['value'] instanceof AssignContextVariable) {
                     $left->setNode(2 * $i + 1, new AssignContextVariable($pair['value']->getAttribute('name'), $pair['value']->getTemplateLine()));
                 }
             }
 
-            if ($left->isSequence()) {
+            if ($isSequence) {
                 return new SequenceDestructuringSetBinary($left, $right, $token->getLine());
             }
 

tests/ExpressionParserTest.php

@@ -277,7 +277,7 @@ class ExpressionParserTest extends TestCase
         $env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
 
         $this->expectException(SyntaxError::class);
-        $this->expectExceptionMessage('Cannot destructure to an empty list of variables');
+        $this->expectExceptionMessage('Cannot destructure to an empty list of variables in "index" at line 1.');
         $env->compileSource(new Source($template, 'index'));
     }
 
@@ -285,6 +285,8 @@ class ExpressionParserTest extends TestCase
     {
         yield ['{% do [] = values %}'];
         yield ['{% do {} = values %}'];
+        yield ['{% do [,] = values %}'];
+        yield ['{% do [,,] = values %}'];
     }
 
     public function testObjectDestructuringUsesAssignmentTargets(): void