Skip to content

Commit

Permalink
Mark implicit macro argument default values as such with an attribute
Browse files Browse the repository at this point in the history
This change causes no difference to compiled templates or to macro argument semantics.

Consider the following macro:
```twig
{% macro marco(po, lo = null) %}{% endmacro %}
```

With this change, the `ConstantExpression` for argument `po` will have an attribute `isImplicit`, whose value will be `true`. (Note that `lo` will not have that attribute.)

This allows node visitors to distinguish between arguments that do and those that do not have
explicit default values even if the value is `null`.

This is useful for [static code analysis](twigphp#4003).

For example, a static analysis tool might consider arguments with no explicit default value as non-optional.
  • Loading branch information
drjayvee committed Mar 16, 2024
1 parent b46e93c commit 6128528
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/ExpressionParser.php
Expand Up @@ -591,7 +591,7 @@ public function parseFilterExpressionRaw($node, $tag = null)
* Parses arguments.
*
* @param bool $namedArguments Whether to allow named arguments or not
* @param bool $definition Whether we are parsing arguments for a function definition
* @param bool $definition Whether we are parsing arguments for a function (or macro) definition
*
* @return Node
*
Expand Down Expand Up @@ -642,6 +642,7 @@ public function parseArguments($namedArguments = false, $definition = false, $al
if (null === $name) {
$name = $value->getAttribute('name');
$value = new ConstantExpression(null, $this->parser->getCurrentToken()->getLine());
$value->setAttribute('isImplicit', true);
}
$args[$name] = $value;
} else {
Expand Down
32 changes: 32 additions & 0 deletions tests/ParserTest.php
Expand Up @@ -14,6 +14,7 @@
use PHPUnit\Framework\TestCase;
use Twig\Environment;
use Twig\Error\SyntaxError;
use Twig\Lexer;
use Twig\Loader\LoaderInterface;
use Twig\Node\Node;
use Twig\Node\SetNode;
Expand Down Expand Up @@ -175,6 +176,37 @@ public function testGetVarName()
$this->addToAssertionCount(1);
}

public function testImplicitMacroArgumentDefaultValues()
{
$template = '{% macro marco (po, lo = null) %}{% endmacro %}';
$lexer = new Lexer(new Environment($this->createMock(LoaderInterface::class)));
$stream = $lexer->tokenize(new Source($template, 'index'));

$argumentNodes = $this->getParser()
->parse($stream)
->getNode('macros')
->getNode('marco')
->getNode('arguments')
;

self::assertTrue(
$argumentNodes->getNode('po')->hasAttribute('isImplicit')
);
self::assertTrue(
$argumentNodes->getNode('po')->getAttribute('isImplicit')
);
self::assertNull(
$argumentNodes->getNode('po')->getAttribute('value')
);

self::assertFalse(
$argumentNodes->getNode('lo')->hasAttribute('isImplicit')
);
self::assertNull(
$argumentNodes->getNode('lo')->getAttribute('value')
);
}

protected function getParser()
{
$parser = new Parser(new Environment($this->createMock(LoaderInterface::class)));
Expand Down

0 comments on commit 6128528

Please sign in to comment.