search

Automattic / VIP-Coding-Standards

PHP_CodeSniffer ruleset to enforce WordPress VIP coding standards.
https://wpvip.com/documentation/how-to-install-php-code-sniffer-for-wordpress-com-vip/
Other
236 stars 40 forks source link

Remove JS/CSS Sniffs #442

Open rebeccahum opened 5 years ago

rebeccahum commented 5 years ago

With JS and CSS tokenizers slated to be removed in 4.0 of PHPCS, we may have to remove our JS/CSS sniffs for compatibility: https://github.com/squizlabs/PHP_CodeSniffer/issues/2448

JS sniffs:

CSS Sniffs:

GaryJones commented 4 years ago

Side note: @gudmdharalds has confirmed that the vip-go-ci bot can integrate eslint, and we can therefore look at having it use something like https://github.com/mozilla/eslint-plugin-no-unsanitized and similar eslint plugins to identify some of what the sniffs here are looking out for.

https://github.com/squizlabs/PHP_CodeSniffer/issues/2448#issuecomment-525511341 says that PHPCS 4 won't be released until at least 2020, so we have plenty of time before this needs actioning (unless we feel that the eslint solution gives a better outcome / fewer false positives and want to use it sooner rather than later).

GaryJones commented 6 months ago

In PHPCS itself, https://github.com/PHPCSStandards/PHP_CodeSniffer/pull/276 was used to add @deprecated annotations to the relevant sniffs as a "soft deprecation" (defined as "deprecation via changelog mention and/or announcement only."). We can do this today.

Also https://github.com/PHPCSStandards/PHP_CodeSniffer/issues/188 covers how the sniffs will be hard deprecated ("a deprecation notice will be shown at runtime, but will not affect the exit code of PHPCS.") in the last PHPCS 3.x minor release. This is using the new native handling of deprecated sniffs for PHPCS 3.9 (which we'll need VIPCS to have as a minimum to make use of it).

We can do the same for VIPCS. It means we'll need to have a major VIPCS release that fully removes these sniffs, before we can support PHPCS 4 (which will have the CSS and JS tokenizers removed that our deprecated sniffs would otherwise try and use).

jrfnl commented 6 months ago

@GaryJones Glad you like what I've done to make deprecating sniffs more user-friendly 😀

Just to give you some idea, I'm hoping to release version 4.0 ~ end of May (though conferences & speaking may interfere with my planning).

Before 4.0 will be released, some utilities will be added to PHPCSUtils to make cross-version support for PHPCS 3.x/4.x (for scanning PHP files and running tests) easier and to make it more straight-forward to make sniffs compatible with the Tokenizer changes which are included in PHPCS 4.0.

I want those things to be available before releasing 4.0, which should hopefully also encourage more external standards to adopt PHPCSUtils.

I'm currently working on these new features in Utils and intend to include these utilities in PHPCSUtils 1.1.0 (along with some other new helper functionality).

GaryJones commented 6 months ago

@rebeccahum I see you included the UserExperience/AdminBarRemovalSniff.php sniff in the list, which uses CSS and PHP tokenizers. Do you think there is value in stripping out the CSS portion and just leaving the PHP checks?

Likewise, how about the Twig, UnderscoreJS, VueJS, Mustache sniffs? Should the whole sniff be soft deprecated, or is there still something to be gained by checking with just the PHP tokenizer?

Edit: added https://github.com/Automattic/VIP-Coding-Standards/compare/develop...feature/soft-deprecate-sniffs-for-phpcs-4.0 but will hold off on the PR until these questions have answers.

rebeccahum commented 6 months ago

Re: the admin bar, yeah, I think that would be fine since that is something we will strongly discourage.

Re: the other templating engines, I'm not sure how useful they are, so I'm fine with a soft deprecation.