Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add compile-time checks for the "matches" operator #4330

Merged
merged 1 commit into from
Sep 23, 2024
Merged

Add compile-time checks for the "matches" operator #4330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/Node/Expression/Binary/MatchesBinary.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,27 @@
namespace Twig\Node\Expression\Binary;

use Twig\Compiler;
use Twig\Error\SyntaxError;
use Twig\Node\Node;
use Twig\Node\Expression\ConstantExpression;

class MatchesBinary extends AbstractBinary
{
public function __construct(Node $left, Node $right, int $lineno)
{
if ($right instanceof ConstantExpression) {
$regexp = $right->getAttribute('value');
set_error_handler(static fn ($t, $m) => throw new SyntaxError(\sprintf('Regexp "%s" passed to "matches" is not valid: %s.', $regexp, substr($m, 14)), $lineno));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The number 14 feels like a bit magical to me, can we add a comment or use strlen('...') (which will be inlined to 14) instead?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also somehow doesn't look right:

Twig\Error\RuntimeError: Regexp "3" passed to "matches" is not valid: Delimiter must not be alphanumeric%sbackslash%sin "index.twig" at line 2

It seems like there's at least a space missing before in "index.twig" at line 2.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message is correct, here, this is only the format to cover differences between PHP versions.

try {
preg_match($regexp, '');
} finally {
restore_error_handler();
}
}

parent::__construct($left, $right, $lineno);
}

public function compile(Compiler $compiler): void
{
$compiler
Expand Down
2 changes: 1 addition & 1 deletion src/Test/IntegrationTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ protected function doIntegrationTest($file, $message, $condition, $templates, $e
$output = trim($template->render(eval($match[1].';')), "\n ");
} catch (\Exception $e) {
if (false !== $exception) {
$this->assertSame(trim($exception), trim(\sprintf('%s: %s', \get_class($e), $e->getMessage())));
$this->assertStringMatchesFormat(trim($exception), trim(\sprintf('%s: %s', \get_class($e), $e->getMessage())));

return;
}
Expand Down
4 changes: 4 additions & 0 deletions tests/Fixtures/expressions/matches.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,9 @@
Twig supports the "matches" operator
--TEMPLATE--
{{ 'foo' matches '/o/' ? 'OK' : 'KO' }}
{{ 'foo' matches '/o/'|lower ? 'OK' : 'KO' }}
{{ 'foo' matches '/^fo/' ? 'OK' : 'KO' }}
{{ 'foo' matches '/^' ~ 'fo/' ? 'OK' : 'KO' }}
{{ 'foo' matches '/O/i' ? 'OK' : 'KO' }}
{{ null matches '/o/' }}
--DATA--
Expand All @@ -11,4 +13,6 @@ return []
OK
OK
OK
OK
OK
0
2 changes: 1 addition & 1 deletion ...s/Fixtures/expressions/matches_error.test → ...xpressions/matches_error_compilation.test
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ Twig supports the "matches" operator with a great error message
--DATA--
return []
--EXCEPTION--
Twig\Error\RuntimeError: Regexp "/o" passed to "matches" is not valid: No ending delimiter '/' found in "index.twig" at line 2
Twig\Error\SyntaxError: Regexp "/o" passed to "matches" is not valid: No ending delimiter '/' found in "index.twig" at line 2.
8 changes: 8 additions & 0 deletions tests/Fixtures/expressions/matches_error_runtime.test
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
--TEST--
Twig supports the "matches" operator with a great error message
--TEMPLATE--
{{ 'foo' matches 1 + 2 }}
--DATA--
return []
--EXCEPTION--
Twig\Error\RuntimeError: Regexp "3" passed to "matches" is not valid: Delimiter must not be alphanumeric%sbackslash%sin "index.twig" at line 2