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

Update for PHPCS 3.x and WPCS 0.14.1 #145

Merged
merged 448 commits into from May 27, 2018

Conversation

Projects
None yet
@jrfnl
Contributor

jrfnl commented May 24, 2018

TL;DR

This PR does a back-merge of WPCS to WPTRT based on WPCS master/0.14.1.

It then applies a number of commits to make the Theme sniffs consistent with developments in WPCS upstream.

Lastly, it updates the sniffs to be compatible with PHPCS 3.x.

Relevant commits:

Merge branch 'master' into WPTRT/feature/update-forPHPCS-3.x-WPCS-0.14

Note: For now, I've used the master branch of WPCS as there are some issues which still need to be addressed in WPCS develop. Once those have been sorted, develop should be merged in as it contains significant bug fixes,

Fixes #144

DeprecatedWPConstants: Remove sniff in favour of WPCS sniff

WPCS 0.14.0 introduced a WP.DiscouragedConstants sniff which covers all the constants which were covered by the DeprecatedWPConstants sniff and does so better than the original sniff.

Removed the DeprecatedWPConstants sniff in favour of the new upstream sniff and changing the error level from warning to error from the ruleset maintains the existing behaviour.

Ref: WordPress-Coding-Standards/WordPress-Coding-Standards#1089

Closes #140
Fixes #110

Leverage the WordPress_Sniff class for all sniffs

Most WPCS sniffs extend the WordPress_Sniff class or one of the abstract WP sniffs. However there were some which so far didn't.

This PR changes that whenever possible.

The affected sniffs do not necessarily need to use the WordPress_Sniff class as they don't use any of the utility methods or properties contained therein, however it will make future development slightly more intuitive as (nearly) all sniffs now follow the same pattern.

This same change has been made upstream in WordPress-Coding-Standards/WordPress-Coding-Standards#967

Documentation: Fix two outdated & incorrect code samples

eval can not be sniffed for as a function as it is a language construct, not a function, so this was misleading example code.

This same change has been made upstream in WordPress-Coding-Standards/WordPress-Coding-Standards#812

PHPCS 3.x compat: Ruleset: Make sure the complete sniff is included, not just an errorcode

In PHPCS 2.x when a sniff was referenced by errorcode, this automatically included the whole sniff.
As of PHPCS 3.x it is possible to selectively include just and only a specific errorcode.

This allows for cross-version compat and ensures that if/when new errorcodes are added to the sniffs, they will be included as well.

PHPCS 3.x compat: Namespace all sniffs which extend from one of the WPCS base sniffs

PHPCS 3.x compat: Fix all references to upstream PHPCS classes

  • Add use statements for all PHPCS class references.
  • Where possible use the PHPCS 3.x class name as a use alias to make the future change of dropping 2.x support easier.

PHPCS 3.x compat: Add autoload directive for the alias file

The <autoload> ruleset directive was added in PHPCS 3.0.1 in response to issue squizlabs/PHP_CodeSniffer#1469

PHPCS 3.x compat: Namespace all unit tests

PHPCS 3.x compat: Remove test conditionals for PHP 5.2

Minor code layout & code style fixes

Clean up: Remove // end comments

The upstream CommentedOutCode sniff has been updated and become more accurate and now throws a number of warnings for // End ... comments.

As the rule regarding end comments has been removed from the handbook anyway, we may as well remove them all and get rid of the warnings.

This same change has been made upstream in WordPress-Coding-Standards/WordPress-Coding-Standards#1362


Fixes #139

/cc @dingo-d

GaryJones and others added some commits Jul 22, 2017

Merge pull request #1045 from WordPress-Coding-Standards/WPCS-0.13.0/…
…feature/phpcs-3.x-prep-getgroups-in-abstracts

PHPCS 3.x prep: work round a function which will no longer exist
PHPCS 3.x compat: Namespace all sniffs which extend from one of the W…
…PCS base sniffs

Includes adjustment of FQCN references in `@see` and `@uses` documentation.
PHPCS 3.x compat: Fix all references to upstream PHPCS classes
* Add use statements for all PHPCS class references.
* Where possible use the PHPCS 3.x class name as a use alias to make the future change of dropping 2.x support easier.
PHPCS 3.x compat: Add helper class & implement use
A number of PHPCS 2.x classes have been split out over several classes in PHPCS 3.x.
Aliasing the new classname to the old would therefore not be a good idea.

