Improve PHP Coding Standards checks tuned to projects

Motivation

As a new team member coming to work on the SDP CMS I want to know I can rely on the phpcs and phpcbf tools to find and fix simple coding standards problems, coding practices problems, and PHP incompatibilities.

Code sniffer checks should be limited to the custom code I write, and work consistently, whether in my IDE, my Docker container, CircleCI test runs, or my local machine.

Problem

I am encountering inconsistent behaviour when running the coding standards tools - phpcs and phpcbf. Certain sniffs are not picking up obvious problems during test jobs. I see violations in Pull Requests during code review, even though the tests all pass.

The phpcs configuration file is currently very long and may not have been brought up-to-date in a while. I'd like to proposed some changes, to benefit all SDP projects.

Suggested Changes

Change the config file name

Use phpcs.xml.dist instead.

The configuration file follows same naming conventions as phpunit.xml.dist. A distributed config phpcs.xml.dist can be overridden by individuals, or project, if they choose, by copying to phpcs.xml. The recommendation is to commit phpcs.xml.dist, and ignore phpcs.xml, to allow individuals and teams some local control over behaviour without impacting the SDP distribution.

Set default included paths in the config file

By default, with no paths specified at the command line, the phpcs tool should check the Drupal custom code paths for a project. This has been currently coded into ahoy using environment variables. Those however do not work outside of ahoy, and phpcs provides a supported way to do the same thing.

This can be most simply achieved by setting the default included paths in the phpcs.xml.dist config file for the project like this:

    <!-- Include these files -->
    <file>docroot/sites/default</file>
    <file>docroot/modules/custom</file>
    <file>docroot/themes/custom</file>
    <!--file>docroot/profiles/custom</file-->
    <file>tests</file>

This allows removal of complex path exclusions rules. Complex <exclude-pattern> matches can have unintended consequences, and are hard to override or turn-off. These <file> inclusions only apply when no other path is provided at the command-line.

To manually override the defaults, specify path arguments at the command line. This will not impact ENV vars being used in ahoy lint currently, however it may make the ENV vars unnecessary.

Simplify and reduce the exclusions in the config file

For project development, we should limit the use of exclusions from coding standards checks. Where we disagree with the standards (or a sniff) it would be best to take that up with the community maintainers, to improve them for everyone, rather than ignoring sniffs, rulesets, or file paths altogether.

This simplifies the config, making it easy to understand, and less prone to unintended consequences.

dpc-sdp / dev-tools