Twig icon indicating copy to clipboard operation
Twig copied to clipboard

Add static code analysis and/or linting to improve code quality

Open drjayvee opened this issue 11 months ago • 4 comments

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_variables will throw an Exception, of course, but only if the code is executed. Without strict_variables, Twig will use null, 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:

  1. Separate linter CLI, either included in the main twig/twig package or separately
  2. An extension which adds a token parser (or node visitor?) and throws exceptions at compile-time

drjayvee avatar Feb 29 '24 13:02 drjayvee

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).

stof avatar Feb 29 '24 14:02 stof

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.

drjayvee avatar Feb 29 '24 15:02 drjayvee

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 macros.

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.

drjayvee avatar Mar 16 '24 13:03 drjayvee

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:

  1. Separate linter CLI, either included in the main twig/twig package or separately
  2. An extension which adds a token parser (or node visitor?) and throws exceptions at compile-time

Hi, you might be interested by https://github.com/VincentLanglet/Twig-CS-Fixer

VincentLanglet avatar Aug 17 '24 14:08 VincentLanglet

Closing here as we now have the types tag native in Twig.

fabpot avatar Sep 10 '24 06:09 fabpot

For those wondering what the types tag is : https://twig.symfony.com/doc/3.x/tags/types.html

mathroc avatar Sep 10 '24 10:09 mathroc