AbstractArrayAssignmentRestrictions: remove support for 'callback' #2291
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
[OPTIONAL]
The `getGroups()` method allows to pass a custom callback for any particular group, however, in practice that array key is never used as the `callback()` method (the default) is declared as `abstract` and therefore **must** be defined in a child class.
I propose to remove support for the `callback` array key from the abstract sniff.
Sniffs which extend this abstract can still use different logic for different groups by applying a switch to the `$group` parameter received in the `callback()`, along the lines of:
```php
public function callback( $key, $val, $line, $group ) {
switch( $group ) {
case 'groupA':
return $this->callback_for_group_A($key, val, $line);
case 'groupB':
return $this->callback_for_group_B($key, val, $line);
}
}
```
I have verified that [none of the sniffs published in public repos and which extend this abstract sniff](https://sourcegraph.com/search?q=context:global+use+WordPressCS%5CWordPress%5CAbstractArrayAssignmentRestrictionsSniff&patternType=standard&case=yes&sm=0&groupBy=group) currently use the `'callback'` key.
Removing the support for the key is a breaking change, but one with low impact based on current usage of the abstract.
dingo-d
approved these changes
Jul 4, 2023
GaryJones
deleted the
feature/abstractarrayassignmentrestrictions-remove-callback-option
branch
|
Eh.. okay...guess you all agreed with this proposal, even though it is a breaking change. 😁 |
|
The impact is low, so it's not going to cause any issues. Plus v3.0.0 is a major one, so BC breaks are allowed (and expected). |
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.
👉🏻 [OPTIONAL/PROPOSAL]
The
getGroups()method allows to pass a custom callback for any particular group, however, in practice that array key is never used as thecallback()method (the default) is declared asabstractand therefore must be defined in a child class.I propose to remove support for the
callbackarray key from the abstract sniff.Sniffs which extend this abstract can still use different logic for different groups by applying a switch to the
$groupparameter received in thecallback(), along the lines of:I have verified that none of the sniffs published in public repos and which extend this abstract sniff currently use the
'callback'key.Removing the support for the key is a breaking change, but one with low impact based on current usage of the abstract.