Update for PHPCS 3.x and WPCS 0.14.1 #145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…feature/phpcs-3.x-prep-getgroups-in-abstracts PHPCS 3.x prep: work round a function which will no longer exist
…PCS base sniffs Includes adjustment of FQCN references in `@see` and `@uses` documentation.
* 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.
…one in the global namespace
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.
…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
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.
…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.
…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.
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.
…feature/phpcs-3.x-compatibility Make WPCS compatible with PHPCS 3.x
* 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
... to reflect the current state of the `wp-load.php` file in combination with the current WPCS sniffs.
While rewriting the text in the `Contributing.md` document regarding the unit tests, I noticed some more room for improvement.
While updating the text in the `Readme.md` document regarding the PHPCS 3.x compatibility, I noticed some more room for improvement.
Includes removal of two secondary test case files which were a work-around for unrecognized syntaxes in PHP 5.2.
This should also allow the unit tests to pass against on PHP nightly.
…me-update-example-output Readme: Update the example output
…ributing-text-improvements Minor textual improvements to the Contributing.md document
…me-textual-improvements Minor textual improvements to the Readme.md document
Update Travis CI example in README.md
Fix a mixup between `class_exists()` and `interface_exists()`: * `PHP_CodeSniffer_Sniff` is the interface. * `PHP_CodeSniffer_File` is a class.
…feature/remove-php-5.2-work-arounds PHPCS 3.x compat: remove php 5.2 work arounds and PHP 5.2 specific code
…feature/minor-improvements-own-codestyle-check Build: improve our own codestyle checking
…feature/fix-class-interface-mixup PHPCS 3.x compat: prevent conflicts with other standards.
… setting See issue 862
This update presume that devs using this pre-commit hook will be using PHPCS 3.x.
…ase-0.14.1 Release 0.14.1
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
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
`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
…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.
* 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.
The `<autoload>` ruleset directive was added in PHPCS 3.0.1 in response to issue squizlabs/PHP_CodeSniffer#1469
Closed
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
|
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
reviewed
May 27, 2018
| @@ -7,34 +7,27 @@ | |||
| * @license https://opensource.org/licenses/MIT MIT | |||
| */ | |||
|
|
|||
| namespace WordPress\Tests\CodeAnalysis; | |||
There was a problem hiding this comment.
This file seems to be incorrectly renamed.
There was a problem hiding this comment.
That's just Git being weird, nothing to worry about.
grappler
approved these changes
May 27, 2018
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. |
This was referenced May 27, 2018
|
Thank you for working on this! |
This was referenced May 27, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
masterbranch of WPCS as there are some issues which still need to be addressed in WPCSdevelop. Once those have been sorted,developshould 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.DiscouragedConstantssniff which covers all the constants which were covered by theDeprecatedWPConstantssniff and does so better than the original sniff.Removed the
DeprecatedWPConstantssniff in favour of the new upstream sniff and changing the error level fromwarningtoerrorfrom the ruleset maintains the existing behaviour.Ref: WordPress/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/WordPress-Coding-Standards#967
Documentation: Fix two outdated & incorrect code samples
evalcan 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/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
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#1469PHPCS 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
// endcommentsThe 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/WordPress-Coding-Standards#1362
Fixes #139
/cc @dingo-d