#7997 closed defect (bug) (fixed)
PHP version compatibility sniffs
Reported by: | boonebgorges | Owned by: | netweb |
---|---|---|---|
Milestone: | 4.0 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Build/Test Tools | Keywords: | has-patch commit |
Cc: |
Description
Previously, we introduced PHP linting into our build/test process: #7926.
Leveraging php -l
is a good start for catching blatant syntax errors, but it's very limited in terms of what it catches. Removed functions and invalid (but syntactically correct) constructs are a few examples of things that can only be caught by tokenized analysis. See eg #7925.
I propose that we switch to using PHPCS with the PHPCompatibilityWP sniffs. This change provides us with dramatically better coverage of deprecated usages, on a per-PHP-version basis. It's sensitive to WP's polyfills (for functions like hash_equals()
). And it combines all of the version compatibility checks into a single run, so that the checks only need to be run once in the Travis matrix (instead of separately on each version of PHP).
The incoming patch shows an implementation of this. It does the following:
- Installs phpcompatibility-wp and its dependencies via Composer
- Adds a Grunt task
exec:phpcompat
. (There's agrunt-phpcs
package, but it doesn't let you pass--runtime-set
flags.) I called itphpcompat
because we will likely want to introduce aphpcs
task in the future for coding standards more generally. - Adds
exec:phpcompat
to thegrunt commit
task, replacing the now-redudantphplint
- Adds a Grunt task
travis:phpcompat
task, parallelingtravis:codecoverage
andtravis:grunt
, so that all PHP compatibility checks happen in their own job
To run locally, composer update
(or install
) && npm update
, then grunt exec:phpcompat
. To see in action on TravisCI,
https://travis-ci.org/boonebgorges/BuddyPress (Note that we don't currently pass! That'll be the subject for another ticket.)
Attachments (1)
Change History (11)
#3
@
6 years ago
- Milestone changed from Up Next to 4.0
Thanks for having such a quick look, @netweb !
#7
@
6 years ago
- Resolution set to fixed
- Status changed from new to closed
This is now working, with the caveat that the Composer requirements mean that composer install
now fails on PHP 5.3. And since it's only for require-dev
, it likely won't make a difference for anyone using Composer to pull into a project. I'm going to mark this as closed, with the note that we should revisit some aspects of the current setup when we drop PHP 5.3 support.
#8
@
5 years ago
Just came across this ticket.
While I - of course - fully support adding PHPCompatibilityWP
to the QA toolset, it is not a replacement for linting code.
When PHPCompatibility encounters parse errors which are not typically to do with PHP cross-version compatibility, it will ignore them and not notify you of these.
Also, in case of parse errors, the results of PHPCompatibility may not be reliable as the file may be tokenized incorrectly by PHP/PHPCS because of the parse errors.
I'd strongly recommend using php -l
against at least the lowest and highest PHP version, if not lowest/highest of each PHP major in addition to using PHPCompatibility.
Nice one, tested and works for me 👍