Skip to content

Commit

Permalink
Use trigger_deprecation() instead of trigger_error() for deprecations
Browse files Browse the repository at this point in the history
  • Loading branch information
fabpot committed Aug 8, 2024
1 parent 9252578 commit 0ce79df
Show file tree
Hide file tree
Showing 13 changed files with 131 additions and 30 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG
Original file line number Diff line number Diff line change
@@ -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`
Expand Down
12 changes: 12 additions & 0 deletions doc/advanced.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
11 changes: 11 additions & 0 deletions doc/tags/deprecated.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 3 additions & 12 deletions src/ExpressionParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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();
Expand All @@ -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();
Expand Down
36 changes: 27 additions & 9 deletions src/Node/DeprecatedNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -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")
;
}
}
29 changes: 26 additions & 3 deletions src/TokenParser/DeprecatedTokenParser.php
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@

namespace Twig\TokenParser;

use Twig\Error\SyntaxError;
use Twig\Node\DeprecatedNode;
use Twig\Node\Node;
use Twig\Token;
Expand All @@ -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 <yonelceruto@gmail.com>
*
* @internal
Expand All @@ -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
Expand Down
6 changes: 6 additions & 0 deletions src/TwigFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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'];
Expand Down
6 changes: 6 additions & 0 deletions src/TwigFunction.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down Expand Up @@ -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'];
Expand Down
6 changes: 6 additions & 0 deletions src/TwigTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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'];
Expand Down
10 changes: 10 additions & 0 deletions tests/Fixtures/tags/deprecated/with_package.legacy.test
Original file line number Diff line number Diff line change
@@ -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
10 changes: 10 additions & 0 deletions tests/Fixtures/tags/deprecated/with_package_version.legacy.test
Original file line number Diff line number Diff line change
@@ -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
14 changes: 10 additions & 4 deletions tests/Node/DeprecatedTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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, <<<EOF
// line 1
@trigger_error("This section is deprecated"." (\"foo.twig\" at line 1).", E_USER_DEPRECATED);
trigger_deprecation("twig/twig", "1.1", "This section is deprecated"." in \"foo.twig\" at line 1.");
EOF
];

$t = new Node([
new ConstantExpression(true, 1),
new DeprecatedNode($expr, 2, 'deprecated'),
$dep = new DeprecatedNode($expr, 2, 'deprecated'),
], [], 1);
$node = new IfNode($t, null, 1);
$node->setSourceContext(new Source('', 'foo.twig'));
$dep->setNode('package', new ConstantExpression('twig/twig', 1));
$dep->setNode('version', new ConstantExpression('1.1', 1));

$tests[] = [$node, <<<EOF
// line 1
if (true) {
// line 2
@trigger_error("This section is deprecated"." (\"foo.twig\" at line 2).", E_USER_DEPRECATED);
trigger_deprecation("twig/twig", "1.1", "This section is deprecated"." in \"foo.twig\" at line 2.");
}
EOF
];
Expand All @@ -68,14 +72,16 @@ public function getTests()
$expr = new FunctionExpression('foo', new Node(), 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));

$compiler = $this->getCompiler($environment);
$varName = $compiler->getVarName();

$tests[] = [$node, <<<EOF
// line 1
\$$varName = foo();
@trigger_error(\$$varName." (\"foo.twig\" at line 1).", E_USER_DEPRECATED);
trigger_deprecation("twig/twig", "1.1", \$$varName." in \"foo.twig\" at line 1.");
EOF
, $environment];

Expand Down
4 changes: 2 additions & 2 deletions tests/Util/DeprecationCollectorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ class DeprecationCollectorTest extends TestCase
public function testCollect()
{
$twig = new Environment(new ArrayLoader());
$twig->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()
Expand Down

0 comments on commit 0ce79df

Please sign in to comment.