Allow PHPCS to use DrupalPractice standard

[christopher-hopper]

My system information:

• Operating system: Ubuntu 16.04LTS
• BLT version: 8.9.0-rc1

When I run this command:

vendor/bin/blt validate:phpcs

I get the following output:

Iterating over fileset files.php.custom.modules...
Iterating over fileset files.php.custom.themes...
Iterating over fileset files.php.tests...

And I expected this to happen:

Iterating over fileset files.php.custom.modules...
FILE: ..._webapps/src/Plugin/Block/SingleItemBlock.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 30 | WARNING | #description values usually have to run through t()
    |         | for translation
----------------------------------------------------------------------
Iterating over fileset files.php.custom.themes...
Iterating over fileset files.php.tests...

I would like to suggest we make use (or allow use of) both the Drupal and DrupalPractice standards that are included in drupal/coder. Currently you hard-code a path the Drupal standard, but this is limiting.

Since drupal/coder version 8.2.11 [8.x-2.11], the package has supported use of Composer installer plugins, such as dealerdirect/phpcodesniffer-composer-installer, to auto-detect and configure available PHPCS standards found in package dependencies. This means that on any call to composer require, composer install or composer update, the path to new standards is automatically configured and the standards are available by name.

composer require --dev drupal/coder:^8.2.11
composer require --dev dealerdirect/phpcodesniffer-composer-installer
vendor/bin/phpcs -i
The installed coding standards are MySource, PEAR, PHPCS, PSR1, PSR2, Squiz, Zend, Drupal and DrupalPractice

From this point you can use the standards by name, and even specify multiple standards when running PHPCS commands.

vendor/bin/phpcs --standard=Drupal,DrupalPractice docroot/modules/custom/**/*

When I do this on my project, additional violation warnings, not detected by BLT, are given. Many are very helpful, like this from a custom Block class:

----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
 30 | WARNING | #description values usually have to run through t()
    |         | for translation
----------------------------------------------------------------------

[thom8]

This could be handled by templating a phpcs.xml file which would live in the root of the project.

For example -- https://github.com/thom8/drupalvm-live/blob/a6dcf39921c729f5fc0ecba8c0dff56a02754328/phpcs.xml

This has the added benefit of being able to reuse this configuration in an IDE like PHPStorm,
so you get immediate feedback on coding violations.

https://www.jetbrains.com/help/phpstorm/php-code-sniffer.html#appointCodingStyle

[christopher-hopper]

Agree @thom8

That was going to be next after getting the hardcoded standards path fixed. I would like to see BLT generate a phpcs.xml.dist. It could simply set the paths included/excluded as per project settings and ensure the base standards were used.

For this issue though, baby steps. Get the standards registered properly, so hardcoded paths can be removed from the calls to PHPCS.

[christopher-hopper]

As I'm not familiar with the BLT code, if someone can point me in the right direction, I'd be happy to create an initial PR for comment.

[thom8]

@christopher-hopper this is the Class for the validate:phpcs namespace.

https://github.com/acquia/blt/blob/8.x/src/Robo/Commands/Validate/PhpcsCommand.php

Perhaps it could skip any configuration if a phpcs.xml is found in the project root as phpcs automatically uses this for it's default config.