Skip to content

Commit

Permalink
Simplify TypesNode by storing types mapping in array attribute
Browse files Browse the repository at this point in the history
  • Loading branch information
@drjayvee
drjayvee committed Aug 29, 2024
1 parent 32df7eb commit d58483a
Show file tree
Hide file tree
Showing 4 changed files with 46 additions and 167 deletions.
34 changes: 5 additions & 29 deletions src/Node/TypesNode.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@

use Twig\Attribute\YieldReady;
use Twig\Compiler;
use Twig\Node\Expression\ArrayExpression;
use Twig\Node\Expression\ConstantExpression;
use Twig\Node\Expression\NameExpression;

/**
* Represents a types node.
Expand All @@ -16,33 +13,12 @@
#[YieldReady]
class TypesNode extends Node implements NodeCaptureInterface
{
public function __construct(ArrayExpression $typesNode, int $lineno)
/**
* @param array<string, array{type: string, optional: bool}> $types
*/
public function __construct(array $types, int $lineno)
{
$this->validateMapping($typesNode);

parent::__construct(['mapping' => $typesNode], [], $lineno);
}

protected function validateMapping(ArrayExpression $typesNode): void
{
foreach ($typesNode->getKeyValuePairs() as $i => $pair) {
$keyExpression = $pair['key'];
$valueExpression = $pair['value'];

if (!$keyExpression instanceof NameExpression) {
throw new \InvalidArgumentException("Key at index $i is not a NameExpression");
}
$name = $keyExpression->getAttribute('name');

if (!$valueExpression instanceof ConstantExpression) {
throw new \InvalidArgumentException("Value for key \"$name\" is not a ConstantExpression");
}
$value = $valueExpression->getAttribute('value');

if (!is_string($value)) {
throw new \InvalidArgumentException("Value for key \"$name\" is not a string");
}
}
parent::__construct([], ['mapping' => $types], $lineno);
}

public function compile(Compiler $compiler)
Expand Down
27 changes: 11 additions & 16 deletions src/TokenParser/TypesTokenParser.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,6 @@
namespace Twig\TokenParser;

use Twig\Error\SyntaxError;
use Twig\ExpressionParser;
use Twig\Node\Expression\ArrayExpression;
use Twig\Node\Expression\ConstantExpression;
use Twig\Node\Expression\NameExpression;
use Twig\Node\Node;
use Twig\Node\TypesNode;
use Twig\Token;
Expand All @@ -35,22 +31,22 @@ public function parse(Token $token): Node
{
$stream = $this->parser->getStream();

$expression = $this->parseSimpleMappingExpression($stream);
$types = $this->parseSimpleMappingExpression($stream);

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

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

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

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

$first = true;
while (!$stream->test(Token::PUNCTUATION_TYPE, '}')) {
Expand All @@ -65,21 +61,20 @@ private function parseSimpleMappingExpression(TokenStream $stream): ArrayExpress
$first = false;

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

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

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

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

$node->addElement($valueExpression, $nameExpression);

$types[$nameToken->getValue()] = [
'type' => $valueToken->getValue(),
'optional' => $isOptional,
];
}
$stream->expect(Token::PUNCTUATION_TYPE, '}', 'An opened mapping is not properly closed');

return $node;
return $types;
}

public function getTag(): string
Expand Down
96 changes: 13 additions & 83 deletions tests/Node/TypesTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,102 +2,32 @@

namespace Twig\Tests\Node;

use Twig\Node\Expression\ArrayExpression;
use Twig\Node\Expression\ConstantExpression;
use Twig\Node\Expression\NameExpression;
use Twig\Node\TypesNode;
use Twig\Test\NodeTestCase;

class TypesTest extends NodeTestCase
{
/** @return ArrayExpression */
private function getValidMapping()
{
// {foo: 'string', bar: 'int'}
return new ArrayExpression([
new NameExpression('foo', 1),
new ConstantExpression('string', 1),

new NameExpression('bar', 1),
new ConstantExpression('int', 1),
], 1);
}

public function testConstructor()
{
$types = $this->getValidMapping();
$node = new TypesNode($types, 1);

$this->assertEquals($types, $node->getNode('mapping'));
}

/** @return array<array<ArrayExpression>> */
public function getInvalidMappings()
private function getValidMapping(): array
{
// {foo: 'string', bar?: 'int'}
return [
// {'foo': string}
[
new ArrayExpression([
new ConstantExpression('foo', 1),
new ConstantExpression('string', 1),
], 1),
'Key at index 0 is not a NameExpression'
],

// [13, 37]
[
new ArrayExpression([
new ConstantExpression(13, 1),
new ConstantExpression(37, 1),
], 1),
'Key at index 0 is not a NameExpression'
],

// {foo: bar}
[
new ArrayExpression([
new NameExpression('foo', 1),
new NameExpression('bar', 1),
], 1),
'Value for key "foo" is not a ConstantExpression'
],

// {foo: true}
[
new ArrayExpression([
new NameExpression('foo', 1),
new ConstantExpression(true, 1),
], 1),
'Value for key "foo" is not a string'
],

// {foo: 123}
[
new ArrayExpression([
new NameExpression('foo', 1),
new ConstantExpression(123, 1),
], 1),
'Value for key "foo" is not a string'
],

// {foo: {}}}
[
new ArrayExpression([
new NameExpression('foo', 1),
new ConstantExpression(new ArrayExpression([], 1), 1),
], 1),
'Value for key "foo" is not a string'
'foo' => [
'type' => 'string',
'optional' => false,
],
'bar' => [
'type' => 'int',
'optional' => true,
]
];
}

/** @dataProvider getInvalidMappings */
public function testConstructorThrowsOnInvalidMapping(ArrayExpression $mapping, string $message)
public function testConstructor()
{
$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage($message);
$types = $this->getValidMapping();
$node = new TypesNode($types, 1);

new TypesNode($mapping, 1);
$this->assertEquals($types, $node->getAttribute('mapping'));
}

public function getTests()
Expand Down
56 changes: 17 additions & 39 deletions tests/TokenParser/TypesTokenParserTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,21 @@
use Twig\Environment;
use Twig\Loader\ArrayLoader;
use Twig\Node\Expression\ArrayExpression;
use Twig\Node\Expression\ConstantExpression;
use Twig\Node\Expression\NameExpression;
use Twig\Parser;
use Twig\Source;

class TypesTokenParserTest extends TestCase
{
/** @dataProvider getMappingTests */
public function testMappingParsing(string $template, ArrayExpression $expected): void
public function testMappingParsing(string $template, array $expected): void
{
$env = new Environment(new ArrayLoader(), ['cache' => false, 'autoescape' => false]);
$stream = $env->tokenize($source = new Source($template, ''));
$parser = new Parser($env);
$expected->setSourceContext($source);

$typesNode = $parser->parse($stream)->getNode('body')->getNode('0');

self::assertEquals($expected, $typesNode->getNode('mapping'));
self::assertEquals($expected, $typesNode->getAttribute('mapping'));
}

public function getMappingTests(): array
Expand All @@ -32,61 +29,42 @@ public function getMappingTests(): array
// empty mapping
[
'{% types {} %}',
new ArrayExpression([], 1),
[],
],

// simple
[
'{% types {foo: "bar"} %}',
new ArrayExpression([
$this->createNameExpression('foo', false),
new ConstantExpression('bar', 1),
], 1),
['foo' => 'bar'],
[
'foo' => ['type' => 'bar', 'optional' => false]
],
],

// trailing comma
[
'{% types {foo: "bar",} %}',
new ArrayExpression([
$this->createNameExpression('foo', false),
new ConstantExpression('bar', 1),
], 1),
['foo' => 'bar'],
[
'foo' => ['type' => 'bar', 'optional' => false]
],
],

// optional name
[
'{% types {foo?: "bar"} %}',
new ArrayExpression([
$this->createNameExpression('foo', true),
new ConstantExpression('bar', 1),
], 1),
['foo?' => 'bar'],
[
'foo' => ['type' => 'bar', 'optional' => true]
],
],

// multiple pairs, duplicate values
[
'{% types {foo: "foo", bar?: "foo", baz: "baz"} %}',
new ArrayExpression([
$this->createNameExpression('foo', false),
new ConstantExpression('foo', 1),

$this->createNameExpression('bar', true),
new ConstantExpression('foo', 1),

$this->createNameExpression('baz', false),
new ConstantExpression('baz', 1),
], 1),
['foo' => 'foo', 'bar?' => 'foo', 'baz' => 'baz'],
[
'foo' => ['type' => 'foo', 'optional' => false],
'bar' => ['type' => 'foo', 'optional' => true],
'baz' => ['type' => 'baz', 'optional' => false]
],
],
];
}

private function createNameExpression(string $name, bool $isOptional): NameExpression
{
$name = new NameExpression($name, 1);
$name->setAttribute('is_optional', $isOptional);
return $name;
}
}

0 comments on commit d58483a

Please sign in to comment.