feature #4367 Trigger deprecations when using "??" and "not" without explicit parentheses when precedence will change in 4.0 (fabpot)

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

Discussion
----------

Trigger deprecations when using "??" and "not" without explicit parentheses when precedence will change in 4.0

Closes #3387
Closes #3642

Commits
-------

2081b1ff74 Deprecate using the not unary operator without parenthesis
f4aacafd78 Deprecate using ?? without explicit parentheses
This commit is contained in:
Fabien Potencier
2024-10-02 14:17:10 +02:00
12 changed files with 197 additions and 37 deletions
+4 -1
View File
@@ -1,6 +1,9 @@
# 3.15.0 (2024-XX-XX)
* Deprecate using `~` with `+` or `-` in an expression without using parentheses to clarify precedence
* Add support for triggering deprecations for future operator precedence changes
* Deprecate using the `not` unary operator in an expression with ``*``, ``/``, ``//``, or ``%`` without using explicit parentheses to clarify precedence
* Deprecate using the `??` binary operator without explicit parentheses
* Deprecate using the `~` binary operator in an expression with `+` or `-` without using parentheses to clarify precedence
* Deprecate not passing `AbstractExpression` args to most constructor arguments for classes extending `AbstractExpression`
* Fix `power` expressions with a negative number in parenthesis (`(-1) ** 2`)
* Deprecate instantiating `Node` directly. Use `EmptyNode` or `Nodes` instead.
+38 -4
View File
@@ -314,10 +314,9 @@ Node
Operators
---------
* Using ``~`` with ``+`` or ``-`` in an expression without using parentheses to
clarify precedence is deprecated as of Twig 3.15 (in Twig 4.0, parentheses
won't be needed anymore as ``+`` / ``-`` will have a higher precedence than
``~``).
* Using ``~`` in an expression with the ``+`` or ``-`` operators without using
parentheses to clarify precedence triggers a deprecation as of Twig 3.15 (in
Twig 4.0, ``+`` / ``-`` will have a higher precedence than ``~``).
For example, the following expression will trigger a deprecation in Twig 3.15::
@@ -331,3 +330,38 @@ Operators
{# or #}
{{ '42' ~ (1 + 41) }} {# this is equivalent to what Twig 4.x will do without the parentheses #}
* Using ``??`` without explicit parentheses to clarify precedence triggers a
deprecation as of Twig 3.15 (in Twig 4.0, ``??`` will have the lowest
precedence).
For example, the following expression will trigger a deprecation in Twig 3.15::
{{ 'notnull' ?? 'foo' ~ '_bar' }}
To avoid the deprecation, wrap the ``??`` expression in parentheses to clarify
the precedence::
{{ ('notnull' ?? 'foo') ~ '_bar' }} {# this is equivalent to what Twig 3.x does without the parentheses #}
{# or #}
{{ 'notnull' ?? ('foo' ~ '_bar') }} {# this is equivalent to what Twig 4.x will do without the parentheses #}
* Using the ``not`` unary operator in an expression with ``*``, ``/``, ``//``,
or ``%`` operators without explicit parentheses to clarify precedence
triggers a deprecation as of Twig 3.15 (in Twig 4.0, ``not`` will have a
higher precedence than ``*``, ``/``, ``//``, and ``%``).
For example, the following expression will trigger a deprecation in Twig 3.15::
{{ not 1 * 2 }}
To avoid the deprecation, wrap the concatenation in parentheses to clarify
the precedence::
{{ (not 1 * 2) }} {# this is equivalent to what Twig 3.x does without the parentheses #}
{# or #}
{{ (not 1) * 2 }} {# this is equivalent to what Twig 4.x will do without the parentheses #}
+66 -19
View File
@@ -20,9 +20,7 @@ use Twig\Node\Expression\ArrayExpression;
use Twig\Node\Expression\ArrowFunctionExpression;
use Twig\Node\Expression\AssignNameExpression;
use Twig\Node\Expression\Binary\AbstractBinary;
use Twig\Node\Expression\Binary\AddBinary;
use Twig\Node\Expression\Binary\ConcatBinary;
use Twig\Node\Expression\Binary\SubBinary;
use Twig\Node\Expression\ConditionalExpression;
use Twig\Node\Expression\ConstantExpression;
use Twig\Node\Expression\GetAttrExpression;
@@ -57,6 +55,7 @@ class ExpressionParser
/** @var array<string, array{precedence: int, class: class-string<AbstractBinary>, associativity: self::OPERATOR_*}> */
private $binaryOperators;
private $readyNodes = [];
private array $precedenceChanges = [];
public function __construct(
private Parser $parser,
@@ -64,16 +63,37 @@ class ExpressionParser
) {
$this->unaryOperators = $env->getUnaryOperators();
$this->binaryOperators = $env->getBinaryOperators();
$ops = [];
foreach ($this->unaryOperators as $n => $c) {
$ops[] = $c + ['name' => $n, 'type' => 'unary'];
}
foreach ($this->binaryOperators as $n => $c) {
$ops[] = $c + ['name' => $n, 'type' => 'binary'];
}
foreach ($ops as $config) {
if (!isset($config['precedence_change'])) {
continue;
}
$name = $config['type'].'_'.$config['name'];
$min = min($config['precedence_change']->getNewPrecedence(), $config['precedence']);
$max = max($config['precedence_change']->getNewPrecedence(), $config['precedence']);
foreach ($ops as $c) {
if ($c['precedence'] > $min && $c['precedence'] < $max) {
$this->precedenceChanges[$c['type'].'_'.$c['name']][] = $name;
}
}
}
}
public function parseExpression($precedence = 0, $allowArrow = false)
public function parseExpression($precedence = 0, $allowArrow = false, bool $deprecationCheck = true)
{
if ($allowArrow && $arrow = $this->parseArrow()) {
return $arrow;
}
$expr = $this->getPrimary();
$token = $this->parser->getCurrentToken();
$previousToken = $token = $this->parser->getCurrentToken();
while ($this->isBinary($token) && $this->binaryOperators[$token->getValue()]['precedence'] >= $precedence) {
$op = $this->binaryOperators[$token->getValue()];
$this->parser->getStream()->next();
@@ -90,10 +110,15 @@ class ExpressionParser
$expr = new $class($expr, $expr1, $token->getLine());
}
$expr->setAttribute('operator', 'binary_'.$token->getValue());
$previousToken = $token;
$token = $this->parser->getCurrentToken();
}
$this->triggerPrecedenceDeprecations($expr, $token);
if ($deprecationCheck) {
$this->triggerPrecedenceDeprecations($expr, $previousToken);
}
if (0 === $precedence) {
return $this->parseConditionalExpression($expr);
@@ -104,18 +129,37 @@ class ExpressionParser
private function triggerPrecedenceDeprecations(AbstractExpression $expr, Token $token): void
{
// Precedence of the ~ operator will be lower than + and - in Twig 4.0
if ($expr instanceof AddBinary || $expr instanceof SubBinary) {
/** @var AbstractExpression $left */
$left = $expr->getNode('left');
/** @var AbstractExpression $right */
$right = $expr->getNode('right');
if (
($left instanceof ConcatBinary && !$left->hasExplicitParentheses())
||
($right instanceof ConcatBinary && !$right->hasExplicitParentheses())
) {
trigger_deprecation('twig/twig', '3.15', \sprintf('As "+" / "-" will have a higher precedence than "~" in Twig 4.0, please add parentheses to keep the current behavior in "%s" at line %d.', $this->parser->getStream()->getSourceContext()->getName(), $token->getLine()));
// Check that the all nodes that are between the 2 precedences have explicit parentheses
if (!$expr->hasAttribute('operator') || !isset($this->precedenceChanges[$expr->getAttribute('operator')])) {
return;
}
if (str_starts_with($unaryOp = $expr->getAttribute('operator'), 'unary')) {
if ($expr->hasExplicitParentheses()) {
return;
}
$target = explode('_', $unaryOp)[1];
$change = $this->unaryOperators[$target]['precedence_change'];
/** @var AbstractExpression $node */
$node = $expr->getNode('node');
foreach ($this->precedenceChanges as $operatorName => $changes) {
if (!in_array($unaryOp, $changes)) {
continue;
}
if ($node->hasAttribute('operator') && $operatorName === $node->getAttribute('operator')) {
trigger_deprecation($change->getPackage(), $change->getVersion(), \sprintf('Add explicit parentheses around the "%s" unary operator to avoid behavior change in the next major version as its precedence will change in "%s" at line %d.', $target, $this->parser->getStream()->getSourceContext()->getName(), $token->getLine()));
}
}
} else {
foreach ($this->precedenceChanges[$expr->getAttribute('operator')] as $operatorName) {
foreach ($expr as $node) {
/** @var AbstractExpression $node */
if ($node->hasAttribute('operator') && $operatorName === $node->getAttribute('operator') && !$node->hasExplicitParentheses()) {
$op = explode('_', $operatorName)[1];
$change = $this->binaryOperators[$op]['precedence_change'];
trigger_deprecation($change->getPackage(), $change->getVersion(), \sprintf('Add explicit parentheses around the "%s" binary operator to avoid behavior change in the next major version as its precedence will change in "%s" at line %d.', $op, $this->parser->getStream()->getSourceContext()->getName(), $token->getLine()));
}
}
}
}
}
@@ -188,10 +232,13 @@ class ExpressionParser
$expr = $this->parseExpression($operator['precedence']);
$class = $operator['class'];
return $this->parsePostfixExpression(new $class($expr, $token->getLine()));
$expr = new $class($expr, $token->getLine());
$expr->setAttribute('operator', 'unary_'.$token->getValue());
return $this->parsePostfixExpression($expr);
} elseif ($token->test(Token::PUNCTUATION_TYPE, '(')) {
$this->parser->getStream()->next();
$expr = $this->parseExpression()->setExplicitParentheses();
$expr = $this->parseExpression(deprecationCheck: false)->setExplicitParentheses();
$this->parser->getStream()->expect(Token::PUNCTUATION_TYPE, ')', 'An opened parenthesis is not properly closed');
return $this->parsePostfixExpression($expr);
+4 -4
View File
@@ -67,6 +67,7 @@ use Twig\Node\Expression\Unary\NotUnary;
use Twig\Node\Expression\Unary\PosUnary;
use Twig\Node\Node;
use Twig\NodeVisitor\MacroAutoImportNodeVisitor;
use Twig\OperatorPrecedenceChange;
use Twig\Parser;
use Twig\Source;
use Twig\Template;
@@ -297,7 +298,7 @@ final class CoreExtension extends AbstractExtension
{
return [
[
'not' => ['precedence' => 50, 'class' => NotUnary::class],
'not' => ['precedence' => 50, 'precedence_change' => new OperatorPrecedenceChange('twig/twig', '3.15', 70), 'class' => NotUnary::class],
'-' => ['precedence' => 500, 'class' => NegUnary::class],
'+' => ['precedence' => 500, 'class' => PosUnary::class],
],
@@ -325,8 +326,7 @@ final class CoreExtension extends AbstractExtension
'..' => ['precedence' => 25, 'class' => RangeBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'+' => ['precedence' => 30, 'class' => AddBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'-' => ['precedence' => 30, 'class' => SubBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
// Precedence of the ~ operator will change to 27 in Twig 4.0
'~' => ['precedence' => 40, 'class' => ConcatBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'~' => ['precedence' => 40, 'precedence_change' => new OperatorPrecedenceChange('twig/twig', '3.15', 27), 'class' => ConcatBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'*' => ['precedence' => 60, 'class' => MulBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'/' => ['precedence' => 60, 'class' => DivBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'//' => ['precedence' => 60, 'class' => FloorDivBinary::class, 'associativity' => ExpressionParser::OPERATOR_LEFT],
@@ -334,7 +334,7 @@ final class CoreExtension extends AbstractExtension
'is' => ['precedence' => 100, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'is not' => ['precedence' => 100, 'associativity' => ExpressionParser::OPERATOR_LEFT],
'**' => ['precedence' => 200, 'class' => PowerBinary::class, 'associativity' => ExpressionParser::OPERATOR_RIGHT],
'??' => ['precedence' => 300, 'class' => NullCoalesceExpression::class, 'associativity' => ExpressionParser::OPERATOR_RIGHT],
'??' => ['precedence' => 300, 'precedence_change' => new OperatorPrecedenceChange('twig/twig', '3.15', 5), 'class' => NullCoalesceExpression::class, 'associativity' => ExpressionParser::OPERATOR_RIGHT],
],
];
}
+2 -2
View File
@@ -67,8 +67,8 @@ interface ExtensionInterface
* @return array<array> First array of unary operators, second array of binary operators
*
* @psalm-return array{
* array<string, array{precedence: int, class: class-string<AbstractExpression>}>,
* array<string, array{precedence: int, class?: class-string<AbstractExpression>, associativity: ExpressionParser::OPERATOR_*}>
* array<string, array{precedence: int, precedence_change?: OperatorPrecedenceChange, class: class-string<AbstractExpression>}>,
* array<string, array{precedence: int, precedence_change?: OperatorPrecedenceChange, class?: class-string<AbstractExpression>, associativity: ExpressionParser::OPERATOR_*}>
* }
*/
public function getOperators();
+42
View File
@@ -0,0 +1,42 @@
<?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;
/**
* Represents a precedence change for an operator.
*
* @author Fabien Potencier <fabien@symfony.com>
*/
class OperatorPrecedenceChange
{
public function __construct(
private string $package,
private string $version,
private int $newPrecedence,
) {
}
public function getPackage(): string
{
return $this->package;
}
public function getVersion(): string
{
return $this->version;
}
public function getNewPrecedence(): string
{
return $this->newPrecedence;
}
}
+2 -2
View File
@@ -597,8 +597,8 @@ class EnvironmentTest_Extension extends AbstractExtension implements GlobalsInte
public function getOperators(): array
{
return [
['foo_unary' => []],
['foo_binary' => []],
['foo_unary' => ['precedence' => 0]],
['foo_binary' => ['precedence' => 0]],
];
}
@@ -1,8 +1,8 @@
--TEST--
+/- will have a higher precedence over ~ in Twig 4.0
--DEPRECATION--
Since twig/twig 3.15: As "+" / "-" will have a higher precedence than "~" in Twig 4.0, please add parentheses to keep the current behavior in "index.twig" at line 2.
Since twig/twig 3.15: As "+" / "-" will have a higher precedence than "~" in Twig 4.0, please add parentheses to keep the current behavior in "index.twig" at line 3.
Since twig/twig 3.15: Add explicit parentheses around the "~" binary operator to avoid behavior change in the next major version as its precedence will change in "index.twig" at line 2.
Since twig/twig 3.15: Add explicit parentheses around the "~" binary operator to avoid behavior change in the next major version as its precedence will change in "index.twig" at line 3.
--TEMPLATE--
{{ '42' ~ 1 + 41 }}
{{ '42' ~ 43 - 1 }}
@@ -0,0 +1,9 @@
--TEST--
*, /, //, and % will have a higher precedence over not in Twig 4.0
--DEPRECATION--
Since twig/twig 3.15: Add explicit parentheses around the "not" unary operator to avoid behavior change in the next major version as its precedence will change in "index.twig" at line 2.
--TEMPLATE--
{{ not 1 * 2 }}
--DATA--
return []
--EXPECT--
@@ -0,0 +1,9 @@
--TEST--
*, /, //, and % will have a higher precedence over not in Twig 4.0
--TEMPLATE--
{{ (not 1) * 2 }}
{{ (not 1 * 2) }}
--DATA--
return []
--EXPECT--
0
@@ -0,0 +1,16 @@
--TEST--
Twig supports the ?? operator
--DEPRECATION--
Since twig/twig 3.15: Add explicit parentheses around the "??" binary operator to avoid behavior change in the next major version as its precedence will change in "index.twig" at line 4.
Since twig/twig 3.15: Add explicit parentheses around the "??" binary operator to avoid behavior change in the next major version as its precedence will change in "index.twig" at line 5.
--TEMPLATE--
{{ nope ?? nada ?? 'OK' -}} {# no deprecation as the operators have the same precedence #}
{{ 1 + nope ?? nada ?? 2 }}
{{ 1 + nope ?? 3 + nada ?? 2 }}
--DATA--
return []
--EXPECT--
OK
3
6
+3 -3
View File
@@ -10,9 +10,9 @@ Twig supports the ?? operator
{{ foo.bar.baz.missing ?? 'OK' }}
{{ foo['bar'] ?? 'KO' }}
{{ foo['missing'] ?? 'OK' }}
{{ nope ?? nada ?? 'OK' }}
{{ 1 + nope ?? nada ?? 2 }}
{{ 1 + nope ?? 3 + nada ?? 2 }}
{{ nope ?? (nada ?? 'OK') }}
{{ 1 + (nope ?? (nada ?? 2)) }}
{{ 1 + (nope ?? 3) + (nada ?? 2) }}
--DATA--
return ['bar' => 'OK', 'foo' => ['bar' => 'OK']]
--EXPECT--