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.

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

Mark implicit macro argument default values as such with an attribute #4010

Open
wants to merge 1 commit into
base: 3.x
Choose a base branch
from

Conversation

drjayvee
Copy link
Contributor

This change causes no difference to compiled templates or to macro argument semantics.

Consider the following macro:

{% 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.

For example, a static analysis tool might consider arguments with no explicit default value as non-optional.

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.
@drjayvee
Copy link
Contributor Author

Similar to #4009, this is a prerequisite for a desired static analysis inspection in TwigStan.

I can extend this feature by adding this attribute to every function/macro argument to make it more consistent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant