PSR12/OperatorSpacing: hot fix #2697
Merged
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
PR squizlabs#2640 made it so the `Squiz.WhiteSpace.OperatorSpacing` sniff now sets a property with the `$nonOperandTokens` from the `register()` method. However, the PSR12 sniff for the same extends the Squiz sniff, but overloads the `register()` method to set different targets. This means that the `$nonOperandTokens` array was empty and therefore the `isOperator()` method would nearly always return `true`, which was often incorrect. Fixed by calling the `parent::register()` method before setting the `$targets` for the PSR12 sniff. Includes adding unit tests to the PSR12 sniff related to the exclusions handled via the `isOperator()` method, to prevent breakage like this from being able to be merged in the future.
gsherwood
added a commit
that referenced
this pull request
Nov 14, 2019
|
Thanks so much for finding and fixing this before it was released. |
A failing external standards build kind of helps with that ;-) |
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.
PR #2640 made it so the
Squiz.WhiteSpace.OperatorSpacingsniff now sets a property with the$nonOperandTokensfrom theregister()method.However, the PSR12 sniff for the same extends the Squiz sniff, but overloads the
register()method to set different targets.This means that the
$nonOperandTokensarray was empty and therefore theisOperator()method would nearly always returntrue, which was often incorrect.Fixed by calling the
parent::register()method before setting the$targetsfor the PSR12 sniff.Includes adding unit tests to the PSR12 sniff related to the exclusions handled via the
isOperator()method, to prevent breakage like this from being able to be merged in the future.Fixes #2696