This helper class acts as a compatibility layer for these cases.
The helper class only addresses the most common functionality needed and is by no means exhaustive.
The helper class contains one function which is currently not (yet) used `get_version()`. This method was added in anticipation of continuing support for both PHPCS 3.x as well as 2.x for another year. PHPCS 3.x is likely to bring functionality which will not be available in PHPCS 2.x, so being able to retrieve the PHPCS version will, at some point, probably be helpful.

N.B. The class name for this class has to be in CamelCaps (without underscore) as otherwise the PHPCS 2.x autoloader can not find it.
PHPCS 3.x compat: Alias PHPCS 3.x classes to their PHPCS 2.x counterp…
…arts

Notes:
* The file is automatically loaded by PHPCS using the new `<autoload>` option. This option, when encountered in the ruleset, is ignored by previous PHPCS versions, so can be used safely and will automatically make sure the file is only loaded for PHPCS 3.x.
* The file is loaded after the PHPCS native autoloader has started, but before any sniffs are loaded making for a clean way to add the aliases and prevent fatal 'class not found' errors.
* Each class alias statement has to be wrapped in a `class_exists` condition to prevent fatal `class already declared` errors when this ruleset is used in combination with other external ruleset(s) which use the same/similar methodology to create PHPCS cross-version compatibility.

The `<autoload>` ruleset directive was added in PHPCS 3.0.1 in response to issue squizlabs/PHP_CodeSniffer#1469
PHPCS 3.x compat: Namespace all unit tests
While the `PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest` class is called `AbstractSniffUnitTest` in PHPCS 2.x and we'll actually be using our own version instead, we can alias it to the PHPCS 3.x name via a bootstrap file loaded by PHPUnit.
This allows us to already use the `use` statement as you would expect for PHPCS 3.x which means that these files won't have to be changed again when PHPCS 2.x support will be dropped.
PHPCS 3.x compat: Make setting the tab-width in the unit tests compat…
…ible with PHPCS 3.x

The way the `tab-width` cli value is being passed has changed in PHPCS 3.x.

A helper method would not be useful here as:
1. PHPCS looks for the exact method name in the test class.
2. One of the methods sets something, the other returns something.
PHPCS 3.x compat: Get the unit tests working on both PHPCS 2.x as wel…
…l as 3.x

The principle of how the unit tests will be run is now significantly different for PHPCS 2.x vs 3.x.

* To get the unit tests running, we need to make sure that our class aliases are in place, however to alias the PHPCS native classes, PHP needs to be able to find them, so we need to load the PHPCS native autoloader first.
* Additionally, the PHPCS native autoloader has a bug which means that non-sniff classes (abstracts, helper class) are not loaded correctly. This bug is fixed in PHPCS `master`, but is not contained (yet) in a released version. As a work-around, a WPCS autoloader is registered to make sure our classes can be loaded anyway. This work-around can be removed once the WPCS minimum required PHPCS 3.x version goes up to 3.1.0. See: squizlabs/PHP_CodeSniffer/issues/1564

* The PHPCS 2.x unit test suite does not handle namespaced unit test classes. Or rather: it is not capable of translating the namespaced unit test classes to the namespaced sniff they belong to.
* To work around this, two small snippets of code need to be added to the PHPCS 2.x unit test classes for which we overload select methods.
* However, one of the methods we need to overload is declared as `final`, so for the `AbstractSniffUnitTest` class, we need a complete copy of the file and cannot just overload the relevant method.
* The changes made to the classes are fenced with inline comments annotating the change.
* The code style of these copied in classes has been left as is and these files will be excluded from the WPCS code style check.
* The WPCS version of the `AbstractSniffUnitTest` class is aliased to the PHPCS 3.x name via the PHPUnit bootstrap, so all the unit test classes can already use the PHPCS 3.x class name in their `use` statements.

This also means that the command to run the unit tests will be different in PHPCS 2.x vs 3.x:
```
For running the unit tests with PHPCS 3.x:
phpunit --bootstrap="./Test/phpcs3-bootstrap.php" --filter WordPress /path/to/PHP_CodeSniffer/tests/AllTests.php

For running the unit tests with PHPCS 2.x:
phpunit --bootstrap="./Test/phpcs2-bootstrap.php" --filter WordPress ./Test/AllTests.php
```
The contributing instructions will be updated to reflect this.

In travis, a `PHPCS_BRANCH` environment variable will be available, so there is also a generic `bootstrap.php` file which toggles between the version specific bootstraps based on the environment variable.
PHPCS 3.x compat: Update readme, build script and config files
Travis:
* No longer test the build against PHP 5.2.
* Only test against PHP 5.3 icw PHPCS 2.x.
* Test all other PHP versions against both PHPCS 2.x as well as `master` (3.x).
* Test the build against PHPCS 3.x using the `master` branch.
* Removed allowance for build failures against `master`.
* Adjusted phpunit command to allow for testing on both PHPCS 2.x as well as 3.x.

Composer:
* Add the minimum required PHP version.
* Make the PHPCS requirement more specific and allow for PHPCS 3.x.

"Own" PHPCS custom ruleset:
* Enforce PSR1 namespaces.

Readme:
* Adjusted (minimum) requirements information.
* Updated installation instructions.
* Updated travis example code.

Contributing
* Updated unit test instructions.
* Updated example sniff code.
* Removed reference to potentially upstreaming WPCS to PHPCS as reason for using the PHPCS unit test suite. Let's be fair: the sniffs ought to be unit tested anyway, no matter what.
* Update unit testing conventions example. The example was referring to a now deprecated sniff, so selected another sniff for the example code.
Merge pull request #1047 from WordPress-Coding-Standards/WPCS-0.13.0/…
…feature/phpcs-3.x-compatibility

Make WPCS compatible with PHPCS 3.x
💚 Build: improve our own codestyle checking
* Run the sniffs using a higher PHP and PHPCS version.
* Minor simplification in the command to call phpcs.
* Show warnings, but do not break the build for it. Previously warnings wouldn't be shown at all.
* Move a number of command line options to the ruleset.
* Exclude the Composer `vendor` directory if it exists.
* Update the section in the `Contributing.md` regarding code style
📚 Readme: Update the example output
... to reflect the current state of the `wp-load.php` file in combination with the current WPCS sniffs.
📚 Minor textual improvements to the Contributing.md document
While rewriting the text in the `Contributing.md` document regarding the unit tests, I noticed some more room for improvement.
📚 Minor textual improvements to the Readme.md document
While updating the text in the `Readme.md` document regarding the PHPCS 3.x compatibility, I noticed some more room for improvement.
PHPCS 3.x compat: remove work-arounds for compatibility with PHP 5.2
This should also allow the unit tests to pass against on PHP nightly.
PHPCS 3.x compat: remove test skipping for PHP 5.2
Includes removal of two secondary test case files which were a work-around for unrecognized syntaxes in PHP 5.2.
Merge pull request #1053 from WordPress-Coding-Standards/feature/read…
…me-update-example-output

Readme: Update the example output
Merge pull request #1054 from WordPress-Coding-Standards/feature/cont…
…ributing-text-improvements

Minor textual improvements to the Contributing.md document
Merge pull request #1055 from WordPress-Coding-Standards/feature/read…
…me-textual-improvements

Minor textual improvements to the Readme.md document
Merge pull request #1060 from barryceelen/travis-example
Update Travis CI example in README.md
PHPCS 3.x compat: prevent conflicts with other standards.
Fix a mixup between `class_exists()` and `interface_exists()`:
* `PHP_CodeSniffer_Sniff` is the interface.
* `PHP_CodeSniffer_File` is a class.
Merge pull request #1050 from WordPress-Coding-Standards/WPCS-0.13.0/…
…feature/remove-php-5.2-work-arounds

PHPCS 3.x compat: remove php 5.2 work arounds and PHP 5.2 specific code
Merge pull request #1052 from WordPress-Coding-Standards/WPCS-0.13.0/…
…feature/minor-improvements-own-codestyle-check

Build: improve our own codestyle checking
Merge pull request #1063 from WordPress-Coding-Standards/WPCS-0.13.0/…
…feature/fix-class-interface-mixup

