From 0ce79df5c4054d18e8e543a382bcfda35770dbd2 Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Thu, 8 Aug 2024 11:56:21 +0200 Subject: [PATCH] Use trigger_deprecation() instead of trigger_error() for deprecations --- CHANGELOG | 2 ++ doc/advanced.rst | 12 +++++++ doc/tags/deprecated.rst | 11 ++++++ src/ExpressionParser.php | 15 ++------ src/Node/DeprecatedNode.php | 36 ++++++++++++++----- src/TokenParser/DeprecatedTokenParser.php | 29 +++++++++++++-- src/TwigFilter.php | 6 ++++ src/TwigFunction.php | 6 ++++ src/TwigTest.php | 6 ++++ .../tags/deprecated/with_package.legacy.test | 10 ++++++ .../with_package_version.legacy.test | 10 ++++++ tests/Node/DeprecatedTest.php | 14 +++++--- tests/Util/DeprecationCollectorTest.php | 4 +-- 13 files changed, 131 insertions(+), 30 deletions(-) create mode 100644 tests/Fixtures/tags/deprecated/with_package.legacy.test create mode 100644 tests/Fixtures/tags/deprecated/with_package_version.legacy.test diff --git a/CHANGELOG b/CHANGELOG index 24fc6e7d364..fd552fe3563 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,5 +1,7 @@ # 3.11.0 (2024-XX-XX) + * Add the possibility to add a package and a version to the `deprecated` tag + * Add the possibility to add a package for filter/function/test deprecations * Mark `ConstantExpression` as being `@final` * Add the `find` filter * Fix optimizer mode validation in `OptimizerNodeVisitor` diff --git a/doc/advanced.rst b/doc/advanced.rst index c1073242fd5..1e89694e7e2 100644 --- a/doc/advanced.rst +++ b/doc/advanced.rst @@ -285,6 +285,18 @@ deprecated one when that makes sense:: // ... }, ['deprecated' => true, 'alternative' => 'new_one']); +.. versionadded:: 3.11 + + The ``deprecating_package`` option was added in Twig 3.11. + +You can also set the ``deprecating_package`` option to specify the package that +is deprecating the filter, and ``deprecated`` can be set to the package version +when the filter was deprecated:: + + $filter = new \Twig\TwigFilter('obsolete', function () { + // ... + }, ['deprecated' => '1.1', 'deprecating_package' => 'foo/bar']); + When a filter is deprecated, Twig emits a deprecation notice when compiling a template using it. See :ref:`deprecation-notices` for more information. diff --git a/doc/tags/deprecated.rst b/doc/tags/deprecated.rst index d2dbe7f2bf7..811e88e3ff1 100644 --- a/doc/tags/deprecated.rst +++ b/doc/tags/deprecated.rst @@ -23,6 +23,17 @@ You can also deprecate a macro in the following way: Note that by default, the deprecation notices are silenced and never displayed nor logged. See :ref:`deprecation-notices` to learn how to handle them. +.. versionadded:: 3.11 + + The ``package`` and ``version`` options were added in Twig 3.11. + +You can optionally add the package and the version that introduced the deprecation: + +.. code-block:: twig + + {% deprecated 'The "base.twig" template is deprecated, use "layout.twig" instead.' package='twig/twig' %} + {% deprecated 'The "base.twig" template is deprecated, use "layout.twig" instead.' package='twig/twig' version='3.11' %} + .. note:: Don't use the ``deprecated`` tag to deprecate a ``block`` as the diff --git a/src/ExpressionParser.php b/src/ExpressionParser.php index 006c1bdbba7..84484404864 100644 --- a/src/ExpressionParser.php +++ b/src/ExpressionParser.php @@ -774,16 +774,13 @@ private function getTestNodeClass(TwigTest $test): string $stream = $this->parser->getStream(); $message = \sprintf('Twig Test "%s" is deprecated', $test->getName()); - if ($test->getDeprecatedVersion()) { - $message .= \sprintf(' since version %s', $test->getDeprecatedVersion()); - } if ($test->getAlternative()) { $message .= \sprintf('. Use "%s" instead', $test->getAlternative()); } $src = $stream->getSourceContext(); $message .= \sprintf(' in %s at line %d.', $src->getPath() ?: $src->getName(), $stream->getCurrent()->getLine()); - @trigger_error($message, \E_USER_DEPRECATED); + trigger_deprecation($test->getDeprecatingPackage(), $test->getDeprecatedVersion(), $message); } return $test->getNodeClass(); @@ -800,16 +797,13 @@ private function getFunctionNodeClass(string $name, int $line): string if ($function->isDeprecated()) { $message = \sprintf('Twig Function "%s" is deprecated', $function->getName()); - if ($function->getDeprecatedVersion()) { - $message .= \sprintf(' since version %s', $function->getDeprecatedVersion()); - } if ($function->getAlternative()) { $message .= \sprintf('. Use "%s" instead', $function->getAlternative()); } $src = $this->parser->getStream()->getSourceContext(); $message .= \sprintf(' in %s at line %d.', $src->getPath() ?: $src->getName(), $line); - @trigger_error($message, \E_USER_DEPRECATED); + trigger_deprecation($function->getDeprecatingPackage(), $function->getDeprecatedVersion(), $message); } return $function->getNodeClass(); @@ -826,16 +820,13 @@ private function getFilterNodeClass(string $name, int $line): string if ($filter->isDeprecated()) { $message = \sprintf('Twig Filter "%s" is deprecated', $filter->getName()); - if ($filter->getDeprecatedVersion()) { - $message .= \sprintf(' since version %s', $filter->getDeprecatedVersion()); - } if ($filter->getAlternative()) { $message .= \sprintf('. Use "%s" instead', $filter->getAlternative()); } $src = $this->parser->getStream()->getSourceContext(); $message .= \sprintf(' in %s at line %d.', $src->getPath() ?: $src->getName(), $line); - @trigger_error($message, \E_USER_DEPRECATED); + trigger_deprecation($filter->getDeprecatingPackage(), $filter->getDeprecatedVersion(), $message); } return $filter->getNodeClass(); diff --git a/src/Node/DeprecatedNode.php b/src/Node/DeprecatedNode.php index 1a07ab81afb..afeb8332e29 100644 --- a/src/Node/DeprecatedNode.php +++ b/src/Node/DeprecatedNode.php @@ -35,21 +35,39 @@ public function compile(Compiler $compiler): void $expr = $this->getNode('expr'); - if ($expr instanceof ConstantExpression) { - $compiler->write('@trigger_error(') - ->subcompile($expr); - } else { + if (!$expr instanceof ConstantExpression) { $varName = $compiler->getVarName(); - $compiler->write(\sprintf('$%s = ', $varName)) + $compiler + ->write(\sprintf('$%s = ', $varName)) ->subcompile($expr) ->raw(";\n") - ->write(\sprintf('@trigger_error($%s', $varName)); + ; + } + + $compiler->write('trigger_deprecation('); + if ($this->hasNode('package')) { + $compiler->subcompile($this->getNode('package')); + } else { + $compiler->raw("''"); + } + $compiler->raw(', '); + if ($this->hasNode('version')) { + $compiler->subcompile($this->getNode('version')); + } else { + $compiler->raw("''"); + } + $compiler->raw(', '); + + if ($expr instanceof ConstantExpression) { + $compiler->subcompile($expr); + } else { + $compiler->write(\sprintf('$%s', $varName)); } $compiler - ->raw('.') - ->string(\sprintf(' ("%s" at line %d).', $this->getTemplateName(), $this->getTemplateLine())) - ->raw(", E_USER_DEPRECATED);\n") + ->raw(".") + ->string(\sprintf(' in "%s" at line %d.', $this->getTemplateName(), $this->getTemplateLine())) + ->raw(");\n") ; } } diff --git a/src/TokenParser/DeprecatedTokenParser.php b/src/TokenParser/DeprecatedTokenParser.php index 31416c79c15..c17c4aadc29 100644 --- a/src/TokenParser/DeprecatedTokenParser.php +++ b/src/TokenParser/DeprecatedTokenParser.php @@ -11,6 +11,7 @@ namespace Twig\TokenParser; +use Twig\Error\SyntaxError; use Twig\Node\DeprecatedNode; use Twig\Node\Node; use Twig\Token; @@ -21,6 +22,8 @@ * {% deprecated 'The "base.twig" template is deprecated, use "layout.twig" instead.' %} * {% extends 'layout.html.twig' %} * + * {% deprecated 'The "base.twig" template is deprecated, use "layout.twig" instead.' package="foo/bar" version="1.1" %} + * * @author Yonel Ceruto * * @internal @@ -29,11 +32,31 @@ final class DeprecatedTokenParser extends AbstractTokenParser { public function parse(Token $token): Node { - $expr = $this->parser->getExpressionParser()->parseExpression(); + $stream = $this->parser->getStream(); + $expressionParser = $this->parser->getExpressionParser(); + $expr = $expressionParser->parseExpression(); + $node = new DeprecatedNode($expr, $token->getLine(), $this->getTag()); + + while ($stream->test(Token::NAME_TYPE)) { + $k = $stream->getCurrent()->getValue(); + $stream->next(); + $stream->expect(Token::OPERATOR_TYPE, '='); + + switch ($k) { + case 'package': + $node->setNode('package', $expressionParser->parseExpression()); + break; + case 'version': + $node->setNode('version', $expressionParser->parseExpression()); + break; + default: + throw new SyntaxError(\sprintf('Unknown "%s" option.', $k), $stream->getCurrent()->getLine(), $stream->getSourceContext()); + } + } - $this->parser->getStream()->expect(Token::BLOCK_END_TYPE); + $stream->expect(Token::BLOCK_END_TYPE); - return new DeprecatedNode($expr, $token->getLine(), $this->getTag()); + return $node; } public function getTag(): string diff --git a/src/TwigFilter.php b/src/TwigFilter.php index c02469b916f..2b80df9399b 100644 --- a/src/TwigFilter.php +++ b/src/TwigFilter.php @@ -46,6 +46,7 @@ public function __construct(string $name, $callable = null, array $options = []) 'preserves_safety' => null, 'node_class' => FilterExpression::class, 'deprecated' => false, + 'deprecating_package' => '', 'alternative' => null, ], $options); } @@ -128,6 +129,11 @@ public function isDeprecated(): bool return (bool) $this->options['deprecated']; } + public function getDeprecatingPackage(): string + { + return $this->options['deprecating_package']; + } + public function getDeprecatedVersion(): string { return \is_bool($this->options['deprecated']) ? '' : $this->options['deprecated']; diff --git a/src/TwigFunction.php b/src/TwigFunction.php index c15def63303..bfee7eb87e0 100644 --- a/src/TwigFunction.php +++ b/src/TwigFunction.php @@ -44,6 +44,7 @@ public function __construct(string $name, $callable = null, array $options = []) 'is_safe_callback' => null, 'node_class' => FunctionExpression::class, 'deprecated' => false, + 'deprecating_package' => '', 'alternative' => null, ], $options); } @@ -116,6 +117,11 @@ public function isDeprecated(): bool return (bool) $this->options['deprecated']; } + public function getDeprecatingPackage(): string + { + return $this->options['deprecating_package']; + } + public function getDeprecatedVersion(): string { return \is_bool($this->options['deprecated']) ? '' : $this->options['deprecated']; diff --git a/src/TwigTest.php b/src/TwigTest.php index 3769ec162b6..0b43a284906 100644 --- a/src/TwigTest.php +++ b/src/TwigTest.php @@ -38,6 +38,7 @@ public function __construct(string $name, $callable = null, array $options = []) 'is_variadic' => false, 'node_class' => TestExpression::class, 'deprecated' => false, + 'deprecating_package' => '', 'alternative' => null, 'one_mandatory_argument' => false, ], $options); @@ -83,6 +84,11 @@ public function isDeprecated(): bool return (bool) $this->options['deprecated']; } + public function getDeprecatingPackage(): string + { + return $this->options['deprecating_package']; + } + public function getDeprecatedVersion(): string { return \is_bool($this->options['deprecated']) ? '' : $this->options['deprecated']; diff --git a/tests/Fixtures/tags/deprecated/with_package.legacy.test b/tests/Fixtures/tags/deprecated/with_package.legacy.test new file mode 100644 index 00000000000..877643f01f4 --- /dev/null +++ b/tests/Fixtures/tags/deprecated/with_package.legacy.test @@ -0,0 +1,10 @@ +--TEST-- +Deprecating a template with "deprecated" tag +--TEMPLATE-- +{% deprecated 'The "index.twig" template is deprecated, use "greeting.twig" instead.' package="foo/bar" %} + +Hello Fabien +--DATA-- +return [] +--EXPECT-- +Hello Fabien diff --git a/tests/Fixtures/tags/deprecated/with_package_version.legacy.test b/tests/Fixtures/tags/deprecated/with_package_version.legacy.test new file mode 100644 index 00000000000..68722994e04 --- /dev/null +++ b/tests/Fixtures/tags/deprecated/with_package_version.legacy.test @@ -0,0 +1,10 @@ +--TEST-- +Deprecating a template with "deprecated" tag +--TEMPLATE-- +{% deprecated 'The "index.twig" template is deprecated, use "greeting.twig" instead.' package="foo/bar" version=1.1 %} + +Hello Fabien +--DATA-- +return [] +--EXPECT-- +Hello Fabien diff --git a/tests/Node/DeprecatedTest.php b/tests/Node/DeprecatedTest.php index 90e958cb4dc..440bd520e52 100644 --- a/tests/Node/DeprecatedTest.php +++ b/tests/Node/DeprecatedTest.php @@ -39,25 +39,29 @@ public function getTests() $expr = new ConstantExpression('This section is deprecated', 1); $node = new DeprecatedNode($expr, 1, 'deprecated'); $node->setSourceContext(new Source('', 'foo.twig')); + $node->setNode('package', new ConstantExpression('twig/twig', 1)); + $node->setNode('version', new ConstantExpression('1.1', 1)); $tests[] = [$node, <<setSourceContext(new Source('', 'foo.twig')); + $dep->setNode('package', new ConstantExpression('twig/twig', 1)); + $dep->setNode('version', new ConstantExpression('1.1', 1)); $tests[] = [$node, <<setSourceContext(new Source('', 'foo.twig')); + $node->setNode('package', new ConstantExpression('twig/twig', 1)); + $node->setNode('version', new ConstantExpression('1.1', 1)); $compiler = $this->getCompiler($environment); $varName = $compiler->getVarName(); @@ -75,7 +81,7 @@ public function getTests() $tests[] = [$node, <<addFunction(new TwigFunction('deprec', [$this, 'deprec'], ['deprecated' => '1.1'])); + $twig->addFunction(new TwigFunction('deprec', [$this, 'deprec'], ['deprecated' => '1.1', 'deprecating_package' => 'foo/bar'])); $collector = new DeprecationCollector($twig); $deprecations = $collector->collect(new Iterator()); - $this->assertEquals(['Twig Function "deprec" is deprecated since version 1.1 in deprec.twig at line 1.'], $deprecations); + $this->assertEquals(['Since foo/bar 1.1: Twig Function "deprec" is deprecated in deprec.twig at line 1.'], $deprecations); } public function deprec()