Compatibility with PHPCS 3.x

[jrfnl]

The preparations for PHPCS 3.x are ongoing and PHPCS 3 is expected to be released over the next few months. Current version is PHPCS 3.0-RC4.

The upgrade guide for sniff developers makes it very clear that there will be no way whatsoever to have one standard be compatible with both PHPCS 2.x as well as PHPCS 3.x.

Once PHPCS 3.x is released, I expect other external standards to start upgrading as most only support PHPCS 2.x now anyway.

For end-users who use both another external standard as well as PHPCompatibility, this will cause an insurmountable dependency issue with the other standard depending on PHPCS 3.x and PHPCompatibility depending on PHPCS 2.x/1.x.

I think now would be a good time to start thinking about a strategy for PHPCS 3.x compatibility for this sniff standard, so it can be solved relatively quickly once PHPCS 3.x has been released.

There are a couple of strategies I can think of, but this list is by no means complete:

• Have a separate branch within this repo for PHPCS 3.x with its own tagged releases. This would need some careful thinking about the tag names as we'd need to make sure that people get the right version if they use packagist.
• Have a secondary repository which is regularly synced with this repo for the PHPCS 3.x version.
• Drop PHPCS 1.x and 2.x support completely and make the current repo PHPCS 3.x compatible (probably the least favoured option).

Both option 1 and 2 would add an additional maintenance burden as every single PR would have to be merged twice and reviewed for incompatibilities with PHPCS 3.x

Option 2 would additionally potentially cause issues with PRs being pulled against the 3.x version only and never making it into this repo.

Option 3 would be the simplest, but would give issues for users who have no choice of PHPCS version used as they are locked into a version for whatever reason.

Opinions and additional options to explore very welcome!

---

Action list to make the library compatible with PHPCS 3.x:

• Add namespaces and use statements to all sniffs.
• Change all calls to PHPCS native functions/classes/constants, typehints etc to namespace compatible calls/typehints - in both sniffs as well as the unit test suite.
• (Optional) Remove all PHPCS 1.x and PHPCS 2.x < current work-arounds.

This might also be the right time to solve issue #102 / #107 - at least for the PHPCompatibilty PHPCS 3.x version.

[MarkMaldaba]

Would it be possible to implement a shim? That would be the preferred method, as it would retain full support without requiring multiple versions of the code.

For example, could we add a bunch of classes implemented like this, and only included when necessary:

class PHP_CodeSniffer_Sniff extends PHP_CodeSniffer\Sniffs\Sniff { }