PHPCS 3.x compat: prevent conflicts with other standards.
Update the pre-commit hook code
This update presume that devs using this pre-commit hook will be using PHPCS 3.x.

jrfnl added some commits May 24, 2018

DeprecatedWPConstants: Remove sniff in favour of WPCS sniff
WPCS 0.14.0 introduced a `WP.DiscouragedConstants` sniff which covers all the constants which were covered by the `DeprecatedWPConstants` sniff and does so better than the original sniff.

Removed the `DeprecatedWPConstants` sniff in favour of the new upstream sniff and changing the error level from `warning` to `error` from the ruleset maintains the existing behaviour.

Ref: WordPress-Coding-Standards/WordPress-Coding-Standards 1089
PHPCS 3.x compat: Ruleset: Make sure the complete sniff is included, …
…not just an errorcode

In PHPCS 2.x when a sniff was referenced by errorcode, this automatically included the whole sniff.
As of PHPCS 3.x it is possible to selectively include just and only a specific errorcode.

This allows for cross-version compat and ensures that if/when new errorcodes are added to the sniffs, they will be included as well.
Leverage the WordPress_Sniff class for all sniffs
Most WPCS sniffs extend the WordPress_Sniff class or one of the abstract WP sniffs. However there were some which so far didn't.

This PR changes that whenever possible.

The affected sniffs do not necessarily need to use the WordPress_Sniff class as they don't use any of the utility methods or properties contained therein, however it will make future development slightly more intuitive as (nearly) all sniffs now follow the same pattern.

This same change has been made upstream in WordPress-Coding-Standards/WordPress-Coding-Standards 967
Documentation: Fix two outdated & incorrect code samples
`eval` can not be sniffed for as a function as it is a language construct, not a function, so this was misleading example code.

This same change has been made upstream in WordPress-Coding-Standards/WordPress-Coding-Standards 812
PHPCS 3.x compat: Fix all references to upstream PHPCS classes
* Add use statements for all PHPCS class references.
* Where possible use the PHPCS 3.x class name as a use alias to make the future change of dropping 2.x support easier.
PHPCS 3.x compat: Add autoload directive for the alias file
The `<autoload>` ruleset directive was added in PHPCS 3.0.1 in response to issue squizlabs/PHP_CodeSniffer#1469
Clean up: Remove `// end` comments
The upstream CommentedOutCode sniff has been updated and become more accurate and now throws a number of warnings for // End ... comments.

As the rule regarding end comments has been removed from the handbook anyway, we may as well remove them all and get rid of the warnings.

This same change has been made upstream in WordPress-Coding-Standards/WordPress-Coding-Standards 1362
@jrfnl

This comment has been minimized.

Contributor

jrfnl commented May 25, 2018

I've added an additional commit to be in line with the latest PR merged into WPCS. I've adjusted the above PR message to include the commit message for that last commit.

@grappler

I have reviewed the files that I have been edited in /Theme/. I expect the rest of the changes are from WPCS.

I wonder if we can get the git rename fixed.

@@ -7,34 +7,27 @@
* @license https://opensource.org/licenses/MIT MIT
*/
namespace WordPress\Tests\CodeAnalysis;

This comment has been minimized.

@grappler

grappler May 27, 2018

Member

This file seems to be incorrectly renamed.

This comment has been minimized.

@jrfnl

jrfnl May 27, 2018

Contributor

That's just Git being weird, nothing to worry about.

@grappler grappler merged commit 501380b into feature/theme-review-sniffs May 27, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@jrfnl

This comment has been minimized.

Contributor

jrfnl commented May 27, 2018

I wonder if we can get the git rename fixed.

That's just an artifact of the fact that we do a back-merge here. It may look a little weird when viewing the Git history, but I can't partially back-merge. In WPCS the history is unbroken.

As there is a proposal to make this repo stand-alone, this may soon not be relevant anymore anyway.

@jrfnl jrfnl deleted the WPTRT/feature/update-forPHPCS-3.x-WPCS-0.14 branch May 27, 2018

@grappler

This comment has been minimized.

Member

grappler commented May 27, 2018

Thank you for working on this!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment