Fix precedence rules

twigphp · Feb 5, 2025 · 9ec0dda · 9ec0dda
1 parent 33f1701
commit 9ec0dda
Show file tree

Hide file tree

Showing 26 changed files with 346 additions and 161 deletions.
diff --git a/CHANGELOG b/CHANGELOG
@@ -1,5 +1,7 @@
 # 3.20.0 (2025-XX-XX)
 
+ * Deprecate using the `|` operator in an expression with `+` or `-` without using parentheses to clarify precedence
+ * Deprecate operator precedence outside of the [-512, 512] range
  * Introduce expression parser classes to describe operators and operands provided by extensions
    instead of arrays (it comes with many deprecations that are documented in
    the ``deprecated`` documentation chapter)

diff --git a/bin/generate_operators_precedence.php b/bin/generate_operators_precedence.php
@@ -1,65 +1,50 @@
 <?php
 
 use Twig\Environment;
+use Twig\ExpressionParser\ExpressionParserDescriptionInterface;
+use Twig\ExpressionParser\ExpressionParserType;
 use Twig\ExpressionParser\InfixAssociativity;
 use Twig\ExpressionParser\InfixExpressionParserInterface;
-use Twig\ExpressionParser\PrefixExpressionParserInterface;
 use Twig\Loader\ArrayLoader;
 
 require_once dirname(__DIR__).'/vendor/autoload.php';
 
-function printExpressionParsers($output, array $expressionParsers, bool $withAssociativity = false)
-{
-    if ($withAssociativity) {
-        fwrite($output, "\n=========== =========== =============\n");
-        fwrite($output, "Precedence  Operator    Associativity\n");
-        fwrite($output, "=========== =========== =============\n");
-    } else {
-        fwrite($output, "\n=========== ===========\n");
-        fwrite($output, "Precedence  Operator\n");
-        fwrite($output, "=========== ===========\n");
-    }
-
-    usort($expressionParsers, function($a, $b) {
-        $aPrecedence = $a->getPrecedenceChange() ? $a->getPrecedenceChange()->getNewPrecedence() : $a->getPrecedence();
-        $bPrecedence = $b->getPrecedenceChange() ? $b->getPrecedenceChange()->getNewPrecedence() : $b->getPrecedence();
-        return $bPrecedence - $aPrecedence; 
-    });
-
-    $current = \PHP_INT_MAX;
-    foreach ($expressionParsers as $expressionParser) {
-        $precedence = $expressionParser->getPrecedenceChange() ? $expressionParser->getPrecedenceChange()->getNewPrecedence() : $expressionParser->getPrecedence();
-        if ($precedence !== $current) {
-            $current = $precedence;
-            if ($withAssociativity) {
-                fwrite($output, \sprintf("\n%-11d %-11s %s", $precedence, $expressionParser->getName(), InfixAssociativity::Left === $expressionParser->getAssociativity() ? 'Left' : 'Right'));
-            } else {
-                fwrite($output, \sprintf("\n%-11d %s", $precedence, $expressionParser->getName()));
-            }
-        } else {
-            fwrite($output, "\n".str_repeat(' ', 12).$expressionParser->getName());
-        }
-    }
-    fwrite($output, "\n");
-}
-
 $output = fopen(dirname(__DIR__).'/doc/operators_precedence.rst', 'w');
 
 $twig = new Environment(new ArrayLoader([]));
-$prefixExpressionParsers = [];
-$infixExpressionParsers = [];
+$expressionParsers = [];
 foreach ($twig->getExpressionParsers() as $expressionParser) {
-    if ($expressionParser instanceof PrefixExpressionParserInterface) {
-        $prefixExpressionParsers[] = $expressionParser;
-    } elseif ($expressionParser instanceof InfixExpressionParserInterface) {
-        $infixExpressionParsers[] = $expressionParser;
-    }
+    $expressionParsers[] = $expressionParser;
 }
 
-fwrite($output, "Unary operators precedence:\n");
-printExpressionParsers($output, $prefixExpressionParsers);
-
-fwrite($output, "\nBinary and Ternary operators precedence:\n");
-printExpressionParsers($output, $infixExpressionParsers, true);
+fwrite($output, "\n=========== ============== ======= ============= ===========\n");
+fwrite($output, "Precedence  Operator       Type    Associativity Description\n");
+fwrite($output, "=========== ============== ======= ============= ===========");
+
+usort($expressionParsers, function($a, $b) {
+    $aPrecedence = $a->getPrecedenceChange() ? $a->getPrecedenceChange()->getNewPrecedence() : $a->getPrecedence();
+    $bPrecedence = $b->getPrecedenceChange() ? $b->getPrecedenceChange()->getNewPrecedence() : $b->getPrecedence();
+    return $bPrecedence - $aPrecedence;
+});
+
+$previous = null;
+foreach ($expressionParsers as $expressionParser) {
+    $precedence = $expressionParser->getPrecedenceChange() ? $expressionParser->getPrecedenceChange()->getNewPrecedence() : $expressionParser->getPrecedence();
+    $previousPrecedence = $previous ? ($previous->getPrecedenceChange() ? $previous->getPrecedenceChange()->getNewPrecedence() : $previous->getPrecedence()) : \PHP_INT_MAX;
+    $associativity = $expressionParser instanceof InfixExpressionParserInterface ? (InfixAssociativity::Left === $expressionParser->getAssociativity() ? 'Left' : 'Right') : 'n/a';
+    $previousAssociativity = $previous ? ($previous instanceof InfixExpressionParserInterface ? (InfixAssociativity::Left === $previous->getAssociativity() ? 'Left' : 'Right') : 'n/a') : 'n/a';
+    if ($previousPrecedence !== $precedence) {
+        $previous = null;
+    }
+    fwrite($output, rtrim(\sprintf("\n%-11s %-14s %-7s %-13s %s\n",
+        !$previous || $previousPrecedence !== $precedence ? $precedence : '',
+        '`'.$expressionParser->getName().'`',
+        !$previous || ExpressionParserType::getType($previous) !== ExpressionParserType::getType($expressionParser) ? ExpressionParserType::getType($expressionParser)->value : '',
+        !$previous || $previousAssociativity !== $associativity ? $associativity : '',
+        $expressionParser instanceof ExpressionParserDescriptionInterface ? $expressionParser->getDescription() : '',
+    )));
+    $previous = $expressionParser;
+}
+fwrite($output, "\n=========== ============== ======= ============= ===========\n");
 
 fclose($output);
diff --git a/doc/deprecated.rst b/doc/deprecated.rst
@@ -378,6 +378,8 @@ Node
 Operators
 ---------
 
+* An operator precedence must be part of the [-512, 512] range as of Twig 3.20.
+
 * The ``.`` operator allows accessing class constants as of Twig 3.15.
   This can be a BC break if you don't use UPPERCASE constant names.
 
