Skip to content

Functions/StripTags: various sniff improvements #854

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Jul 23, 2025

Conversation

jrfnl
Copy link
Collaborator

@jrfnl jrfnl commented Jul 22, 2025

Functions/StripTags: improve the tests

  • Test against false positives for calls to methods or namespaced function calls.
  • Test against false positives for attribute class using the same name as the function.
  • Ensure function import use statements are not flagged. We're not interested in those.
  • Add some more variations to the pre-existing tests:
    • Non-lowercase function call(s).
    • Fully qualified function calls.
    • Use PHP 7.3+ trailing comma's in a few function calls.

Includes cleaning up some "Warning" comments which contained irrelevant information as the sniff isn't looking for the things which were previously mentioned.

Functions/StripTags: always flag use of the function

This function should never be used, so should always be flagged, no matter whether the code is (not yet) valid.

Realistically, the existing StripTagsOneParameter error code should be replaced with the new Used error code, but that would be a breaking change, which would have to wait for the next major (if deemed worth it).

I checked via a code search and the error code as-is, is referenced in various code bases, so I'm maintaining BC for now.

Functions/StripTags: document handling of PHP 5.6 argument unpacking

This is flagged under the StripTagsOneParameter error code.

Functions/StripTags: add support for handling PHP 8.0+ function calls using named parameters

This does change the logic of the sniff a little as well, as if there are parameters, but all of those would use incorrect parameter names, we should still want the function call to be flagged.

Includes tests.

Functions/StripTags: add support for handling PHP 8.1 first class callables

First class callables are basically syntax sugar for closures.
So:

add_action('my_action', strip_tags(...));

... could also be written as:

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

Closes #519

jrfnl added 5 commits July 22, 2025 16:13
* Test against false positives for calls to methods or namespaced function calls.
* Test against false positives for attribute class using the same name as the function.
* Ensure function import `use` statements are not flagged. We're not interested in those.
* Add some more variations to the pre-existing tests:
    - Non-lowercase function call(s).
    - Fully qualified function calls.
    - Use PHP 7.3+ trailing comma's in a few function calls.

Includes cleaning up some "Warning" comments which contained irrelevant information as the sniff isn't looking for the things which were previously mentioned.
This function should never be used, so should always be flagged, no matter whether the code is (not yet) valid.

Realistically, the existing `StripTagsOneParameter` error code should be replaced with the new `Used` error code, but that would be a breaking change, which would have to wait for the next major (if deemed worth it).

I checked via a code search and the error code as-is, is [referenced in various code bases](https://github.com/search?q=StripTagsOneParameter&type=code), so I'm maintaining BC for now.
This is flagged under the `StripTagsOneParameter` error code.
… using named parameters

This does change the logic of the sniff a little as well, as if there are parameters, but all of those would use incorrect parameter names, we should still want the function call to be flagged.

Includes tests.
…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
@jrfnl jrfnl added this to the 3.1.0 milestone Jul 22, 2025
@jrfnl jrfnl requested a review from a team as a code owner July 22, 2025 21:17
@jrfnl jrfnl added Type: Enhancement Type: Maintenance PHPCSUtils The addition and utilisation of PHPCSUtils package labels Jul 22, 2025
@GaryJones GaryJones merged commit fd041e4 into develop Jul 23, 2025
42 checks passed
@GaryJones GaryJones deleted the feature/functions-striptags-sniff-review branch July 23, 2025 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PHPCSUtils The addition and utilisation of PHPCSUtils package Type: Enhancement Type: Maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Review the WordPressVIPMinimum.Functions.StripTags sniff
2 participants