Adopt vincentlanglet/twig-cs-fixer for Twig coding standards

This will need a dependency evaluation.

Twig Coding Standard (twig-cs-fixer) found 120 errors, check attachment.

vincentlanglet/twig-cs-fixer follow the official Twig Coding Standard Rules

How to use:

1. Check errors: vendor/bin/twig-cs-fixer lint /path/to/code
2. Fix errors: vendor/bin/twig-cs-fixer lint --fix /path/to/code

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

MR !5248 adds vincentlanglet/twig-cs-fixer instead of friendsoftwig/twigcs about which is this issue.

That being said, friendsoftwig/twigcs maintainer is looking for someone to take over the maintainership and considers putting the project in the EOL status if no one will step up.
https://github.com/friendsoftwig/twigcs/issues/304

One alternative they point to is vincentlanglet/twig-cs-fixer (which MR !5248 adds).
Personally, I also prefer vincentlanglet/twig-cs-fixer since it also includes automatic fixes. We already using it for some time at work in Drupal based projects, and it works great.

Looks like `friendsoftwig/twigcs` will be EOL soon and `vincentlanglet/twig-cs-fixer` is a promising replacement.

Should we change the title and description to be more generic?

Yes, vincentlanglet/twig-cs-fixer is a tool with more adoption, more maintained and is more aligned with drupal releases in aspects like phpstan and following symfony releases.

I don't see the failure in the current PR but anyway it needs a rebase.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

The Needs Review Queue Bot tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

#2 is still not addressed; this issue could be a whole lot of wasted effort if the dependency doesn't pass its dependency evaluation.

I recommend ceasing rerolling/rebasing/etc. and instead completing the dependency evaluation, then tagging it "Needs release manager review" and "Needs frontend framework manager review", and only continuing work on the MR itself if or when both those signoffs have been given for the change.

Edit: For examples of useful dependency evaluations, see #3326239: Add phpstan/phpstan-phpunit as a dev dependency or especially #2550717: [JS] Replace jQuery.cookie with JS-cookie and provide a BC layer. These have some good examples of what information to provide and summarize. I'm going to add those to the handbook page as references since the docs lack a cohesive example.

Also things with lockfile changes are never good candidates for the friendly bot as they break constantly, and it's not worth rerolling something forever without other progress. So tagging so the bot leaves it alone. Thanks!

Took an initial stab at the dependency evaluation. Only thing that seems to be missing is the security policy – opened https://github.com/VincentLanglet/Twig-CS-Fixer/issues/255

Very nice work @Feuerwagen! Setting NR for review.

My security policy proposal got adopted, see https://github.com/VincentLanglet/Twig-CS-Fixer/security/policy

Personally think the evaluation contains all good information.

Think this would be a welcome addition, have a current project I'm scared how much will get flagged.

I'm marking RTBC to get in front of the various managers :)

+1 for this.

I've started using this on a client site and had to customize it a bit for Drupal:

For spacing:

$ruleset->overrideRule(new TwigCsFixer\Rules\Whitespace\IndentRule(
    spaceRatio : 2,
    useTab : FALSE,
));

And for now I've had to exclude templates that use {% trans %} because it isn't defined in the default rules, and I couldn't get the ignore next line comments to work.

Conflicts are resolved, tests are passing, twig-cs-fixer is moved to require-dev. Seems like everything is addressed.

Per https://www.drupal.org/project/drupal/issues/3284817#comment-15729173, I have had the same issue with templates that utilize {% trans %} tags as well.

• The Symfony Twig documentation indicates that the proper syntax to use is {% trans %}Submitted by %author.name% on %node.date%{% endtrans %}. This syntax validates.
• Drupal's documentation indicates that the syntax to use for the {% trans %} tags is {% trans %} Submitted by {{ author.name }} on {{ node.date }} {% endtrans %}, which does not validate.

Which is correct?

If it is the former, before we adopt this we should correct our documentation, and IMO this is worth noting in the release notes.

Another issue that this linter detected as incorrect syntax has to do with include tags. Valid use of {% include %} is auto-corrected to change the syntax to utilize the "include" function instead. However the use of {% include %} tags, while a valid syntax, is flagged with the following message:

>> | Include function must be used instead of include tag.

Which is not entirely correct either.

Restoring to RTBC.

I am also getting the following error while trying to research this.

Error: Tag "trans" is already registered.
Failed to execute command vendor/bin/twig-cs-fixer lint --fix docroot/modules/custom --debug: exit status 2

I found that commenting out this line allowed me to run it.

$config->addTokenParser(new Drupal\Core\Template\TwigTransTokenParser());

@thejimbirch
could you please provide more information on when and how you got that error.

I just tested on TwigCSFixer 3.0.1, 3.0.3, 3.1.0 on D11.0.4 with and without $config->addTokenParser(new Drupal\Core\Template\TwigTransTokenParser()); on the following code

{% trans %} Submitted by {{ author.name }} on {{ node.date }} {% endtrans %}
{% trans %}Submitted by %author.name% on %node.date%{% endtrans %}

and it passed in all cases without issues.

