Drop PHPCS 2.x support by August 31, 2018

[jrfnl]

As proposed in this comment #718 (comment), PHPCS 2.x should be dropped by August 31, 2018 at the latest.

That gives other external standards which build onto WPCS a year's time to upgrade their standards.

This issue is intended to document what needs to be done to drop PHPCS 2.x support, based on the PHPCS cross-version implementation as pulled in #1047.

Prerequisite: The minimum supported PHPCS version for WPCS will need to be 3.1.0 or higher.

Sniff files:

• Change namespace from namespace WordPress... to namespace WordPressCS\WordPress.
• Change the use statements which refer to PHPCS classes to use the PHPCS 3.x names:

  // Current:
  use PHP_CodeSniffer_Sniff as PHPCS_Sniff;
  use PHP_CodeSniffer_File as File;
  use PHP_CodeSniffer_Tokens as Tokens;
  
  // Should become:
  use PHP_CodeSniffer\Sniffs\Sniff as PHPCS_Sniff;
  use PHP_CodeSniffer\Files\File;
  use PHP_CodeSniffer\Util\Tokens;

Rulesets:

• All: remove the <autoload>./PHPCSAliases.php</autoload> statement.
• ./WordPress/ruleset.xml: adjust <ruleset name="WordPress" namespace="WordPress"> to <ruleset name="WordPress" namespace="WordPressCS\WordPress">
• Review which customizations currently done per error code can now be done for the complete sniff in one go. Related Ruleset: maintain compatibility with PHPCS 2.x #1144
• Review other notes in the ruleset about additional customizations / sniff replacements which can be done once the minimum requirement has gone up & action them

Cleanup:

• Remove the ./WordPress/PHPCSAliases.php file.
• Remove the ./phpunit.xml.dist file.
• Remove the ./Test subdirectory and all the files therein. The bootstraps and the PHPCS 2.x test suite files are no longer needed as long as the minimum PHPCS version goes up to 3.1.0.
• Decide whether to keep or remove the ./WordPress/PHPCSHelper.php file. It might be useful to leave it in place to be used for future PHPCS cross-version compatibility issues. The PHPCS 2.x specific code in the file can be removed.

Build script and config files:

• .travis.yml: Remove the PHPCS 2.x specific PHPUnit commands.
• .travis.yml: Stop testing against PHP 5.3 (not supported by PHPCS 3.x).
• .travis.yml: Stop testing against PHPCS 2.x branches.
• .travis.yml: Remove the toggle for the PHPCS_BIN location.
• .travis.yml: Limit the downloading of a custom PHPUnit version to HHVM, PHPCS supports PHPUnit 6.x since PHPCS 3.1.0. See Fix travis build: load PHPCS native bootstrap if available and test against PHP 7.2 #1130
• .travis,yml: If needed: add the PHPCS native bootstrap to the PHPUnit command and/or the phpunit.xml.dist file. See Make the unit test suite compatible with PHPUnit v6 squizlabs/PHP_CodeSniffer#1384
• Update the minimum required PHPCS version in the composer.json file.

Documentation:

• Readme: update minimum requirements & remove references to PHPCS 2.x.
• Contributing: update the unit test instructions.
• Remove the PHPCS 3 and testing individual sniffs wiki page as it should no longer apply.

Dropping PHP 5.3 support:

• Remove code related to the short_open_tag ini setting from the XSS.EscapeOutput sniff.
• Remove the short_open_tag related line from .travis.yml.
• Remove unit tests work-around for short_open_tag ini setting icw PHP 5.3 in the CodeAnalysis.EmptyStatement, Files.FileName, WhiteSpace.PrecisionAlignment sniffs. See PrecisionAlignment: fix various bugs #1273, Improve handling short echo open tag #1274

[jrfnl]

As a side-note: any WPCS sniff which would be pulled upstream over the next year can not (yet) be dropped from WPCS as the upstream version will only go into PHPCS 3.x.

So, if any WPCS sniffs would be upstreamed:

• Deprecate / Remove any sniffs of which there is now an upstream version and replace ruleset references to the WPCS version with the upstream sniff name.

---

Candidates to be removed/replaced with upstream version once PHPCS 2.x support has been dropped:

• CodeAnalysis.AssignmentInCondition sniff / PR Add new CodeAnalysis.AssignmentInCondition sniff to Extra #1093 - merged upstream in 3.1.0. The ternary part which is included in the WP version of the sniff does still need to be pulled upstream.
• PHP.DiscourageGoto sniff / PR Add New "WordPress.PHP.DiscourageGoto" sniff to Extra #1150 - merged upstream in 3.2.0
• Functions.FunctionCallSignatureNoParams can be removed if the minimum version goes up to PHPCS 3.1.1 - see PEAR.Functions.FunctionCallSignature enforces spaces when no arguments if required spaces is not 0 squizlabs/PHP_CodeSniffer#1512, the fix for which will be included in 3.1.1
• WhiteSpace.SemicolonSpacing sniff / PR Improve fixes for PEAR.Files.IncludingFile.BracketsNotRequired #1163 - fixed for the related issues has been merged upstream in 3.2.0
• CodeAnalysis.EmptyStatement sniff / PR ✨ New CodeAnalysis.EmptyStatement sniff #980 - upstream PR open New Generic.CodeAnalysis.EmptyPHPStatement sniff squizlabs/PHP_CodeSniffer#1814 - merged upsteam in 3.4.0.
• WhiteSpace.ArbitraryParenthesesSpacing sniff / PR New ArbitraryParenthesesSpacing sniff for Core #1195 - upstream PR New Generic.WhiteSpace.ArbitraryParenthesesSpacing sniff squizlabs/PHP_CodeSniffer#1701 merged in PHPCS 3.3.0

[jrfnl]

Another one where a WPCS work-around may be removed if PHPCS merges the upstream fix: squizlabs/PHP_CodeSniffer#1689 / #1185

[Edit:] upstream fix has been merged and will be included in the PHPCS 3.2.0 release.

[jrfnl]

A readme change which can be made once the minimum version has gone up to PHPCS 3.1.0 or higher:

• Recommend naming a custom project ruleset .phpcs.xml.dist (note the . at the start of the filename)

[jrfnl]

The PHPCS native ignore annotations will be changed in v3.2.0. The old type of annotations will be deprecated and they will be removed in PHPCS 4.x.

If WPCS would raise it's minimum PHPCS requirement to 3.2.0 when the work to drop PHPCS 2.x support will be done, this would be a good moment to address the change in annotations in all (unit test case) files.

It would also be a good moment to evaluate whether and if so, when to drop/deprecate the WPCS native whitelist comments.

From the upstream changelog:

  - This release deprecates the @codingStandards comment syntax used for sending commands to PHP_CodeSniffer
    -- The existing sytax will continue to work in all version 3 releases, but will be removed in version 4
    -- The comment formats have been replaced by a shorter syntax:
       --- @codingStandardsIgnoreFile becomes phpcs:ignoreFile
       --- @codingStandardsIgnoreStart becomes phpcs:disable
       --- @codingStandardsIgnoreEnd becomes phpcs:enable
       --- @codingStandardsIgnoreLine becomes phpcs:ignore
       --- @codingStandardsChangeSetting becomes phpcs:set

For more details:

• Selectively disable rules with codingStandardsIgnoreStart and codingStandardsIgnoreLine squizlabs/PHP_CodeSniffer#604
• squizlabs/PHP_CodeSniffer@5325170
• squizlabs/PHP_CodeSniffer@389e52f
• squizlabs/PHP_CodeSniffer@1f07a69
• squizlabs/PHP_CodeSniffer@a32e481 (example)

[Edit]: Also keep an eye on squizlabs/PHP_CodeSniffer#1819 and in what version that will be implemented.

[Update:] Support for prefixing the new whitelist comments with a @ has been been implemented and will be in the PHPCS 3.2.3 release.

[Update]: As of PHPCS 3.2.3, there will also be a Tokens::$phpcsCommentTokens array available to help identify PHPCS whitelist comments more easily.

[Update]: There were a lot of bugs in the implementation upstream of the new PHPCS annotations, on the one hand with the tokenizer not tokenizing the annotations correctly, on the other hand with upstream sniffs not allowing for the PHPCS annotations in their code examination.

I've fixed most of the issues and most of the related PRs have gone into PHPCS 3.2.3, though I suspect that PHPCS 3.3.0 will contain yet more fixes for sniffs being incompatible with the new annotations, so I highly recommend the minimum PHPCS dependency to be upped to PHPCS 3.3.0 or higher.

For reference, I'm referring to these upstream PRs: 1820, 1827, 1831, 1832, 1833, 1834, 1835, 1836, 1837, 1839, 1841, 1843, 1848, 1901, 1904 - all of which were merged in PHPCS 3.2.3.

[jrfnl]

The NamingConventions.ValidVariableName sniff currently contains a $php_reserved_vars property. Once upstream PR squizlabs/PHP_CodeSniffer#1888 has been merged, that property can be removed and the upstream $phpReservedVars property can be used instead.

[Update]: The upstream PR has been merged and will be included in PHPCS 3.3.0.

[jrfnl]

PHPCS upstream introduces a new Generic.WhiteSpace.LanguageConstructSpacing sniff to replace the Squiz.WhiteSpace.LanguageConstructSpacing sniff in v 3.3.0.
Once the minimum requirement of WPCS has gone up to 3.3.0, we should switch over.

Also see: #1328

[jrfnl]

More upstream goodies:

A PR has just been merged and will be included in the 3.3.0 release which changes the array format for properties.
The old format will remain valid as well for now, but is expected to be removed in v4.

If/when the minimum PHPCS version WPCS support goes up to 3.3.0, we should:

• Adjust the array properties used in the WPCS ruleset (if any).
• Adjust the wiki page regarding the WPCS custom properties and update all example code for array properties, such as the text-domain.
• Advise people through the changelog that the new array format is recommended for their own custom rulesets.

Old format:

<property name="forbiddenFunctions" type="array" value="print=>echo,create_function=>null" />

New format:

<property name="forbiddenFunctions" type="array">
    <element key="print" value="echo"/>
    <element key="create_function" value="null"/>
</property>

See: squizlabs/PHP_CodeSniffer#1665

---

Additionally, array formats are as of PHPCS 3.3.0 properly covered for the new PHPCS // phpcs:set annotation, which is used by the unit tests, so those can be adjusted too.

Also: some of the "hacks" we had in place to deal with array values for properties in unit tests can possibly be removed 🎉

By "hacks", I mean:

• allow array values to be passed as string and exploded on the ,
• resetting an array property by passing false

See: squizlabs/PHP_CodeSniffer#1999

[GaryJones]

When we drop 2.x support, will we then have a minimum of 3.3.0 (or whatever release makes sense at the time), rather than 3.0.0?

[jrfnl]

@GaryJones Based on the interesting things which have been happening in PHPCS, my current recommendation would be to have a minimum of 3.3.0.

• The whole 3.0 range is buggy for external rulesets because of autoload issues.
• 3.1 versions are ok.
• 3.2 adds the awesome new selective ignore syntax, but then forgot to take it into account for all the sniffs, so most of the sniffs (and more so the fixers) are buggy as they don't handle the new annotations well.
• 3.3 is looking good so far.

If nothing else, having 3.3.0 as a minimum would be helpful to our end-users combined with a good upgrade guide. End-users which use a custom ruleset will need to check their ruleset anyway what with the WPCS renamed sniffs being dropped in 2.x, so if we can entice them to update the properties to the new format in one go, it will save them some work.

[jrfnl]

Also see: #1336 for info about a tokenizer change in PHPCS 3.3.0

[jrfnl]

PHPCS 3.3.0 will add a new Generic.PHP.LowerCaseType sniff (squizlabs/PHP_CodeSniffer@5a98939) which can check that type casts, parameter and return type declarations (non-class based ones) are all lowercase.

While not strictly covered in the Coding Standards handbook, I would suggest adding it to the Core ruleset, in line with the previously added Generic.PHP.LowerCaseConstant and Generic.PHP.LowerCaseKeyword sniffs which are also in that ruleset.

[jrfnl]

PHPCS 3.3.0 has added a new PSR12.Classes.ClassInstantiation sniff which partially covers what the WordPress.Classes.ClassInstantiation sniff covers. There were some bugs, but those have now been fixed and will be in the PHPCS 3.3.1 release.

When this ticket becomes active, it is worth checking if some of the checks in the WPCS sniff can be removed in favour of the upstream sniff.

Also see: squizlabs/PHP_CodeSniffer#2053

[jrfnl]

Reminder: don't forget to also review and update the phpcs.xml.dist.sample file!

[jrfnl]

Reminder: drop support for incorrectly passed array properties as per #1368
Related: squizlabs/PHP_CodeSniffer#1999 / #1048 (comment)

[jrfnl]

Another one which I'd forgotten to add to this thread before:

Anywhere where an array is used containing T_CLASS, T_ANON_CLASS, T_TRAIT, T_INTERFACE, it can be replaced with Tokens::$ooScopeTokens.

This new array was introduced in PHPCS 3.1.0 via squizlabs/PHP_CodeSniffer#1415

[GaryJones]

In terms of timelines, would this dropping of PHPCS 2.x need be done at a major release, or not?

[jrfnl]

In terms of timelines, would this dropping of PHPCS 2.x need be done at a major release, or not?

@GaryJones It depends on your point of view.

Yes:

• Removing significant things should be done in major releases and removing support for PHPCS 2.x could be considered significant in that respect.
• It's a compatibility break which could prevent people from upgrading if they combine using (sniffs from) WPCS with using other external rulesets and the other ruleset(s) is/are not (yet) compatible with PHPCS 3.x.

No:

• PHPCS could be considered as only a dependency of WPCS and something which is an implementation detail.

[GaryJones]

I saw a post recently about how Doctrine dropped support for a version of PHP (7.0?) in a non-major release, but that doesn't have the explicit tie-in that we have with PHPCS.

[jrfnl]

FYI: the Travis builds against nightly for PHPCS 2.9 and 3.1 are failing because of a PHP 7.3 issue in PHPCS itself.
A fix for this issue has been pulled & merged and will be released in PHPCS 3.3.1, so the build against master should now pass again (just tested and it does).

However, I think this issue is something to keep in mind when determining the minimum required PHPCS version for the next major release of WPCS.

[GaryJones]

Just because I looked it up:

• PHPCS 3.0.0 bumped the minimum level of PHP from 5.1.2 to 5.4.0.
• master branch (3.3.0) of PHPCS is still PHP 5.4.0.

So WPCS can jump to at least PHP 5.4.0 when we drop PHPCS 2.x.