PHPCS 3.5 - up minimum version or provide cross-version support ?

[jrfnl]

PHPCS 3.5.0 will contain major (non-breaking) changes under the hood. The idea for making those changes in PHPCS now, in a non-breaking way, is to make the transition to and - optionally - dual version support with, PHPCS 4.x easier and smoother.

Aside from deprecating a large number of the utility methods in the File class in favour of moving these - sometimes in improved form - to dedicated utility classes, the release will also introduce a whole slew of new utility methods to PHPCS, some based on or inspired by the utility methods which currently already exist in the WPCS Sniff class.

This should make life easier in the long run as maintenance for those is then centralized, they will benefit from bugs reported and fixed by people maintaining other external standards and the functions will be more easily available to other (WPCS-adjacent and other) external standards.

While the release of PHPCS 3.5.0 may still be a few months off, I think it would be a good idea to have a think about how we want to handle this, especially with semver and the fact that there are numerous external standards which depend on WPCS (and may need to be adjusted as well), in mind.

In my opinion there are three options:

1. Cross-version support.
   • For at least a little while, maintain the current minimum required PHPCS version (whatever that is at the time, currently 3.3.1) as is.
   • Don't implement any of the new methods yet, though possibly start to make an inventory of where they can be applied.
   • Add method_exists toggles to WPCS methods which now have equivalent or better upstream versions.
2. Raise the minimum required PHPCS version to 3.5.0.
   • Implement any and all of the new methods from which WPCS can benefit.
   • Deprecate the WPCS native methods which now have an upstream version. Where the results would be the same, route them through to the new methods under the hood or keep the existing code in place if the upstream method is a significantly different implementation.
     The deprecated methods will be removed in the next major version of WPCS.
   • Note: This will allow us to more easily start creating WPCS native documentation sniffs as some major improvements/helpful utility functions/an abstract docblock class are expected to be included in PHPCS 3.5.0.
3. Prepare a new major version of WPCS with a minimum required PHPCS version of 3.5.0.
   • Implement any and all of the new methods from which WPCS can benefit.
   • Remove the WPCS native methods for which there will now be upstream alternatives.
   • Optionally, do a similar refactor for WPCS for the remaining utility methods. Related Extract re-usable code from Sniff.php into new package #1465

Opinions ?

[dingo-d]

I wouldn't raise the major version since you mentioned that the changes are non breaking. I'd leave the major change (3.0) for the release of PHPCS 4.0

[jrfnl]

The changes are (mostly) non-breaking for PHPCS, but when we'd remove the WPCS versions of the methods, that would be a breaking change for WPCS, i.e. a change requiring the major to change.

[NielsdeBlaauw]

I feel option 2 would have the least impact on users while not creating additional complexity for maintenance and development.

[jrfnl]

Little update about this:

The refactor this issue started with, will (for now) not go into PHPCS. Instead a new PHPCSUtils repo has been created which will contain the utility methods which were contained in the refactor and more.

The new repo will be compatible with PHPCS 2.6.0 and higher and will also provide compatibility methods for PHPCS native utility methods which have undergone changes between PHPCS 2.6.0 and master.

If you're interested, feel free to watch the repo.
I expect to start publishing code there over the next week/next few weeks.

So, now the options we need to think about for WPCS will be slightly different.

1. We won't need to raise the minimum PHPCS version to start using the PHPCSUtils utility methods.
2. However, the installation instructions for WPCS would change, which, AFAICS, would mean the change over can only happen in a major version.
   • The installation instructions for Composer won't change significantly.
     Basically, things will just work if WPCS is installed via Composer. The only thing which would need to change there is that we can remove the recommendation to install the DealerDirect Composer Plugin as that will now be installed by default as a dependency of PHPCSUtils.
     Instead we should recommend that people remove that dependency if they have it, to prevent version conflicts.
   • The installation instructions for a non-Composer install will change as users will need to also install/clone the PHPCSUtils repo and register it as a standard with PHPCS.

So, the choices we now have to make are:

1. Do we want to start using PHPCSUtils ?
   My own vote for this will be a very loud: "yes", if for no other reason than that it will make my life easier in not having to maintain the same utility functions in various repos.
   Aside from that, better versions of existing utility functions and new utility functions will become available through that repo, so using it will give us more tools to build effective sniffs.
2. If yes, does everyone agree that the change-over should be done in a major version ?
3. If yes, how do we want to handle deprecation/removal of existing WPCS utility methods which won't be needed anymore ?
   • If the change-over is done in a major, we could just delete them.
   • Alternatively, if we want to be a bit more careful/considerate to standards build upon WPCS, we could deprecate the methods and under the hood point to the new PHPCSUtils methods and use those in our codebase already.
     In the next major after that, we could then remove the deprecated methods.
4. If yes, what timeline should we aim for ?
   Personally, I think we should probably release a new minor with the current changes in develop and preferably some more changes from the open PRs in the near future.
   The change-over to using the PHPCSUtils repo could happen soon after that.

Opinions ?

Other notes:

• PHPCSUtils will be released under the LGPL-v3 license. AFAICS that should be fine to combine with WPCS (MIT), as long as it's pulled in as a dependency. Correct ?
• The methods and abstract classes which will initially be contained in PHPCSUtils are largely methods and classes which I've written previously for PHPCompatibility or WPCS, which have not been touched by anyone else.
  That means there is no copyright issue with those.
  However, there are also some utility methods in WPCS which I didn't originally create, but did improve over time. Some of those could be candidates for PHPCSUtils (but aren't in the initial setup).
  How should this be handled ?

[dingo-d]

I'm just saddened that the util didn't get merged to PHPCS, especially after all the time you've spent in working on it.

I'm definitely for using this, as all the other standards will benefit from it (I know that I will probably find a few places where utils can be used in WPThemeReview).

I'm all for using Utils as soon as possible. Since this is a separate repo, we can use it in the external standards. but I'd rather have one place that is using it as a dependency, than adding it in the external repo, then have to take extra care when the WPCS adds it (guess this is just me being lazy 😄 ).
I think we should see what issues/PRs we want to have in the next minor releases prior to adding this in the major. After separating those in the milestones we'll have a better look on how to approach this.

However, there are also some utility methods in WPCS which I didn't originally create, but did improve over time. Some of those could be candidates for PHPCSUtils (but aren't in the initial setup).

If we keep the original commits, and if the license is not clashing with the WPCS license then it should be ok to use 🤷‍♂

[jrfnl]

If we keep the original commits, and if the license is not clashing with the WPCS license then it should be ok to use

As it is a separate repo and unrelated to WPCS, keeping the original commits is not an option.
I could look into adding additional committers to a commit which would add a function or, at the very least, giving people credit in the commit message, but that's about all I can do in that regards.

[dingo-d]

If their license is compatible, just mentioning where the original code came from and the author who made it should be ok if I'm not mistaken.

[jrfnl]

Closing. As discussed here and in #1877, PHPCSUtils will become a dependency for WPCS 3.0+ and this has been actioned in #1905.