Skip to content

Commit

Permalink
Move parsing of {name: 'string'} mappings to ExpressionParser
Browse files Browse the repository at this point in the history 
This also adds support for optional keys as discussed in twigphp#4165.

I really don't like using a static method, but I couldn't figure out how
else to call the method in tests. See the notes on the method itself.

Any help is appreciated!
  • Loading branch information
@drjayvee
drjayvee committed Aug 28, 2024
1 parent a5a1d11 commit cde0426
Show file tree
Hide file tree
Showing 3 changed files with 183 additions and 40 deletions.
43 changes: 43 additions & 0 deletions src/ExpressionParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,49 @@ public function parseMappingExpression()
return $node;
}

/*
* This method is static to make it testable.
* Calling `$this->parser->getStream()` as done in `parseMappingExpression()` only works if
* `parser->parse()` has been called, but that won't ever call this method.
*/
public static function parseNameToStringMapping(TokenStream $stream, bool $allowOptionalKeys): ArrayExpression
{
$stream->expect(Token::PUNCTUATION_TYPE, '{', 'A mapping element was expected');

$node = new ArrayExpression([], $stream->getCurrent()->getLine());

$first = true;
while (!$stream->test(Token::PUNCTUATION_TYPE, '}')) {
if (!$first) {
$stream->expect(Token::PUNCTUATION_TYPE, ',', 'A mapping value must be followed by a comma');

// trailing ,?
if ($stream->test(Token::PUNCTUATION_TYPE, '}')) {
break;
}
}
$first = false;

$nameToken = $stream->expect(Token::NAME_TYPE);
$nameExpression = new NameExpression($nameToken->getValue(), $nameToken->getLine());

if ($allowOptionalKeys) {
$isOptional = $stream->nextIf(Token::PUNCTUATION_TYPE, '?') !== null;
$nameExpression->setAttribute('is_optional', $isOptional);
}

$stream->expect(Token::PUNCTUATION_TYPE, ':', 'A mapping key must be followed by a colon (:)');

$valueToken = $stream->expect(Token::STRING_TYPE);
$valueExpression = new ConstantExpression($valueToken->getValue(), $valueToken->getLine());

$node->addElement($valueExpression, $nameExpression);
}
$stream->expect(Token::PUNCTUATION_TYPE, '}', 'An opened mapping is not properly closed');

return $node;
}

public function parsePostfixExpression($node)
{
while (true) {
Expand Down
50 changes: 10 additions & 40 deletions src/TokenParser/TypesTokenParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,15 @@
/**
* Declare variable types.
*
* {% types {foo: 'int', bar: 'string'} %}
*
* The mapping must use names as keys (not strings) and type strings as values.
*
* Names may be suffixed with `?` to mark them as optional (i.e., the context may not
* have that variable defined).
*
* Example:
* ```
* {% types {foo: 'int', bar?: 'string'} %}
* ```
* @author Jeroen Versteeg <[email protected]>
* @see https://github.com/twigphp/Twig/issues/4165
* @internal
Expand All @@ -37,50 +44,13 @@ public function parse(Token $token): Node
{
$stream = $this->parser->getStream();

$expression = $this->parseSimpleMappingExpression($stream);
$expression = ExpressionParser::parseNameToStringMapping($stream, true);

$stream->expect(Token::BLOCK_END_TYPE);

return new TypesNode($expression, $token->getLine(), $this->getTag());
}

/**
* @throws SyntaxError
* @see ExpressionParser::parseMappingExpression()
*/
private function parseSimpleMappingExpression(TokenStream $stream): ArrayExpression
{
$stream->expect(Token::PUNCTUATION_TYPE, '{', 'A mapping element was expected');

$node = new ArrayExpression([], $stream->getCurrent()->getLine());

$first = true;
while (!$stream->test(Token::PUNCTUATION_TYPE, '}')) {
if (!$first) {
$stream->expect(Token::PUNCTUATION_TYPE, ',', 'A mapping value must be followed by a comma');

// trailing ,?
if ($stream->test(Token::PUNCTUATION_TYPE, '}')) {
break;
}
}
$first = false;

$nameToken = $stream->expect(Token::NAME_TYPE);
$nameExpression = new NameExpression($nameToken->getValue(), $nameToken->getLine());

$stream->expect(Token::PUNCTUATION_TYPE, ':', 'A mapping key must be followed by a colon (:)');

$valueToken = $stream->expect(Token::STRING_TYPE);
$valueExpression = new ConstantExpression($valueToken->getValue(), $valueToken->getLine());

$node->addElement($valueExpression, $nameExpression);
}
$stream->expect(Token::PUNCTUATION_TYPE, '}', 'An opened mapping is not properly closed');

return $node;
}

public function getTag(): string
{
return 'types';
Expand Down
130 changes: 130 additions & 0 deletions tests/ExpressionParserTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Twig\Attribute\FirstClassTwigCallableReady;
use Twig\Environment;
use Twig\Error\SyntaxError;
use Twig\ExpressionParser;
use Twig\Extension\AbstractExtension;
use Twig\Loader\ArrayLoader;
use Twig\Node\Expression\ArrayExpression;
Expand Down Expand Up @@ -208,6 +209,135 @@ public function getTestsForSequence()
];
}

/** @dataProvider getTestsForNameToStringMapping */
public function testNameToStringMapping(string $template, ArrayExpression $expected, bool $allowOptionalKeys): void
{
$env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
$stream = $env->tokenize(new Source($template, ''));
$stream->next();

$this->assertEquals($expected, ExpressionParser::parseNameToStringMapping($stream, $allowOptionalKeys));
}

public function getTestsForNameToStringMapping(): array
{
return [
// empty map
[
'{{ {} }}',
new ArrayExpression([], 1),
false
],

// two keys, empty string for 2nd value
[
"{{ {foo: 'bar', baz: ''} }}",
new ArrayExpression([
$this->createNameExpression('foo', []),
new ConstantExpression('bar', 1),

$this->createNameExpression('baz', []),
new ConstantExpression('', 1),
], 1),
false
],

// test trailing comma, allow optional keys (but without specifying any)
[
"{{ {foo: 'bar', baz: 'bloop',} }}",
new ArrayExpression([
$this->createNameExpression('foo', ['is_optional' => false]),
new ConstantExpression('bar', 1),

$this->createNameExpression('baz', ['is_optional' => false]),
new ConstantExpression('bloop', 1),
], 1),
true
],

// optional non-optional keys
[
"{{ {foo?: 'bar', baz: 'bloop'} }}",
new ArrayExpression([
$this->createNameExpression('foo', ['is_optional' => true]),
new ConstantExpression('bar', 1),

$this->createNameExpression('baz', ['is_optional' => false]),
new ConstantExpression('bloop', 1),
], 1),
true
],
];
}

/** @dataProvider getFailingTestsForNameToStringMapping */
public function testNameToStringMappingSyntaxError(string $template, bool $allowOptionalKeys): void
{
$this->expectException(SyntaxError::class);

$env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
$stream = $env->tokenize(new Source($template, ''));

ExpressionParser::parseNameToStringMapping($stream, $allowOptionalKeys);
}

public function getFailingTestsForNameToStringMapping(): array
{
return [
// missing closing brace
[
'{{ { }}',
false
],

// trailing comma without first element
[
'{{ , }}',
false
],

// no :
[
"{{ {'foo' = 'bar'} }}",
false
],

// string key
[
"{{ {'foo': 'bar'} }}",
false
],

// non-string values
[
"{{ {foo: bar} }}",
false
],
[
"{{ {foo: true} }}",
false
],
[
"{{ {foo: 1337} }}",
false
],
[
"{{ {foo: 13 + 37} }}",
false
],
[
"{{ {foo: {}} }}",
false
],

// optional key when not allowed
[
"{{ {foo?: 'bar'} }}",
false
],
];
}

public function testStringExpressionDoesNotConcatenateTwoConsecutiveStrings()
{
$this->expectException(SyntaxError::class);
Expand Down

0 comments on commit cde0426

Please sign in to comment.