Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CI/QA: add code style check #434

Merged
merged 10 commits into from
Nov 17, 2020
Merged

CI/QA: add code style check #434

merged 10 commits into from
Nov 17, 2020

Conversation

jrfnl
Copy link
Member

@jrfnl jrfnl commented Nov 11, 2020

QA: add code style check for the code in this repo

This PR:

  • Adds dev requirements for the WP Coding Standards, PHPCompatibility and the DealerDirect Composer PHPCS plugin.
  • Adds a custom PHPCS ruleset which uses the WordPress-Extra ruleset to check the code style and code quality of code in this repo and the PHPCompatibility standard to test for PHP cross-version compatibility.
  • Adds convenience scripts to the composer.json file to check the code of the repo.
  • Allows for individual developers to overload the .phpcs.xml.dist file by ignoring the typical overload files.
  • Adds the CS check to the Travis script in a separate stage.
    The CS check only needs to be run on one build, preferably against a high PHP version.
    To that end, I've set up stages and added the CS check to a separate stage.
    Ref: https://docs.travis-ci.com/user/build-stages/
    Also note that the PHPCS dependencies have a minimum supported PHP version of PHP 5.4, while this repo still supports PHP 5.2.
    To prevent a failing composer install on PHP < 5.4, the PHPCS related dependencies are removed for the test stage.

Note:
The WordPress Coding Standards were chosen as the basis for the code style checks as most contributors to this repo originate from the WordPress community and will be familiar with this code style.

Main differences from the WordPress Coding Standards based on a discussion last year with Ryan + an analysis of the code styles used in the code base as-it-was:

  • No "nacin-spacing", i.e. no whitespace on the inside of parentheses.
  • No Yoda conditions. The WPCS Yoda conditions check does not prevent assignments in conditions anyway and it decreases readbility. Instead, a "no assignments in conditions" rule which is included in WordPress-Extra is enforced.

Also note that the WordPress-Docs checks are not enabled. This may be added at a later point in time. Enabling the scan at this time would, however, yield over 900 violations which would still need to be fixed first.

PHPCS ruleset: implementation examples can use different PHP minimum

Don't enforce the PHP 5.2 minimum supported PHP version on the code samples in the examples folder.
It's perfectly fine for some of those code samples to show implementation examples targetted at a higher minimum PHP version.

PHPCS ruleset: enforce "space-poor" code style

See the inline documentation in the ruleset for full details.

PHPCS ruleset: forbid Yoda conditions

Instead forbid assignments in conditions which is what Yoda is supposed to be about anyway.

The only exception made is for while() loops in which assignments in condition can actually be a valid choice.

As the Generic.ControlStructures.DisallowYodaConditions sniff used for enforcing this is only available since PHPCS 3.5.0, the PHPCS package is also added to the composer.json file.

Normally, this wouldn't be needed and shouldn't be done as such requirements might conflict with the requirements from the other PHPCS related dependencies, but their minimum supported PHPCS version is lower than PHPCS 3.5.0, so we need to enforce that PHPCS 3.5.0 or newer is installed.

Currently, WPCS requires a minimum of PHPCS 3.3.1 and PHPCompatibility a minimum of PHPCS 2.3 or 3.0.2.

Once the minimum supported PHPCS version for (one of) these external standards used goes up to 3.5.0, the requirement in the Requests' composer.json file should be removed to prevent future conflicts.

PHPCS ruleset: exclude various other rules

WordPress.Files.FileName

This repo uses PSR-0 for the class name determination, which conflicts with the WPCS rules, so ignoring the WPCS sniff.

There is currently no sniff available to enforce PSR-0.

Squiz.ControlStructures.ControlSignature.SpaceAfterCloseBrace

This contrast with the code style used in the code base as-is. The opposite can in the future be enforced once PHPCSExtra 1.0 has been tagged (which will be included in WPCS 3.0.0).

Ignore various WP specific sniffs.

As Requests is a stand-alone library, sniffs referring to WordPress native functions or doing things "the WordPress way" are irrelevant.

PHPCS ruleset: add minimal sniff specific configuration

Certain sniffs will only work when given some more information (PrefixAllGlobals), others will be less noisy with some minimal configuration.

This adds such configuration for three sniffs coming from WPCS.

WordPress.PHP.NoSilencedErrors

Some PHP function will throw warnings in certain situations which cannot be avoided, no matter how much error checking is done ahead of the function call.
This whitelists three such functions used in the Requests library to be allowed to use error silencing.

WordPress.NamingConventions.PrefixAllGlobals

The PrefixAllGlobals sniff will only work when it knows which prefix is desired.

In addition, code which is not part of the distributed package as used by consumers of this package - i.e. tests, build script etc - does not have to comply with the "prefix all globals" rules.

WordPress.Arrays.MultipleStatementAlignment

The standard configuration for arrow alignment in multi-line arrays can be quite fiddly.
This overloads the WordPress defaults to use a slightly more sane configuration.

PHPCS ruleset: add very selective file based exceptions

Add some very selective file name based exceptions to the rules for various reasons as documented in the ruleset with each individual exception.

PHPCS: whitelist select compatibility issues