That would be the best/simplest solution if it would work (which it may not - I haven't investigated).

Failing that, I think option 3 (dropping v1/v2 support) is a very bad idea. I expect there will be a lot of people who won't upgrade to PHPCS v3 for quite a while, because of the massive re-tooling it will require if you have custom sniffs. Therefore there will still be a big demand for PHPCS 2.0 support, at least.

It seems like a dumb decision on behalf of the PHPCS author - I can't see how the switch to namespaces gives any functional benefit (beyond a nice warm fuzzy feeling - yay, namespaces!), but it sure causes a headache for everyone down-stream. However, it's probably too late to do anything about that.

[jrfnl]

@MarkMaldaba

Would it be possible to implement a shim?

Unfortunately I don't think that will be possible. See the upgrade guide I linked to above.

The most important points to take away are:

• All sniffs have to be namespaced to be compatible with PHPCS 3.x which would break the support in the PHPCompatibility standard for PHP 5.1 and PHP 5.2 straight off.
• All sniffs have to be namespaced and while I do believe PHPCS 2.x already supports a namespaced standard, I'm pretty sure it would break compatibility with PHPCS 1.x, though both (PHPCS 1.x and 2.x support for namespaces) would need to be tested
• The shims would need to cover a lot of ground as we would need shims for methods, properties and constants from the PHP_CodeSniffer, PHP_CodeSniffer_File, PHP_Codesniffer_Tokens, PHP_CodeSniffer_Exception classes.
• We'd also need an interim method for the abstract process() method in the PHP_CodesSniffer_File class as the type hint has changed which would cause a fatal error.
• etc.

However, it's probably too late to do anything about that.

It is (too late) and while I understand where you're coming from, we should also be realistic. PHPCS 3.x has a minimum requirement of PHP 5.4 which itself is an antiquated version of PHP which hasn't been supported for two years now.
It's not as if the minimum requirement has gone up to PHP 5.6 or even PHP 7.

[MarkMaldaba]

I read the upgrade guide, but the implications weren't fully clear. Some of the points you make can be fixed by a shim, but perhaps not all of them, which obviously scuppers things if that is the case.

It's not as if the minimum requirement has gone up to PHP 5.6 or even PHP 7.

It's not the version requirement, so much as the API breakage, for what appears to be no good reason aside from 'shiny code'. Thousands of developer hours will now have to be spent updating existing standards so they use Foo/Bar/Baz instead of Foo_Bar_Baz, for little material gain.

[jrfnl]

This was discussed at length during the meeting @wimg and me had last Wednesday.

The current intention is to try and find a way to keep everything within one standard and work with shims to work-around the differences between PHPCS 1.x/2.x and 3.x.

From comments in upstream issues, it looks like we should be able to use namespaces with both PHPCS 1.x/2.x, though this will still need to be thoroughly tested.

The one thing which I think might still throw a spanner in the wheels is the typehint used in the process() method by PHPCS as that will start throwing errors for the method signature not matching the one declared in the interface.
If anyone has a bright idea about that, please pitch in.
The only solution I can currently think of, is removing the implements ... part of the PHPCompatibility class definition to get round it.

[jrfnl]

FYI: PHPCS 3.0 has just been released. https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.0.0

[GaryJones]

Could you do something like http://develop.svn.wordpress.org/trunk/tests/phpunit/includes/phpunit6-compat.php ?

[jrfnl]

@GaryJones Thanks for that link. Interesting way to do the cross-version compat for PHPUnit. It probably won't be as easy as that with PHPCS, but the principle of what we intend to do is similar.

[jrfnl]

Status update:

I've started looking into this and while I think I can get it working, we will need to wait for a new upstream release containing a number of fixes for bugs I've found while testing.

I've made the following PRs to fix what I've come across so far:
squizlabs/PHP_CodeSniffer#1450
squizlabs/PHP_CodeSniffer#1451
squizlabs/PHP_CodeSniffer#1452

[jrfnl]

Oh and @GaryJones Thanks again for that link. That definitely solved the type hint issue!

[jrfnl]

Yet another issue found: squizlabs/PHP_CodeSniffer#1453 (not yet resolved)

[real-or-random]

I think this is a similar problem:
https://travis-ci.org/real-or-random/dokuwiki-plugin-icalevents/builds/229644145

I'm not sure if it's the same as squizlabs/PHP_CodeSniffer#1453 .

[jrfnl]

Forgot to add this link to the thread: squizlabs/PHP_CodeSniffer#1447 (yet another issue, though this has been fixed since I reported it yesterday)

[jrfnl]

@real-or-random No, it is not. You're trying to (already) run on PHPCS 3.x which is why it's not working.
You need to change the following line in your composer file to limit it to the PHPCS 2.x branch:

"require-dev": {
 "squizlabs/php_codesniffer": "~2"
}

You probably want something like "^2.8"

Edit: only just noticed you've changed number of things already since you posted. Either way, the issue you are having is that you're trying to use PHPCS 3.x already while PHPCompatibility is not ready for it yet.

[real-or-random]

~2 does not include 3. (Even though it's not so clever probably.)

And yes, it does not work because it's related to PHPCS 3.x. That's why I added it to this thread. ;)

[PatrickRose]

Is there anything I can do to help work on this? I'm in the process of changing our PHPCS pipeline slightly so I was going to update to PHPCS 3 while I was at it.

[wimg]

We're currently waiting for PHPCS 3.0.1 to be released, which should fix the bugs that are blocking.

[bfintal]

Just a heads up, 3.0.1 is out :)

[syrm]

I'm waiting for this too

[jrfnl]

All: PHPCompatibility 7.1.5 has just been released. Version 8.0.0 will be compatible with PHPCS 3.x and should be released soon.

[jrfnl]

FYI: The PR for this has been pulled - #482. Testing appreciated!