Rule Idea: Using named argument for include with_context or boolean twig extension calls

[alexander-schranz]

Rule(s) related to or rule(s) missing

Rule Idea.

Expected behavior

I'm using with_context = false a lot in my twig files. While updating twig cs fixer to the latest version I stumble over that existing {% include %} where migrated and just use , false for it. For more transparent I prefer to use named argument for the with_context parameter:

{{ include('include.html.twig', {}, with_context = false) }}

So I thought maybe it would be nice to have a rule for the with_context or even for all boolean based twig extensin calls.

Actual behavior

No rule yet:

{{ include('include.html.twig', {}, false) }}

[VincentLanglet]

Hi,

First, I'm not sure about having such rule here since it may be consider too much opinionated.

• One could prefer having no named param
• One could prefer having every named param
• You prefer to use named param for the third param only

Second, I'm not sure about how easy a fixer would be.
In include('include.html.twig', {}, false), I don't have an easy way to know which value is the third param. I could consider it after the second , ; but I have to take care of code like

• include(getTemplate(foo, bar), {}, false)
• include(foo, with_context = false)

Maybe you could be interested in a custom Node-based rule like
https://github.com/VincentLanglet/Twig-CS-Fixer/blob/3.0.0/src/Rules/Node/ValidConstantFunctionRule.php

Since it play with Node rather than Token, you can easily access to function arguments, and you'll be able to write a rule:

• Checking the with_context param is set to false
• Checking it use the named_param.

The only drawback is the fact that Node-based rule are not fixable (so far).

[alexander-schranz]

@VincentLanglet the basic idea what was in my mind case is if not on a specific argument instead a specific scalar types. If a given value to a twig extension call is a boolean value and it is not a named argument add the name of the argument. So more a BooleanNamedArgumentRule.

-{{ someExtension(someOtherExtension(true), {}, true) }}
+{{ someExtension(someOtherExtension(name_2: true), {}, name_2: true) }}

cases like following would stay untouched as then already a meaning was give to the value via a param name:

-{{ someExtension(someOtherExtension(otherArgument), {}, withContext) }}
+{{ someExtension(someOtherExtension(otherArgument), {}, withContext) }}

But I understand your decision if its to complex to keep it out of core.

The only drawback is the fact that Node-based rule are not fixable (so far).

Current case I want to avoid adding non fixable validators to the twig lint task, but yeah would be another option which I keep in my backmind.

[VincentLanglet]

@VincentLanglet the basic idea what was in my mind case is if not on a specific argument instead a specific scalar types. If a given value to a twig extension call is a boolean value and it is not a named argument add the name of the argument. So more a BooleanNamedArgumentRule.

I think this have to be on specific argument since I cannot guess the name of the argument.
You will need to end with some config like

['include' => [3 => 'with_context']]

and list all the wanted methods.

But I understand your decision if its to complex to keep it out of core.

I'll let the issue open to get feedback/suggestions, ... but yeah I think it might be better out of core.
I can still help you if you want to try working on the custom rule.

The only drawback is the fact that Node-based rule are not fixable (so far).

Current case I want to avoid adding non fixable validators to the twig lint task, but yeah would be another option which I keep in my backmind.

I understand. If you want to make the rule fixable it would require

• That the tool is able to have NodeBasedRule fixable (I don't think it's possible)
• To write the rule with TokenBasedRule

It may be possible with TokenBasedRule with the logic:

• If the token is not a FUNCTION_TYPE, returns
• If the name is not "include" returns
• Take the related ( if exists
• count the , before the related ) and skip any content when another (, { or [ came (until the closer).
• then check the parameter value
• if you don't find the named parameter, add it

[alexander-schranz]

Thank you for your detailed feedback. Let see if I'm the only one which searching for such rule.

Are there some instruction docs about how to create custom Node or Token based rules?

[VincentLanglet]

Are there some instruction docs about how to create custom Node or Token based rules?

ATM no, you have to get inspired by existing rules. But it's on my todo to improve the doc. Any feedback/PR is welcomed.

[VincentLanglet]

Thank you for your detailed feedback. Let see if I'm the only one which searching for such rule.

Are there some instruction docs about how to create custom Node or Token based rules?

I'm working on documentation (still wip) about writing such rules https://github.com/VincentLanglet/Twig-CS-Fixer/blob/main/docs/custom.md any feedback is welcome @alexander-schranz

[VincentLanglet]

BTW @alexander-schranz #302 introduce a new NAMED_ARGUMENT_SEPARATOR_TYPE token which could help to write your rule.

Anyway, I consider this rool won't be implemented in this library so I'll close.
I can still help to implement it (in your project) here or in a discussion