AbstractFunctionParameterSniff: don't ignore first class callables #2544
Conversation
This file contains hidden or 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
... but pass them to a dedicated `process_first_class_callable()` method instead to allow sniffs to decide whether to flag these or not. Typical use-case for why first class callables should not be ignored by default: A sniff which ALWAYS flags the use of a certain function, but has different error messages depending on whether parameters are passed or not. In that situation, I believe first class callables should be treated the same as other uses of the target function. First class callables are basically syntax sugar for closures (example: https://3v4l.org/cra8s) and if the sniff would flag the use of the target function within a closure, it is only reasonable to also flag the use of the target function as a first class callable. This commit implements this. The commit does not include tests as there are no sniffs in WPCS for which the above would apply. However, I have manually tested this change via a sniff in an external standard for which this change is relevant.
Just to clarify - the previous PR did add tests with first class callables, so the logic in this PR is actually covered by tests. |
|
@GaryJones @dingo-d Could you please have a look at this ? |
GaryJones
approved these changes
Jul 21, 2025
dingo-d
approved these changes
Jul 21, 2025
jrfnl
deleted the
feature/abstractfunctionparams-process-first-class-callables
branch
jrfnl
added a commit
to Automattic/VIP-Coding-Standards
that referenced
this pull request
Jul 22, 2025
…lables
First class callables are basically syntax sugar for closures.
So:
```php
add_action('my_action', strip_tags(...));
```
... could also be written as:
```php
add_action('my_action', function(...$args) {
return strip_tags(...$args);
});
```
Example: https://3v4l.org/cra8s#veol
As the code would be flagged when used in a closure, it is my opinion, it should also be flagged when used as a first class callable.
This commit implements this in a WPCS cross-version compatible manner.
Prior to WPCS 3.2.0, first class callables would be send to the `process_no_parameters()` method.
As of WPCS 3.2.0, they will be send to the dedicated `process_first_class_callable()` method. See WordPress/WordPress-Coding-Standards#2544
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.
... but pass them to a dedicated
process_first_class_callable()method instead to allow sniffs to decide whether to flag these or not.Typical use-case for why first class callables should not be ignored by default:
A sniff which ALWAYS flags the use of a certain function, but has different error messages depending on whether parameters are passed or not.
In that situation, I believe first class callables should be treated the same as other uses of the target function.
First class callables are basically syntax sugar for closures (example: https://3v4l.org/cra8s) and if the sniff would flag the use of the target function within a closure, it is only reasonable to also flag the use of the target function as a first class callable.
This commit implements this.
The commit does not include tests as there are no sniffs in WPCS for which the above would apply. However, I have manually tested this change via a sniff in an external standard for which this change is relevant.
Follow up on #2518, also see: #2518 (comment)