Skip to content

Tentatively explore making PHPCSUtils a runtime dependency #1364

@jrfnl

Description

@jrfnl

Background

A long, long time ago.... 🐉🏰

Okay, not that long, but still a good few years back, I proposed a partial refactor of the utility function side of PHP_CodeSniffer.

After nearly a year of work on this refactor, with little progress towards actually merging the PR, it was decided to not pursue this any longer within PHP_CodeSniffer itself, but to publish the work on new and improved utility functions as a separate package.

Hence PHPCSUtils was born 🐣

Since then, PHPCSUtils has grown with more utility functions being added regularly and yet more lined up for the future.

PHPCSUtils has, by now, been adopted by a number of widely used external PHPCS standards, like PHPCompatibility, PHPCSExtra, WordPress Coding Standards, Magento Coding Standards, as the go-to library for utility functions for writing sniffs.

If anyone wants to read up on the history:

Advantages and disadvantages of it being a separate package

At the time it was decided to publish these utilities as a separate package, the advantages and disadvantages of it being a separate package were listed as follows:

The advantages of having the utility functions available as a separate package:

  • The utilities would be available for PHPCS 3.x+.
    External standards wouldn't need to raise their minimum PHPCS requirement.
  • Turn-around speed.
    I already have quite a few other classes and functions lying around which should be added to the Utility function suite, but I haven't added them here as I didn't want to muddy the waters for this PR/make reviewing more difficult with things being added all the time.
    And I fear that even if this PR would be merged, the turn-around time for adding additional utilities would still be just as slow as things have been for the past few years, so having a separate package would make these utilities available to external standards a lot faster.
  • I would still get rid of the maintenance burden of maintaining the duplicate code of these utility functions in a number of external standards and could start cleaning up those standards.

The disadvantages of it being a separate package:

  • The multitude of bugs in PHPCS itself would not get fixed (unless PHPCS decides to add the utility functions package as a dependency and implement the use of it).
  • I have quite a number of local branches with PSR-12 sniffs and other fixes ready to be pulled, but can't pull them to PHPCS as they need the utility functions/other changes from this PR (or would contain lots of duplicate code which would have to be removed at a later point or maintained in several places now, which I'm really not keen on).
    I'd need to think it over, but it's quite likely that I would publish them in a separate external PSR-12 standard instead of pulling them here if the utility methods would become a separate package.
    (Red.: a lot of these have since been published via PHPCSExtra)

So, in other words, advantages for everyone, except PHPCS itself.

Since then, the "landscape" in which PHPCS lives has changed, with me taking over as the primary maintainer and the repo moving to the PHPCSStandards organisation, in which PHPCSUtils lives as well.

Should PHPCSUtils become a runtime dependency for PHPCS ?

In a recent PR, which proposed adding some functionality already available in PHPCSUtils to PHPCS itself, the question came up whether PHPCSUtils and PHPCS should be merged or if PHPCSUtils should become a runtime dependency for PHPCS, so PHPCS itself can benefit from the functionality offered by PHPCSUtils.

In my opinion, this would be a good thing™ for the following reasons:

  1. Will allow to solve some long-standing bugs in PHPCS native sniffs (in particular related to short array vs short list, but also related to fixer conflicts).
  2. Will allow to reduce code duplication and lower the maintenance burden.
    At this time, PHPCSUtils contains "mirror"-methods of the PHPCS native utility functions for PHPCS cross-version compatibility, as well as containing its own versions of a lot of these methods.
    This means that both the utility method code, as well as the associated tests for these functions, are now maintained in duplicate and sometimes even in triplicate.
  3. Will allow to improve sniff performance for the PHPCS native sniffs.
    PHPCSUtils includes a form of runtime caching, which means that if several sniffs call the same utility function for the same token (in the same file), the results will be returned instantaneously the second time the function is called.
    For sniffs using utility functions which do a lot of token walking, this significantly improves the performance of these sniffs.
  4. Will improve the discoverability of the available utilities for sniff developers.
    Originally, the PHP_CodeSniffer documentation was automatically published via the PEAR website.
    However, this mechanism was broken on the PEAR site as of the PHPCS 3.5.5 release, making the PHPCS 3.5.4 documentation of the available utility functions the last one published.
    In contrast, the documentation for PHPCSUtils is current and available via a GitHub Pages website.

This issue is intended to "feel the waters" of how the community feels about making the relation between PHPCS and PHPCSUtils tighter, as well as to explore what form this should take.

/cc @asispts @dingo-d @fredden @GaryJones @greg0ire @kukulich @michalbundyra @Ocramius @sirbrillig @stronk7 @weierophinney @wimg

Investigations/decisions needed

Aside from the primary decision of whether or not any action should be taken on this at all, there are additional things which will need to be investigated/decisions which will need to be taken before any action can be taken.

  • Investigate the advantages and disadvantages of the two (or more?) ways in which the projects could be combined.
    This includes an impact analysis of the potential BC breaks for external standards.
    Keep in mind that the project supports multiple installation methods, such as Composer, but also PHAR and that a PHAR file which includes PHPCSUtils should not conflict with a Composer installed version of PHPCSUtils if installed via an external standard.
    The following two options come to mind, but I'd be open to hearing additional options:
    1. Make PHPCSUtils a runtime dependency for PHP_CodeSniffer.
    2. Deprecate PHPCSUtils and move the PHPCSUtils functionality into the PHP_CodeSniffer repository.
  • Investigate what would be needed to make the path towards integrating the projects as smooth as possible for existing projects already using PHPCSUtils (and how to prevent conflicts for those projects).
  • If PHPCSUtils would become a runtime dependency, investigate which functionality which has duplication should live where.
    Think: the PHPCS File::getMemberProperties() method - should the PHPCS version be deprecated in favour of the PHPCSUtils Variables::getMemberProperties() method ? Or should the File::getMemberProperties() method be updated to match the PHPCSUtils version of the method and the PHPCSUtils version be deprecated ?
    But also, and this ties back to the PR which inspired this issue, should certain "token collections" which are currently only available via PHPCSUtils, be moved to PHPCS ?
  • Think through the timing/release planning of the potential steps.
    This needs to take into account that there are external standards which already use PHPCSUtils as well as external standard which may need to start using PHPCSUtils.

Opinions

Opinions welcome, as well as input about things I may be overlooking and/or additional things which may need investigating if we go down this road.

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions