search

WPTT / WPThemeReview

PHP_CodeSniffer rules (sniffs) to enforce WordPress theme review coding conventions
MIT License
207 stars 38 forks source link

Disallow long array syntax while writing sniffs, and enforce short array syntax #238

Closed dingo-d closed 4 years ago

dingo-d commented 4 years ago

This is done to prevent failing of the TravisCI due to changes made in the WPCS 2.2.0 where short array syntax was disallowed.

dingo-d commented 4 years ago

I do prefer short syntax array.

If we enforce them that means that all the sniffs will have to have the array syntax changed, but that can be autofixed, so it shouldn't be problematic I guess. Should I make a new PR and close this one then?

jrfnl commented 4 years ago

Either that or update this PR to do the reverse (and change the description and title).

If needs be, you can look at this PR for inspiration: https://github.com/Yoast/yoastcs/pull/159/files

jrfnl commented 4 years ago

FYI: I haven't merged any of the others yet as this one is basically needed to get the builds to pass again. So once this PR is updated (or closed and a new one created), I'll merge it and then merge the others.

dingo-d commented 4 years ago

I excluded the short array syntax using this

<rule ref="Generic.Arrays">
    <exclude name="Generic.Arrays.DisallowShortArraySyntax"/>
</rule>

instead of loading the entire WordPress ruleset and just excluding this one. This does the same thing. I worry that if I inlcuded the entire WordPress ruleset, I'd have to exclude tons of other sniffs as well.

dingo-d commented 4 years ago

Yeah, autoprefixer actually failed so I did it all by hand :grimacing:

jrfnl commented 4 years ago

Yeah, autoprefixer actually failed so I did it all by hand grimacing

Huh ? Hmm.... I'll have a look as that's something we should be concerned about.

I did run a quick test on the develop commit before the merge and the auto-fixer worked perfectly in isolation (with the ruleset change as has now been merged):

phpcbf --sniffs=generic.arrays.disallowlongarraysyntax
jrfnl commented 4 years ago

And just ran a test using just phpcbf with the ruleset changes as now merged and I don't get any fixer-conflicts.

Could it be you tried with the ruleset which included <rule ref="Generic.Arrays"> ? That could be the reason as - not surprisingly - that just means that the WPCS and the upstream array indentation sniffs work differently and conflict.

That conflict is now resolved though with the updated ruleset.

dingo-d commented 4 years ago

Yeah, I ran it with the old change. So that must be it :+1: