[RFC] Rule whish list

[lyrixx]

Hello,

I have a wish list 🎁 :) Feel free to close it if it's not in your scope!

Remove useless quote in hashes

Do you think it would be doable to apply this diff

- {{ include('hello.html.twig', { 'publications': alreadyPublished }) }}
+ {{ include('hello.html.twig', { publications: alreadyPublished }) }}

Use the same quote everywhere

- {{ include("hello.html.twig" }}
+ {{ include('hello.html.twig' }}

Note: It would depends on the configuration

[VincentLanglet]

Hi, thanks for the feedback.

A way to know how hard the rule would be to implement is to look at the way the Tokenizer tokenize your strings.

{{ include("hello.html.twig") }}

is tokenized

• VAR_START_TYPE {{
• WHITESPACE_TYPE
• NAME_TYPE include
• PUNCTUATION_TYPE (
• STRING_TYPE "hello.html.twig"
• PUNCTUATION_TYPE )
• WHITESPACE_TYPE
• VAR_END_TYPE }}

It should be possible to introduce a rule which say that every STRING_TYPE starting/ending with " should be reformatted.

Something like

{{ include("foo #{p.first} bar") }}

is tokenized

• VAR_START_TYPE {{
• WHITESPACE_TYPE
• NAME_TYPE include
• PUNCTUATION_TYPE (
• DQ_STRING_START_TYPE "
• STRING_TYPE foo
• INTERPOLATION_START_TYPE #{
• ...
• INTERPOLATION_END_TYPE }
• STRING_TYPE bar
• DQ_STRING_END_TYPE "
• PUNCTUATION_TYPE )
• WHITESPACE_TYPE
• VAR_END_TYPE }}

So we shouldn't have issue with interpolation.

[VincentLanglet]

For

{{ include('hello.html.twig', { 'publications': alreadyPublished }) }}

it's (without the whitespace type)

• VAR_START_TYPE {{
• NAME_TYPE include
• PUNCTUATION_TYPE (
• STRING_TYPE 'hello.html.twig'
• PUNCTUATION_TYPE ,
• STRING_TYPE 'publications'
• PUNCTUATION :
• NAME_TYPE
• PUNCTUATION_TYPE )
• VAR_END_TYPE }}

We could look at STRING_TYPE which is just before a PUNCTUATION_TYPE :.
This shouldn't conflict with

foo ? 'bar' :  'baz'

because in such syntax I tokenize : as an OPERATOR_TYPE (and not a punctuation anymore).

NB

{{ include('hello.html.twig', { publications: alreadyPublished }) }}

would be tokenized publications as a NAME_TYPE (not a string anymore).

[VincentLanglet]

So I assume that both of these rule are possible and not so hard.
Would you be interested working on @lyrixx ?

[lyrixx]

I would like to contribute to this project, But I fear I'll lake time! We'll see :)

[smnandre]

How would you handle keys that do need quotes ? Keep the quotes only for them ? (seems legit)

{{ include('foo.twig', { '123': 123 }) }}

{{ include('foo.twig', { 'data-foo': 123 }) }}

[smnandre]

- {{ include("hello.html.twig") }}
+ {{ include('hello.html.twig') }}

Would love to have this rule! 👍

[VincentLanglet]

How would you handle keys that do need quotes ? Keep the quotes only for them ? (seems legit)

Yeah it seems legit to keep the quote.

{{ include('foo.twig', { 'data-foo': 123 }) }}

I think we should rely on Tokenizer::REGEX_NAME = '/[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*/A';

{{ include('foo.twig', { '123': 123 }) }}

Why the quote is needed here ?
I assume twig works like php, therefore ['123' => 123]
is simplified to [123 => 123] anymore

On the contrary

{{ include('foo.twig', { 12e+2: 123 }) }}`

is a valid key but is different from

{{ include('foo.twig', { '12e+2': 123 }) }}`

So i think only number with the format [0-9]+ can be simplified, no ?

[smnandre]

Well obviously if i forget to type the most important char.....

{{ include('foo.twig', { '0123': 123 }) }}

So i think you're right: using the Tokenizer::REGEX_NAME should be a perfect check

[VincentLanglet]

Well obviously if i forget to type the most important char.....

{{ include('foo.twig', { '0123': 123 }) }}

So i think you're right: using the Tokenizer::REGEX_NAME should be a perfect check

Oh, I see.

So we could do Tokenizer::REGEX_NAME and [1-9][0-9]*

[VincentLanglet]

Do you want to beta test #212 @lyrixx @smnandre ?

[smnandre]

On it !

[VincentLanglet]

On it !

Any feedback @smnandre ? I was thinking about merging/releasing the feature in the next days.

[smnandre]

Oh sorry!

It's perfect and really usefull to normalize code styles!

Tested on a personal project + ux website: all works as expected.

👏

[VincentLanglet]

Thanks for the feedback, I released.

I will try to work on

{{ include('hello.html.twig', { 'publications': alreadyPublished }) }}

unless you already started @lyrixx ?

[lyrixx]

No, Sorry, I didn't find time to work on it

[VincentLanglet]

The HashQuoteRule is ready for beta-test
#217

By default, it require to use quote for hash (opinionated choice, it gives consistency and avoid mistake).
A key like 0123 is reported as an error non fixable because we're not sure if '0123' or '123' is expected.

An option exist to remove quote when possible, it will remove quote

• For name which doesn't require ' ('foo' is valid 'data-foo' is not).
• For integer which does not start by 0. (I use the check `$value === (string) (int) $value))

WDYT ? Any edge case forgotten @lyrixx @smnandre ?

[lyrixx]

LGTM 👍🏼

[VincentLanglet]

LGTM 👍🏼

Thanks.

@smnandre did you find any false positive ?

[smnandre]

I only tested quickly on a personal project to be honest 🤷 ...

[VincentLanglet]

No problem. I merged the PR to test more easily.
I'll try it on some projects.

I'll also maybe change the default value useQuote from true to false before releasing the feature...
I started a poll to know which syntax is preferred https://twitter.com/MisterDeviling/status/1786535766790640069

[smnandre]

It's not impossible there is no common ground here 🤷

[VincentLanglet]

Released in 2.9.0