Arrays/ArrayKeySpacingRestrictions: implement PHPCSUtils and various bug fixes and other improvements #2075
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Most sniffs will silently bow out on parse errors. This is the preferred behaviour as sniffs are often run in IDEs during live coding. The `ArrayKeySpacingRestrictions` sniff would, however, throw a warning when a close bracket was missing. This commit removes that warning. This is in line with similar changes (no longer throwing warnings/errors on parse errors) which will be included in PHPCS 4.x upstream. Note: there were no tests covering the warning, so the test files are untouched. Refs: * squizlabs/PHP_CodeSniffer 2455
…array assignments without key As it was, this was handled correctly, though the error message was unclear as it referred to "array keys", while these assignments don't have an explicit key. To fix this, I'm now special casing array assignments without an explicit key. The original error message in that situation was: "Array keys must NOT be surrounded by spaces if they only contain a string or an integer." The new error message will be: "There should be no space between the square brackets for an array assignment without an explicit key. Found: ..." The new error message will also have a different, new error code: `SpacesBetweenBrackets`. Note: the new error code could be considered a breaking change, though I doubt many people will notice this as this is not typically an error code which would be used in inline ignore annotations. Includes unit tests. Note: the behaviour for array assignments without an explicit key, but with a comment between the brackets, has not been changed. In that case, the original `NoSpacesAroundArrayKeys` error will still be thrown. All the same, tests have been added to verify and safeguard this.
... to more easily allow for additional tests for integer keys to be added.
... to make them more descriptive.
…alculations and allow for + sign
This commit fixes two bugs:
1. Signed integers with a plus sign would be treated differently from signed integers with a minus sign.
While using the plus sign for unary integers will be rare, it is valid and they should be treated the same as unary integers with a minus sign.
2. When an array key consisted of a calculation with multiple integers and a minus sign, no spaces would be demanded around the array key, even though they key should be surrounded by spaces.
Both bugs have now been fixed by walking the tokens in the array key more precisely.
Includes unit tests covering both situations.
Includes minor tweaks to some conditions as the `$needs_spaces` variables will now always be boolean, so we can make the conditions in which the variable is being used more readable.
.. to safeguard these are handled the same as decimal integers.
Simplify the fixer code for the "Array keys must [NOT] be surrounded by spaces" errors.
This implements use of the PHPCSUtils `SpacesFixer` class for checking and fixing the width of the whitespace inside the brackets. While this doesn't change the effective behaviour of the sniff, it simplifies the code and improves the clarity of the error messages. Note: the error _codes_ do not change. Old error messages: ``` 33 | ERROR | [x] There should be exactly one space before the array key. Found: 5 33 | ERROR | [x] There should be exactly one space after the array key. Found: 4 34 | ERROR | [x] There should be exactly one space before the array key. Found: newline 35 | ERROR | [x] There should be exactly one space after the array key. Found: newline ``` New error messages: ``` 33 | ERROR | [x] There should be exactly 1 space before the array key. Found: 5 spaces 33 | ERROR | [x] There should be exactly 1 space after the array key. Found: 4 spaces 34 | ERROR | [x] There should be exactly 1 space before the array key. Found: a new line 35 | ERROR | [x] There should be exactly 1 space after the array key. Found: multiple new lines ```
…literals with underscores .. to safeguard these are handled correctly.
…meric literals .. to safeguard these are handled correctly.
19 tasks
dingo-d
approved these changes
Aug 12, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR is the result of a complete review of the
WordPress.Arrays.ArrayKeySpacingRestrictionssniff.I've basically gone through the complete sniff with a fine toothcomb with the following questions in the back of my mind:
👉🏻 This PR will be easiest to review by looking at each commit individually.
Related to #1877
Commit details
Arrays/ArrayKeySpacingRestrictions: don't warn on parse error
Most sniffs will silently bow out on parse errors. This is the preferred behaviour as sniffs are often run in IDEs during live coding.
The
ArrayKeySpacingRestrictionssniff would, however, throw a warning when a close bracket was missing.This commit removes that warning.
This is in line with similar changes (no longer throwing warnings/errors on parse errors) which will be included in PHPCS 4.x upstream.
Note: there were no tests covering the warning, so the test files are untouched.
Refs:
Arrays/ArrayKeySpacingRestrictions: improve handling of brackets for array assignments without key
As it was, this was handled correctly, though the error message was unclear as it referred to "array keys", while these assignments don't have an explicit key.
To fix this, I'm now special casing array assignments without an explicit key.
The original error message in that situation was: "Array keys must NOT be surrounded by spaces if they only contain a string or an integer."
The new error message will be: "There should be no space between the square brackets for an array assignment without an explicit key. Found: ..."
The new error message will also have a different, new error code:
SpacesBetweenBrackets.Note: the new error code could be considered a breaking change, though I doubt many people will notice this as this is not typically an error code which would be used in inline ignore annotations.
Includes unit tests.
Note: the behaviour for array assignments without an explicit key, but with a comment between the brackets, has not been changed. In that case, the original
NoSpacesAroundArrayKeyserror will still be thrown.All the same, tests have been added to verify and safeguard this.
Arrays/ArrayKeySpacingRestrictions: tweak test order
... to more easily allow for additional tests for integer keys to be added.
Arrays/ArrayKeySpacingRestrictions: rename three local variables
... to make them more descriptive.
Arrays/ArrayKeySpacingRestrictions: bug fix - require spaces around calculations and allow for + sign
This commit fixes two bugs:
While using the plus sign for unary integers will be rare, it is valid and they should be treated the same as unary integers with a minus sign.
Both bugs have now been fixed by walking the tokens in the array key more precisely.
Includes unit tests covering both situations.
Includes minor tweaks to some conditions as the
$needs_spacesvariables will now always be boolean, so we can make the conditions in which the variable is being used more readable.Arrays/ArrayKeySpacingRestrictions: add tests with non-decimal integers
.. to safeguard these are handled the same as decimal integers.
Arrays/ArrayKeySpacingRestrictions: minor simplifications
Simplify the fixer code for the "Array keys must [NOT] be surrounded by spaces" errors.
Arrays/ArrayKeySpacingRestrictions: implement the PHPCSUtils SpacesFixer
This implements use of the PHPCSUtils
SpacesFixerclass for checking and fixing the width of the whitespace inside the brackets.While this doesn't change the effective behaviour of the sniff, it simplifies the code and improves the clarity of the error messages.
Note: the error codes do not change.
Old error messages:
New error messages:
PHP 7.4 | Arrays/ArrayKeySpacingRestrictions: add tests with numeric literals with underscores
.. to safeguard these are handled correctly.
PHP 8.1 | Arrays/ArrayKeySpacingRestrictions: add tests with octal numeric literals
.. to safeguard these are handled correctly.