... as they are surrounded by the correct safeguards for cross-version compatibility.

PHPCS: whitelist select QA issues

CS: minor additional fixes

Few more tiny fixes for issues found while cleaning up the ruleset.

This PR:
* Adds `dev` requirements for the WP Coding Standards, PHPCompatibility and the DealerDirect Composer PHPCS plugin.
* Adds a custom PHPCS ruleset which uses the `WordPress-Extra` ruleset to check the code style and code quality of code in this repo and the `PHPCompatibility` standard to test for PHP cross-version compatibility.
* Adds convenience scripts to the `composer.json` file to check the code of the repo.
* Allows for individual developers to overload the `.phpcs.xml.dist` file by ignoring the typical overload files.
* Adds the CS check to the Travis script in a separate stage.
    The CS check only needs to be run on one build, preferably against a high PHP version.
    To that end, I've set up stages and added the CS check to a separate `stage`.
    Ref: https://docs.travis-ci.com/user/build-stages/
    Also note that the PHPCS dependencies have a minimum supported PHP version of PHP 5.4, while this repo still supports PHP 5.2.
    To prevent a failing `composer install` on PHP < 5.4, the PHPCS related dependencies are removed for the `test` stage.

Note:
The WordPress Coding Standards were chosen as the basis for the code style checks as most contributors to this repo originate from the WordPress community and will be familiar with this code style.

Main differences from the WordPress Coding Standards based on a discussion last year with Ryan + an analysis of the code styles used in the code base as-it-was:
* No "nacin-spacing", i.e. no whitespace on the inside of parentheses.
* No Yoda conditions. The WPCS Yoda conditions check does not prevent assignments in conditions anyway and it decreases readbility. Instead, a "no assignments in conditions" rule which is included in `WordPress-Extra` is enforced.

Also note that the `WordPress-Docs` checks are not enabled. This may be added at a later point in time. Enabling the scan at this time would, however, yield over 900 violations which would still need to be fixed first.
Don't enforce the PHP 5.2 minimum supported PHP version on the code samples in the `examples` folder.
It's perfectly fine for some of those code samples to show implementation examples targetted at a higher minimum PHP version.
See the inline documentation in the ruleset for full details.
Instead forbid _assignments in conditions_ which is what Yoda is supposed to be about anyway.

The only exception made is for `while()` loops in which assignments in condition can actually be a valid choice.

As the `Generic.ControlStructures.DisallowYodaConditions` sniff used for enforcing this is only available since PHPCS 3.5.0, the PHPCS package is also added to the `composer.json` file.

Normally, this wouldn't be needed and shouldn't be done as such requirements might conflict with the requirements from the other PHPCS related dependencies, but their minimum supported PHPCS version is lower than PHPCS 3.5.0, so we need to enforce that PHPCS 3.5.0 or newer is installed.

Currently, WPCS requires a minimum of PHPCS 3.3.1 and PHPCompatibility a minimum of PHPCS 2.3 or 3.0.2.

Once the minimum supported PHPCS version for (one of) these external standards used goes up to `3.5.0`, the requirement in the Requests' `composer.json` file should be removed to prevent future conflicts.
`WordPress.Files.FileName`

This repo uses PSR-0 for the class name determination, which conflicts with the WPCS rules, so ignoring the WPCS sniff.

There is currently no sniff available to enforce PSR-0.

`Squiz.ControlStructures.ControlSignature.SpaceAfterCloseBrace`

This contrast with the code style used in the code base as-is. The opposite can in the future be enforced once PHPCSExtra 1.0 has been tagged (which will be included in WPCS 3.0.0).

Ignore various WP specific sniffs.

As Requests is a stand-alone library, sniffs referring to WordPress native functions or doing things "the WordPress way" are irrelevant.
Certain sniffs will only work when given some more information (`PrefixAllGlobals`), others will be less noisy with some minimal configuration.

This adds such configuration for three sniffs coming from WPCS.

`WordPress.PHP.NoSilencedErrors`

Some PHP function will throw warnings in certain situations which cannot be avoided, no matter how much error checking is done ahead of the function call.
This whitelists three such functions used in the `Requests` library to be allowed to use error silencing.

`WordPress.NamingConventions.PrefixAllGlobals`

The `PrefixAllGlobals` sniff will only work when it knows which prefix is desired.

In addition, code which is not part of the distributed package as used by consumers of this package - i.e. tests, build script etc - does not have to comply with the "prefix all globals" rules.

`WordPress.Arrays.MultipleStatementAlignment`

The standard configuration for arrow alignment in multi-line arrays can be quite fiddly.
This overloads the WordPress defaults to use a slightly more sane configuration.
Add some very selective file name based exceptions to the rules for various reasons as documented in the ruleset with each individual exception.
... as they are surrounded by the correct safeguards for cross-version compatibility.
Few more tiny fixes for issues found while cleaning up the ruleset.
@jrfnl jrfnl added this to the 1.7.1 milestone Nov 11, 2020
@schlessera schlessera merged commit b304eef into master Nov 17, 2020
@schlessera schlessera deleted the feature/add-cs-check branch November 17, 2020 15:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants