Make WPCS compatible with PHPCS 3.x #1047
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
…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.
This was referenced Jul 24, 2017
GaryJones
approved these changes
Jul 24, 2017
CONTRIBUTING.md
Outdated
| If not, you can navigate to the directory where the `PHP_CodeSniffer` repo is checked out and do `composer install` to install the `dev` dependencies. | ||
| Alternatively, you can [install PHPUnit](https://phpunit.de/manual/5.7/en/installation.html) as a PHAR file. | ||
|
|
||
| **Pro-tip**: PHPUnit is easiest to work with if you add the installation directory to your `PATH` environment variable. |
There was a problem hiding this comment.
This, and line 64, seems to be encouraging global install of PHPUnit, which isn't the best practice.
Line 64 I'm fine with, but I'm not sure that this tip is relevant here (even if it is accurate).
There was a problem hiding this comment.
I'll remove the "Pro-tip".
This, and line 64, seems to be encouraging global install of PHPUnit, which isn't the best practice.
Isn't it (anymore) ?
It's kind of what we are encouraging in our installation instructions for WPCS and PHPCS.
I've always thought that PHPUnit falls under the same umbrella of global tools, though I imagine that version requirements may differ more widely for projects.
There was a problem hiding this comment.
OK - I should clarify - it's not best practice according to my understanding of https://12factor.net/dependencies (or https://roots.io/twelve-factor-02-dependencies/ if you prefer).
For each of my projects, I include WPCS as a require-dev. I only use the global git checkout of PHPCS and WPCS for Editors which don't have project-specific settings.
There was a problem hiding this comment.
@GaryJones I've already remove the pro-tip part.
Other than that, I think how to handle dependencies and what dependencies are, is a different discussion. IMO there is a difference between application dependencies and dev tools, but that's for another day (while sitting in the 🌞 somewhere with a glass of 🍺 or 🍷 )
JDGrimes
approved these changes
Jul 24, 2017
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.
jrfnl
force-pushed
the
WPCS-0.13.0/feature/phpcs-3.x-compatibility
branch
from
f005097
to
3070fb2
Compare
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.
This PR provides cross-version compatibility for WPCS with PHPCS 2.x as well as 3.x.
The minimum PHPCS 3.x requirement will be PHPCS 3.0.2 due to various bugs in earlier versions of the PHPCS 3.x autoloader.
Please see the individual commits for more detailed information. The commits have been set up to facilitate easier code review.
For end users, this PR will allow them to upgrade to PHPCS 3.x. Aside from the path to the
phpcscommand changing in PHPCS 3.x (upstream change), there is no backward-compatibility break for them.For sniff developers who contribute to WPCS, the way to run the unit tests changes and - unfortunately - will be different for running these in combination with PHPCS 2.x vs 3.x.
The information regarding running the unit tests in the
CONTRIBUTING.mdfile has been updated to reflect this change.The PR has been set up with eventually dropping PHPCS 2.x support in mind. Where possible I've made choices which will make the update to WPCS to drop PHPCS 2.x support easier and less involved.
I will open a separate issue documenting how to drop PHPCS 2.x support.
Fixes #718