Add static code analysis and/or linting to improve code quality #4003
Comments
|
Static analysis is definitely much more involved than a linter. And this is a whole project on its own. The current Twig team (which is a way of saying "Fabien Potencier" actually, with a bit a help from Nicolas and me for triaging issues) does not have the resources of creating and maintaining a static analysis tool. So it would have to be created by someone else (ready to invest quite some time in such a project). |
|
Bonjour stof, Thanks for getting back to me so quickly. I understand this is out of scope for the core maintainer team. I started writing an extension with a NodeVisitor which already showed me a few issues in our code. I am lacking some knowledge on the internals and could use some help though. I'll probably open a repo to develop this extension tomorrow. I'll post a link here, of course. Could you help raise some awareness so this project can get some contributions? I'll reach out in our company's network to raise awareness. |
These nodes have no effect on compiled templates. Adding these to the TokenStream and AST enables extensions to add logic in node visitors, such as parsing PHPDoc-style `@var string name` comments. This in turn is invaluable for static code analysis of Twig templates as briefly discussed in [twigphp#4003](twigphp#4003) and in more detail in [TwigStan](https://github.com/alisqi/twig-stan/)'s README.
|
Hey stof and @fabpot, I've created an extension as discussed above: TwigQI 🎉 It already has two working inspections, one of which will forever prevent the very mistake that sparked my interest in static code analysis for Twig templates: use of undeclared/undefined variables inside One prerequisite for most other envisioned inspections is for the parser to add comment nodes to the AST. I've implemented (and motivated) this in #4009. I think we should close this issue (since static code analysis is outside of Twig's scope) and continue the conversation in the PR. |
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.
Hi, you might be interested by https://github.com/VincentLanglet/Twig-CS-Fixer |
…h an attribute in AST (drjayvee)
This PR was squashed before being merged into the 3.x branch.
Discussion
----------
Mark implicit macro argument default values as such with an attribute in AST
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 `is_implicit`, 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](#4003).
For example, a static analysis tool might consider arguments with no explicit default value as non-optional.
Commits
-------
e83a802 Mark implicit macro argument default values as such with an attribute in AST
|
Closing here as we now have the |
|
For those wondering what the |
I'd love to use static code analysis and/or linting for our logic-heavy templates to prevent silly mistakes from breaking production. (Asking for a friend 😉)
Compiling templates catches syntax errors, at least. But there are other types of bugs which aren't detected this way.
For example, suppose that a macro uses an undeclared variable (i.e., not a macro argument or declared using
{% set %}). This is almost certainly a mistake.strict_variableswill throw an Exception, of course, but only if the code is executed. Withoutstrict_variables, Twig will usenull, but that can easily lead to an Exception when passing it as a non-nullable argument.I searched for separate linters, but only found tools that either focus on HTML (curlylint and djLint) or are deprecated (phpstan-twig-rules and twig-lint).
I see two ways of increasing code quality:
twig/twigpackage or separatelyThe text was updated successfully, but these errors were encountered: