Create a WP_CLI_CS standard for PHP_CodeSniffer

[jrfnl]

A PHPCS standard is in principle the same as a custom ruleset with these differences:

Custom ruleset	External PHPCS standard
File: [.]phpcs.xml[.dist]	File: ruleset.xml
Can be included in other rulesets using the path to the file (which can get complicated)	Can be included in other rulesets by name
Can only use existing sniffs	Can add new sniffs native to the standard (not currently used)

Aside from that, one has to keep in mind that all paths used in a ruleset are interpreted relative to the ruleset.
As an external standard can be installed both globally as well as local to a project, it is better not to have any specifics involving file paths in an external standard, but to use a custom ruleset, which generally lives in the project root, for this instead.

So, this PR introduces a WPCliCS standard to the wp-cli-tests project which is a dependency for all WP-CLI projects, making this standard easily available to them all.

As the dealerdirect/phpcodesniffer-composer-installer plugin is a requirement for this project (and therefore all WP-CLI projects), this new WPCliCS standard will automatically be recognized and installed via the plugin.
No extra actions needed 😎

To see it in action, just run a composer install/composer update and then run vendor/bin/phpcs -i and WPCliCS should now be listed as one of the installed standards.

About the ruleset itself, there are a couple of things to take note of:

• Based on the information received in Make the PHPCS setup more reusable #34 - WP won't always be (fully) loaded when code from the WP-CLI commands is run, IMO it will be better to include the PHPCompatibility standard (stricter) than the PHPCompatibilityWP standard, as the polyfills which the PHPCompatibilityWP standard accounts for will not necessarily always be available.
  Includes adjusting the composer.json requirement to match.
• As not all project have the same minimum PHP version, the testVersion config setting is not set in the standard, but should be set in the custom ruleset of each project.
  While it is possible since PHP 3.3.0 to overrule a config setting from the command-line, overruling it from a custom ruleset is still not possible, so it is better to set it in the custom ruleset in the first place.
• While the PHPCompatibility sniffs should be limited to .php files, the other sniffs shouldn't necessarily be limited to this, so instead of setting <arg name="extensions" value="php"/>, I'm using an <include-pattern> for the PHPCompatibility sniffs only.
  • Side-note: By default, PHPCS will scan files with the following file extensions: .php, .inc (interpreted as PHP), .js and .css. As most WP-CLI projects don't include .js or .css files (or .inc files for that matter), there is rarely a need to set limit the extensions from within a project ruleset.
  • Similarly, any directories which don't contain any of the above mentioned file types, don't need to be excluded via a <exclude-pattern> either, as PHPCS won't find any files to scan anyway.
  • Also, take note of the syntax used in the <include-pattern>. Both <include-pattern>s as well as <exclude-pattern>s are basically PHP PCRE regular expressions, with one caveat: any * in the pattern will automatically be replaced by .*.
    This also means that characters which have special meaning in a PCRE regex, like the . have to be escaped properly!
• In the base ruleset in the wp-cli-tests root (and in wp-cli), the WordPress-Core ruleset was included.
  While this is all well and good, the WordPress-Extra ruleset contains many additional sniffs for code quality as well as some sniffs for modern PHP code style, which is not yet covered in the Core code style handbook.
  As the WP-CLI projects use PHP 5.4 as a minimum, using the WordPress-Extra ruleset is IMO the better choice. Even though there are some exceptions to the rules which need to be made, the majority of the additional sniffs contained within the Extra ruleset will be valuable to scan against.
• A standard should be widely applicable and should not contain <exclude> exceptions which are solely for one project.
  This means that the excludes which were used so far have largely been removed.
  Regarding the remaining (and new) excludes:
  • The exclusion of the array_column detection is valid as the polyfill is shipped with wp-cli and therefore available to all WP-CLI projects.
  • The Generic.PHP.Syntax sniff basically does a php -l on a file. As linting the files is already done in a separate Travis job for all projects, this sniff is not needed.
  • The WordPress.File.FileName sniff conflicts with PSR-4, which is used for the autoloading of the WP-CLI files, so this is a legitimate exclusion.
  • The WordPress.PHP.DiscouragedPHPFunctions and WordPress.WP.AlternativeFunctions contain some recommendations which are not all that applicable for project running as cli, so the most typically problematic ones have been excluded.
    The same applies to the Generic.PHP.BacktickOperator sniff.
    Further exclusions can be made, if and when, needed from individual project-based rulesets.
  • Along the same lines, the WordPress.PHP.DevelopmentFunctions discourages the use of typical PHP dev related functions. As WP-CLI is a developers tool, I believe it is legitimate to use these functions in this context.
  • Lastly, variable output send to the command-line should probably use some form of escaping, but the output escaping functions the WordPress.Security.EscapeOutput sniff checks for are _HTML_output escaping functions, which are not applicable in this context.
    In the future, this standard could be extended with a custom sniff for output escaping for the command line.
    Similarly, a custom sniff to verify that arbitrary arguments passed to shell commands are escaped properly using escapeshellcmd() or escapeshellarg() should also be considered.
• I've added configuration for one particular sniff: WordPress.NamingConventions.PrefixAllGlobals.
  Even though the WP environment will not always be (fully) loaded, it oftentimes will be. In that case, naming conflicts between plugins and WP-CLI projects for names in the global namespace can occur.
  To that end, all WP-CLI code should be either namespaced or prefixed.
  As the sniff will just be silent if there is no configuration present, I've added the most generic configuration possible to still demand prefixes.
  Individual projects can overrule and/or - as of PHPCS 3.4.0 - add to this configuration from their own project ruleset.
• While I've added the PHPCS schema tags to the ruleset, the schema will not be able to validate correctly, unless PHPCS 3.3.2+ is used.
  PHPCS 3.3.0 introduced a new syntax for array elements to the rulesets, but the schema wasn't updated for this new syntax until PHPCS 3.3.2.

Fixes #34

Other Changes

bin/run-phpcs-tests: list config files in PHPCS precedence order

PHPCS 3.1.0 introduced the ability to use dot prefixed configuration files.
In PHPCS 3.1.1 the loading order of these got normalized.

So, in effect PHP will search for configuration files in this order:

1. .phpcs.xml
2. phpcs.xml
3. .phpcs.xml.dist
4. phpcs.xml.dist

This is just a minor tweak so the same order of preference is used in this script to list the possible config files.

Refs;

• https://github.com/squizlabs/PHP_CodeSniffer/releases/3.1.0
• https://github.com/squizlabs/PHP_CodeSniffer/releases/3.1.1

Composer: temporarily require PHPCS

Strictly speaking PHPCS is not a requirement of this project, but a requirement of its dependencies WPCS + PHPCompatibility.

However, WPCS currently supports PHPCS ^2.9 || ^3.1 and PHPCompatibility still supports PHPCS all the way back to 2.3.

For the WP-CLI projects, it is preferred that these use a more recent PHPCS version.

Aside from the fact that this means that a lot of sniffs will be more accurate, this will also allow for using, for instance:

• The basepath setting for cleaner result reports. (PHPCS 3.0+)
• The parallel setting for multi-threaded scanning. (PHPCS 3.0+)
• The new, powerful PHPCS selective inline annotations. (PHPCS 3.2+)
• PHPCS on PHP 7.3. (PHPCS 3.3.1+)

Once WPCS 2.0 comes out, which is expected to have a PHPCS ^3.3.1 minimum requirement, the requirement in the composer file in this package can be removed.

Refs:

• https://github.com/squizlabs/PHP_CodeSniffer/releases

.gitignore/.distignore: ignore phpcs/phpunit config files

• .distignore: no need to distribute the phpcs/phpunit config files.
• .gitignore: Allow devs to overwrite config files for PHPUnit and PHPCS
  • The .dist files should be committed. However, for their personal use, devs can overrule those files with versions without .dist.
    Those personal versions should never be committed.

As a side-note: for PHPCS, having a personal version while still using the original .dist file is made very easy, as you can just import the .dist file as a starting point, like so:

<?xml version="1.0"?>
<ruleset name="WP-CLI">
    <rule ref="./phpcs.xml.dist"/>
</ruleset>

README/USING: add instructions on how projects can use the base ruleset

[jrfnl]

FYI: the build won't pass (yet) as - per discussion in #34 - the custom ruleset for this repo hasn't been adjusted yet to use the WPCliCS standard.

[jrfnl]

PR has been updated based on feedback.

[jrfnl]

@schlessera I saw your approval: to get this merged with a passing build, I'd have to either adjust or remove the custom ruleset for this repo. The ruleset as is, isn't testing anything anyway.

Now you've seen the direction this is going in, what would be your preference ?

[jrfnl]

@schlessera ☝️ ?

[jrfnl]

I've force pushed an updated version of this branch (as discussed on Slack).

The significant changes are:

• Require WPCS ^2.1.
• Removed commit 0f74042 as it is no longer needed now WPCS is at ^2.1.
• Added a property for the WordPress.PHP.NoSilencedErrors sniff to the ruleset.
• Updated/improved inline documentation, including adding links to the WPCS documentation where relevant.
• Minor tweaks to the documentation in the USING document.
• Squashed some commits

[jrfnl]

OK, so to get this PR passing, the ruleset in the root of this repo will need to be adjusted and the code of this repo will need to pass the CS check.

Currently there are 105 issues still remaining which need to be fixing, whitelisted or exclusion via configuration.

This is the detailed list:
20190417-phpcs-result-wp-cli-tests.txt

[schlessera]

I went through all the issues and made multiple pull requests.

Here is the list of remaining issues I cannot simply fix:
remaining-issues.txt

[jrfnl]

Thank you for that!

Here is the list of remaining issues I cannot simply fix:

I have made the necessary updates to the repo ruleset and added whitelist comments where relevant (in my local version of this PR branch). Once your PRs have been merged, I will rebase this PR and add the additional commits with which the build should now pass (tested locally & confirmed).