Skip to content

Conversation

@jrfnl
Copy link
Member

@jrfnl jrfnl commented Jul 20, 2023

... with two sniffs from PHPCSExtra.

The original WPCS native sniff did two things:

  1. Check for a comma after the last item in an array (forbid it for single-line arrays, enforce it for multi-line arrays).
  2. Check the spacing around commas in arrays (no space before the comma, one space or new line after the comma with an allowance for aligned trailing comments).

The sniff will now be replaced by the following upstream sniffs:

  • NormalizedArrays.Arrays.CommaAfterLast for the comma after the last item in an array.
  • Universal.WhiteSpace.CommaSpacing for the spacing around commas (and not just in arrays).

The behaviour of the sniffs is 100% in line with the original sniff (verified by running both the old and the new sniffs over the test case file for the WPCS sniff being removed).

Additionally, the Universal.WhiteSpace.CommaSpacing sniff will now check the spacing around commas in more places, where WPCS previously did not check the comma spacing and will make sure the comma is before a trailing comment, not after.

The Universal.WhiteSpace.CommaSpacing sniff by default checks the spacing for all commas with the following exceptions:

  • A comma preceded or followed by a parenthesis, curly or square bracket will not be flagged to prevent conflicts with sniffs handling spacing around braces.
  • A comma preceded or followed by another comma, like for skipping items in a list assignment, will not be flagged.
  • A comma preceded by a non-indented heredoc/nowdoc closer. These will be flagged, but in that case, unless the php_version config directive is set to a version higher than PHP 7.3.0, a new line before will be enforced to prevent parse errors on PHP < 7.3.

This also means that the sniff will flag issues with the spacing around commas in function declarations, function calls and closure use statements, while WPCS already has other sniffs in place to handle this.

Luckily, the upstream sniff offers very modular error codes, so we can selectively silence those errors in the most common situations to prevent duplicate notices about the same issue. For those places were the Universal sniff doesn't offer modular codes, we can, in most cases, selectively silence the errors coming from other sniffs. This commit includes some of those exclusions, but we may need to add more if we discover additional message duplications.

There may, however, still be some places which could get duplicate messages. It is, however, not expected that those will ever lead to fixer conflicts.

Both upstream sniffs contain fixers for all issues.

Ref:

... with two sniffs from PHPCSExtra.

The original WPCS native sniff did two things:
1. Check for a comma after the last item in an array (forbid it for single-line arrays, enforce it for multi-line arrays).
2. Check the spacing around commas in arrays (no space before the comma, one space or new line after the comma with an allowance for aligned trailing comments).

The sniff will now be replaced by the following upstream sniffs:
* `NormalizedArrays.Arrays.CommaAfterLast` for the comma after the last item in an array.
* `Universal.WhiteSpace.CommaSpacing` for the spacing around commas (and not just in arrays).

The behaviour of the sniffs is 100% in line with the original sniff (verified by running both the old and the new sniffs over the test case file for the WPCS sniff being removed).

Additionally, the `Universal.WhiteSpace.CommaSpacing` sniff will now check the spacing around commas in more places, where WPCS previously did not check the comma spacing and will make sure the comma is before a trailing comment, not after.

The `Universal.WhiteSpace.CommaSpacing` sniff by default checks the spacing for _all_ commas with the following exceptions:
* A comma preceded or followed by a parenthesis, curly or square bracket will not be flagged to prevent conflicts with sniffs handling spacing around braces.
* A comma preceded or followed by another comma, like for skipping items in a list assignment, will not be flagged.
* A comma preceded by a non-indented heredoc/nowdoc closer.
    These will be flagged, but in that case, unless the `php_version` config directive is set to a version higher than PHP 7.3.0, a new line before will be enforced to prevent parse errors on PHP < 7.3.

This also means that the sniff will flag issues with the spacing around commas in function declarations, function calls and closure use statements, while WPCS already has other sniffs in place to handle this.

Luckily, the upstream sniff offers very modular error codes, so we can selectively silence those errors in the most common situations to prevent duplicate notices about the same issue.
For those places were the `Universal` sniff doesn't offer modular codes, we can, in most cases, selectively silence the errors coming from other sniffs. This commit includes some of those exclusions, but we may need to add more if we discover additional message duplications.

There may, however, still be some places which could get duplicate messages. It is, however, not expected that those will ever lead to fixer conflicts.

Both upstream sniffs contain fixers for all issues.

Ref:
* PHPCSStandards/PHPCSExtra 11
* PHPCSStandards/PHPCSExtra 254
Copy link
Member

@GaryJones GaryJones left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@GaryJones GaryJones merged commit 2277863 into develop Jul 20, 2023
@GaryJones GaryJones deleted the feature/core-replace-arrays-commaafterlastitem-sniff branch July 20, 2023 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants