Javascript files being checked by phpcs

[gheydon]

My system information:

• Operating system type: Mac OS
• Operating system version: 10.13.5
• BLT version: 9.0.5

With the blt git hooks enable, blt is running phpcs over javascript files which:-

1. reports errors which are not exactly relevant.
2. Shouldn't run phpcs against javascript.

Part of the problem is that blt is passing a list of all files which are being committed, so this can include javascript files and other which are not php.

initially I had problems with this and a generated javascript file which came from a es6.js. I got around this problem by fixing the configuration for building the code in core https://www.drupal.org/project/drupal/issues/2975201 but it is not really a core issue.

[ba66e77]

PHPCS does also sniff/lint JS, so passing those files through PHPCS isn't a problem. There is another issue open where it's been noted that the linting isn't respecting the .eslintrc rules coming form core (#2845). Sounds like maybe that's the root of the issue you're seeing?

If you really want to disable js files checking, you could add the js extension to phpcs.xml as described in the docs for Extending BLT

[gheydon]

I have dug into this further and found what I think is the problem, which relates to consistency between doing a tests:phpcs:sniff:all and doing a tests:phpcs:sniff:files (which is what git hooks calls)

So generally with phpcs and Drupal we use 1 or both of the rulesets Drupal or DrupalPractice which have a list of extensions associated with each ruleset.

For Drupal it uses the extensions php,module,inc,install,test,profile,theme,css,info,txt,md,yml and DrupalPractice has php,module,inc,install,test,profile,theme,yml which from both of these lists there is no inclusion of javascript.

so when just calling phpcs with no arguments it will not by default check any javascript files. But when calling phpcs with a list of files or a file list which includes a javascript file it will check the file as the default exclusions do not have javascript listed as an exclusion, so it will check the javascript file.

This is all well and good, but maybe blt should default to adding javascript as an extension to check so there is consistency between checking all files and checking particular files (like what the git hooks are doing). This will mean you will get the same results from tests:phpcs:sniff:all and tests:phpcs:sniff:files

The secondary problem is that when using yarn to convert .es6.js to .js files, the resultant .js file doesn't pass the phpcs checks and in reality you do not wait phpcs to check these generated files. https://www.drupal.org/project/drupal/issues/2975201 resolves this issue by adding @codingStandardsIgnoreFile to the top of the generated file (plus some other changes to make the heading to use a standard docblock like we do in the .es6.js files.

[danepowell]

Sounds like a dupe of #3448