@alex.skrypnyk, thanks for your testing. Since #27 when I had v2 installed, I've updated to v3 and retested. Now all the lines with trans pass. I can remove the folder exclusions that I had as a temporary workaround.

+1 on linting twig files and on the specific tool considered.

Merge conflict in the MR + a few comments.

Will try to do the rebase + taking review into account.

Hello,

New MR created because previous one was too old and with pull merge commits so impossible to rebase.

I have:
- manually reapplied changes on files in git rebase error/conflict
- updated Twigcs to 3.9.0 (previously 3.0.1), relaunched an automated fix on Twig files
- updated MR to take nod_ review comments into account about position of config file and better CI integration.

Happy review!

Not sure why my comment on GitLab shows up as user/0 on here, but the last comment was me! Nevermind! The above comment got updated with my avatar.

I've implemented Twig linting with this tool on a project recently using a custom config.

I'm currently looking at the docs https://github.com/VincentLanglet/Twig-CS-Fixer/blob/main/docs/configura... to see how that affects the linted files in core.

Check errors: vendor/bin/twig-cs-fixer lint /path/to/code
Fix errors: vendor/bin/twig-cs-fixer lint --fix /path/to/code

Should we add a "lint:twig" script to core/package.json? That would make it easier to see that we have a Twig lint task, but it's not a Node.js package, so… Eh. composer.json also has a "scripts" section, but it doesn't look like its used much. Also all our other lint tasks are in the package.json.

"lint:twig": "../vendor/bin/twig-cs-fixer lint .",

I've added a couple upstream issues because Drupal's current Twig files seem to prefer (but not consistently) to have spaces around inline arrays:

{{ attributes.addClass([ 'foo', 'bar' ]) gets rewritten to {{ attributes.addClass(['foo', 'bar']) by twig-cs-fixer.

I don't think we should wait on the new upstream issue: https://github.com/VincentLanglet/Twig-CS-Fixer/issues/401 But I wanted to give us options if we decide we like the spaces.

I also created https://github.com/VincentLanglet/Twig-CS-Fixer/issues/402 because I prefer having using a named parameter in the include() function: {{ include('template.twig', {}, with_context: false) }} instead of twig-cs-fixer's default {{ include('template.twig', {}, false) }}. Again, we don't need to wait for that upstream to be fixed either.

In the MR, I made these changes:

• added comments to the .twig-cs-fixer.php config file
• added CompactHashRule and IndentRule overrides to the config
• changed twigcs-tmp/twigcs.cache and other "twigcs" variables to use the full name of the tool since all the other eslint, stylelint, phpstan variables/folders use the exact tool name in our various CI scripts. twig-cs-fixer-tmp/twig-cs-fixer.cache, _TWIG_CS_FIXER_CONFIG, etc.
• renamed Olivero's listClasses variable to use snake case and to reflect that item_classes is applied to list items not to the containing list element.
• Ensured that existing usage of {{ include() }} uses , with_context: false instead of just , false

It looks like the GitLab CI changes are working. I haven't tested all the options though. Nor have I tested the commit-code-check.sh script.

Hi,

Thanks for pushing the MR forward.

My personal preferences (regarding your last comment):
- {{ attributes.addClass(['foo', 'bar']): so the opposite of current state of Drupal files, and what TwigCS does by default.
- {{ include('template.twig', {}, with_context: false) }}: agreed with you.

Not sure if TwigCS configuration had been discussed in this issue or in another one.

So seems like it’s downloading 3.0.1 but shouldn’t it be updated to 3.10.

Realize with the rate the ticket has moved getting the most recent could be tough but does seem to be a dozen versions back

By the way, the Prettier Plugin for Twig has an issue with formatting constructions like <div{{ attributes }}> - more details here: https://github.com/zackad/prettier-plugin-twig/issues/139

So, if we're fixing there other styling issues, maybe fix this one too?

P.S. I need to use the Prettier Plugin for Twig to get automatic formatting in VS Code, cuz still can't manage to make this work with "twig-cs-fixer".

Update to the typical format for dependency evaluation. And the dependency evaluation needs to be updated, 2.x is no longer supported.

My personal preferences (regarding your last comment):

- {{ attributes.addClass(['foo', 'bar']): so the opposite of current state of Drupal files, and what TwigCS does by default.

Honestly, that works for me as well. Also the maintainer showed me a way to fix this if we wanted to go with the extra spaces for inline arrays, too. So the https://github.com/VincentLanglet/Twig-CS-Fixer/issues/401 issue was closed after they explained the config option.

- {{ include('template.twig', {}, with_context: false) }}: agreed with you.

Unfortunately, the maintainer closed that issue, [edit: fixed link, re: next comment] but I agree with their reasoning. What I was asking for implied a very complicated configuration. I wasn't asking to "use named parameters or not"; I was asking for "use named parameters only for a specific list of parameter names on specific functions".

Fortunately, the tool will not rewrite the include function if we use {{ include('template.twig', {}, with_context: false) }} in Drupal's Twig files. We just have to ensure consistency when we write include().

Re: Prettier Plugin for Twig. I'll have to test that these two tools are compatible with each other. We wouldn't want each tool to rewrite the output of the other tool. If we run our Twig files through both tools first, then both tools should no longer find fault with the source. And ideally, the order of running the two tools won't create a different Twig format. I'll try to find some time to test this as well.

Hi,

Thanks for pushing this forward!

In the previous comment, I think the second link to Github issue was meant to target: https://github.com/VincentLanglet/Twig-CS-Fixer/issues/402

Because currently both links target the same issue.

Re: Prettier Plugin for Twig. I'll have to test that these two tools are compatible with each other.

Right now, I don't think we can consider using this Prettier plugin for Drupal Core. For these reasons:

1. While it has had several releases since the project creation last year, all of those releases are prereleases. It doesn't have a 1.0.0 release so semver says they can break things with each point release.
2. It mentions following Twig coding standards on the project homepage, but then links to Twig 2.x docs instead of Twig 3.x. That might be a mistake, but there's a very big "Version 2.x is not maintained" notice at the top of that page.
3. The official Twig 3.x docs specifically mention the Twig CS fixer tool at the top of their Coding Standards page.

After noticing that, I decided not to test if the two tools are compatible.

We can certainly revisit later, But I'd like to see a few post 1.0.0 releases of the Prettier plugin before considering it again.

I will try to push forward this issue for DDD 2026

Update dependency evaluation.

New MR created https://git.drupalcode.org/project/drupal/-/merge_requests/15514 because previous one https://git.drupalcode.org/project/drupal/-/merge_requests/13270 rebase was quite hard due to a merge commit.

- changes done in core/.twig-cs-fixer.php preserved
- changes in .gitlab-ci.yml done again manually because logic in other CI tasks changed.
- changes in composer.json properly applied from https://www.drupal.org/about/core/policies/core-dependency-policies-and-...
- changes in core/scripts/dev/commit-code-check.sh done again manually because logic in the file changed.
- search and replace done for ", false)" to ", with_context: false)".
- issue summary updated regarding new dependency evaluation.

Back to needs review as pipeline failure is unrelated to the changes.

TODO:
- add a command in composer.json like with phpcs?
- should a fix command be provided?
- Comment 46 about Prettier may need a dedicated follow-up issue.

I will be present at DDD 2026, if some people want to review and/or we can sit together to hopefully finish this issue. Because rebasing this issue is quite time consuming regarding the number of impacted files.

MR rebased.

Unrelated test failure.

Rebased.

Adding scripts in composer.json as discussed with @johnalbin

Unrelated test failure in the pipeline

Ok. I went through all the non-twig files and everything looks good, including the gitlab config.

I then went through every file line by line and only found one change that needed to be made (an include function that should use named parameters). I pushed that change.

This is RTBC for me! 🎉

edit: Add GitLab CI is also green!

I need to update the gitlab CI so twig-cs-fixer only runs if Twig files have changed.

[edit: waiting on the CI to run with the new config.]

And back to RTBC

Thanks @johnalbin!

Thanks. Dependency evaluation (in "require-dev") also seems to be OK.

We are also formatting the Twig files in the MR, updating 176 of them, which may break ongoing MRs.
But Twig CS Fixer is a "light" formatter, doing some targeted changes without heavily messing with the files (like https://djlint.com/ does), and we are altering 1 or 2 lines by Twig file in average, so I guess it will be OK.

I am planning to merge this week.

I chatted with godotislate, about the meta data in this issue. They noticed that this is tagged for "Needs framework manager review". I am checking why it is tagged for that. This already has release manager and frontend framework manager signoff.

catch has confirmed that this does not need 'framework manager review'. The existing sign-off are sufficient.

Now this has made it to RBTC it's handy for the bot to tell you when it does not apply...

Which the current MR does not...

Hi,

Rebased using Gitlab UI.

As pointed on Slack by @longwave https://drupal.slack.com/archives/C07V7UZNVSM/p1777973740412579?thread_t..., as this issue impacts a lot of files it should be merged during beta phase of 12.0.0-beta1.

So I will rebase at this moment.

MR rebased.

I have linted after rebase, no change.
I have updated Twig CS Fixer to the V4.0.0 released yesterday.
I have linted after update, no change.

I am preparing an MR against 11.x

The Needs Review Queue Bot tested this issue. It fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

MR against 11.x created.

The tests failure seems unrelated.

Back to needs review.

Restoring previous status.

This looks good. I'm not really qualified to review the Twig changes but people are who are have done so. Nice to see the artifact caching implemented up front, we should double check that's all working once this is running on HEAD.

Committed/pushed to main and the respective MR to 11.x, thanks!

Adding the 'approved dependency evaluation' tag per policy, Dependency information links.

Thanks for the merge!

One less issue to rebase at almost every core commit :)

For info, @grimreaper created a Twig CS Fixer job for Contrib projects, and after further enhancements it is now ready for use once Core 11.5.0 is released.

I have opened a Coding Standards issue so that we can co-ordinate with the CS team about the plan/release/announcement.