@@ -433,6 +435,22 @@ Operators
 
     {{ (not 1) * 2 }} {# this is equivalent to what Twig 4.x will do without the parentheses #}
 
+* Using the ``|`` operator in an expression with ``+`` or ``-`` without explicit
+  parentheses to clarify precedence triggers a deprecation as of Twig 3.20 (in
+  Twig 4.0, ``|`` will have a higher precedence than ``+`` and ``-``).
+
+  For example, the following expression will trigger a deprecation in Twig 3.20::
+
+    {{ -1|abs }}
+
+  To avoid the deprecation, add parentheses to clarify the precedence::
+
+    {{ -(1|abs) }} {# this is equivalent to what Twig 3.x does without the parentheses #}
+
+    {# or #}
+
+    {{ (-1)|abs }} {# this is equivalent to what Twig 4.x will do without the parentheses #}
+
 * The ``Twig\Extension\ExtensionInterface::getOperators()`` method is deprecated
   as of Twig 3.20, use ``Twig\Extension\ExtensionInterface::getExpressionParsers()``
   instead:

diff --git a/doc/operators_precedence.rst b/doc/operators_precedence.rst
@@ -1,57 +1,49 @@
-Unary operators precedence:
 
-=========== ===========
-Precedence  Operator
-=========== ===========
-
-500         -
-            +
-70          not
-0           (
-            literal
-
-Binary and Ternary operators precedence:
-
-=========== =========== =============
-Precedence  Operator    Associativity
-=========== =========== =============
-
-300         .           Left
-            [
-            |
-            (
-250         =>          Left
-200         **          Right
-100         is          Left
-            is not
-60          *           Left
-            /
-            //
-            %
-30          +           Left
-            -
-27          ~           Left
-25          ..          Left
-20          ==          Left
-            !=
-            <=>
-            <
-            >
-            >=
-            <=
-            not in
-            in
-            matches
-            starts with
-            ends with
-            has some
-            has every
-18          b-and       Left
-17          b-xor       Left
-16          b-or        Left
-15          and         Left
-12          xor         Left
-10          or          Left
-5           ?:          Right
-            ??
-0           ?           Left
+=========== ============== ======= ============= ===========
+Precedence  Operator       Type    Associativity Description
+=========== ============== ======= ============= ===========
+512         `.`            infix   Left          Get an attribute on a variable
+            `[`                                  Array access
+            `(`                                  Function call
+500         `-`            prefix  n/a
+            `+`
+300         `|`            infix   Left          Filter call
+250         `=>`           infix   Left          Arrow function (x => expr)
+200         `**`           infix   Right
+100         `is`           infix   Left
+            `is not`
+70          `not`          prefix  n/a
+60          `*`            infix   Left
+            `/`
+            `//`                                 Floor division
+            `%`
+30          `+`            infix   Left
+            `-`
+27          `~`            infix   Left
+25          `..`           infix   Left
+20          `==`           infix   Left
+            `!=`
+            `<=>`
+            `<`
+            `>`
+            `>=`
+            `<=`
+            `not in`
+            `in`
+            `matches`
+            `starts with`
+            `ends with`
+            `has some`
+            `has every`
+18          `b-and`        infix   Left
+17          `b-xor`        infix   Left
+16          `b-or`         infix   Left
+15          `and`          infix   Left
+12          `xor`          infix   Left
+10          `or`           infix   Left
+5           `?:`           infix   Right         Elvis operator (a ?: b)
+            `??`                                 Null coalescing operator (a ?? b)
+0           `(`            prefix  n/a           Explicit group expression (a)
+            `literal`                            A literal value (boolean, string, number, sequence, mapping, ...)
+            `?`            infix   Left          Conditional operator (a ? b : c)
+=========== ============== ======= ============= ===========
diff --git a/doc/templates.rst b/doc/templates.rst
@@ -703,7 +703,7 @@ Twig allows you to do math in templates; the following operators are supported:
   ``4``.
 
 * ``//``: Divides two numbers and returns the floored integer result. ``{{ 20
-  // 7 }}`` is ``2``, ``{{ -20  // 7 }}`` is ``-3`` (this is just syntactic
+  // 7 }}`` is ``2``, ``{{ -20 // 7 }}`` is ``-3`` (this is just syntactic
   sugar for the :doc:`round<filters/round>` filter).
 
 * ``*``: Multiplies the left operand with the right one. ``{{ 2 * 2 }}`` would

diff --git a/src/ExpressionParser/ExpressionParserDescriptionInterface.php b/src/ExpressionParser/ExpressionParserDescriptionInterface.php
@@ -0,0 +1,17 @@
+<?php
+
+/*
+ * This file is part of Twig.
+ *
+ * (c) Fabien Potencier
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Twig\ExpressionParser;
+
+interface ExpressionParserDescriptionInterface
+{
+    public function getDescription(): string;
+}
diff --git a/src/ExpressionParser/ExpressionParsers.php b/src/ExpressionParser/ExpressionParsers.php
@@ -44,7 +44,6 @@ final class ExpressionParsers implements \IteratorAggregate
     public function __construct(
         array $parsers = [],
     ) {
-        $this->precedenceChanges = null;
         $this->add($parsers);
     }
 
@@ -55,12 +54,16 @@ public function __construct(
      */
     public function add(array $parsers): self
     {
-        foreach ($parsers as $operator) {
-            $type = ExpressionParserType::getType($operator);
-            $this->parsers[$type->value][$operator->getName()] = $operator;
-            $this->parsersByClass[$type->value][get_class($operator)] = $operator;
-            foreach ($operator->getAliases() as $alias) {
-                $this->aliases[$type->value][$alias] = $operator;
+        foreach ($parsers as $parser) {
+            if ($parser->getPrecedence() > 512 || $parser->getPrecedence() < -512) {
+                trigger_deprecation('twig/twig', '3.20', 'Precedence for "%s" must be between -512 and 512, got %d.', $parser->getName(), $parser->getPrecedence());
+                // throw new \InvalidArgumentException(\sprintf('Precedence for "%s" must be between -1024 and 1024, got %d.', $parser->getName(), $parser->getPrecedence()));
+            }
+            $type = ExpressionParserType::getType($parser);
+            $this->parsers[$type->value][$parser->getName()] = $parser;
+            $this->parsersByClass[$type->value][get_class($parser)] = $parser;
+            foreach ($parser->getAliases() as $alias) {
+                $this->aliases[$type->value][$alias] = $parser;
             }
         }
 

diff --git a/src/ExpressionParser/Infix/ArrowExpressionParser.php b/src/ExpressionParser/Infix/ArrowExpressionParser.php
@@ -12,6 +12,7 @@
 namespace Twig\ExpressionParser\Infix;
 
 use Twig\ExpressionParser\AbstractExpressionParser;
+use Twig\ExpressionParser\ExpressionParserDescriptionInterface;
 use Twig\ExpressionParser\InfixAssociativity;
 use Twig\ExpressionParser\InfixExpressionParserInterface;
 use Twig\Node\Expression\AbstractExpression;
@@ -22,7 +23,7 @@
 /**
  * @internal
  */
-final class ArrowExpressionParser extends AbstractExpressionParser implements InfixExpressionParserInterface
+final class ArrowExpressionParser extends AbstractExpressionParser implements InfixExpressionParserInterface, ExpressionParserDescriptionInterface
 {
     public function parse(Parser $parser, AbstractExpression $expr, Token $token): AbstractExpression
     {
@@ -35,6 +36,11 @@ public function getName(): string
         return '=>';
     }
 
+    public function getDescription(): string
+    {
+        return 'Arrow function (x => expr)';
+    }
+
     public function getPrecedence(): int
     {
         return 250;

diff --git a/src/ExpressionParser/Infix/BinaryOperatorExpressionParser.php b/src/ExpressionParser/Infix/BinaryOperatorExpressionParser.php
@@ -12,6 +12,7 @@
 namespace Twig\ExpressionParser\Infix;
 
 use Twig\ExpressionParser\AbstractExpressionParser;
+use Twig\ExpressionParser\ExpressionParserDescriptionInterface;
 use Twig\ExpressionParser\InfixAssociativity;
 use Twig\ExpressionParser\InfixExpressionParserInterface;
 use Twig\ExpressionParser\PrecedenceChange;
@@ -23,7 +24,7 @@
 /**
  * @internal
  */
-class BinaryOperatorExpressionParser extends AbstractExpressionParser implements InfixExpressionParserInterface
+class BinaryOperatorExpressionParser extends AbstractExpressionParser implements InfixExpressionParserInterface, ExpressionParserDescriptionInterface
 {
     public function __construct(
         /** @var class-string<AbstractBinary> */
@@ -32,6 +33,7 @@ public function __construct(
         private int $precedence,
         private InfixAssociativity $associativity = InfixAssociativity::Left,
         private ?PrecedenceChange $precedenceChange = null,
+        private ?string $description = null,
         private array $aliases = [],
     ) {
     }
@@ -56,6 +58,11 @@ public function getName(): string
         return $this->name;
     }
 
+    public function getDescription(): string
+    {
+        return $this->description ?? '';
+    }
+
     public function getPrecedence(): int
     {
         return $this->precedence;

diff --git a/src/ExpressionParser/Infix/ConditionalTernaryExpressionParser.php b/src/ExpressionParser/Infix/ConditionalTernaryExpressionParser.php
@@ -12,6 +12,7 @@
 namespace Twig\ExpressionParser\Infix;
 
 use Twig\ExpressionParser\AbstractExpressionParser;
+use Twig\ExpressionParser\ExpressionParserDescriptionInterface;
 use Twig\ExpressionParser\InfixAssociativity;
 use Twig\ExpressionParser\InfixExpressionParserInterface;
 use Twig\Node\Expression\AbstractExpression;
@@ -23,7 +24,7 @@
 /**
  * @internal
  */
-final class ConditionalTernaryExpressionParser extends AbstractExpressionParser implements InfixExpressionParserInterface
+final class ConditionalTernaryExpressionParser extends AbstractExpressionParser implements InfixExpressionParserInterface, ExpressionParserDescriptionInterface
 {
     public function parse(Parser $parser, AbstractExpression $left, Token $token): AbstractExpression
     {
@@ -44,6 +45,11 @@ public function getName(): string
         return '?';
     }
 
+    public function getDescription(): string
+    {
+        return 'Conditional operator (a ? b : c)';
+    }
+
     public function getPrecedence(): int
     {
         return 0;