From 174a08bfdfc7a3d11ccb1fcaaf22ab1ad10dfd79 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 17 Jul 2025 18:57:33 +0200 Subject: [PATCH 1/5] 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. --- .../Tests/Functions/StripTagsUnitTest.inc | 32 +++++++++++++++---- .../Tests/Functions/StripTagsUnitTest.php | 10 +++--- 2 files changed, 31 insertions(+), 11 deletions(-) diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc index 1e625c4e..b599439e 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc @@ -1,13 +1,33 @@ haxx0red'; -$html = '
'; +/* + * Not the sniff target. + */ +use strip_tags; +use MyNs\{ + function strip_tags, +}; + +my\ns\strip_tags($a, $b); +$this->strip_tags($a, $b); +$this?->strip_tags($a, $b); +MyClass::strip_tags($a, $b); +echo STRIP_TAGS; +namespace\strip_tags($a, $b); + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Strip_tags('text')] +function foo() {} strip_tag( 'Test', $html ); // Ok - similarly-named function. wp_strip_all_tags( $string ); // Ok. + +/* + * These should all be flagged with a warning. + */ strip_tags( 'Testing' ); // Warning. -strip_tags( 'Test', $html ); // Warning. -strip_tags( 'Test' . ', ' . 'HTML' ); // Warning - concatenation on first parameter. -strip_tags( 'Test, String', $html ); // Warning - comma in first parameter. -strip_tags( $string ); // Warning. +STRIP_TAGS( 'Test', $html ); // Warning. +\strip_tags( 'Test' . ', ' . 'HTML', ); // Warning. +strip_tags( 'Test, String', $html, ); // Warning. +Strip_Tags( $string ); // Warning. diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php index 3dfe3703..0dff19b1 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php @@ -32,11 +32,11 @@ public function getErrorList() { */ public function getWarningList() { return [ - 9 => 1, - 10 => 1, - 11 => 1, - 12 => 1, - 13 => 1, + 29 => 1, + 30 => 1, + 31 => 1, + 32 => 1, + 33 => 1, ]; } } From ece1b1ea5b5390d50c407619ad7027e23bb5ccaa Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 17 Jul 2025 19:16:25 +0200 Subject: [PATCH 2/5] 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](https://github.com/search?q=StripTagsOneParameter&type=code), so I'm maintaining BC for now. --- .../Sniffs/Functions/StripTagsSniff.php | 33 ++++++++++++++++--- .../Tests/Functions/StripTagsUnitTest.inc | 3 ++ .../Tests/Functions/StripTagsUnitTest.php | 1 + 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php index ce9fb845..4090cbfd 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php @@ -43,16 +43,41 @@ class StripTagsSniff extends AbstractFunctionParameterSniff { * in lowercase. * @param array $parameters Array with information about the parameters. * - * @return int|void Integer stack pointer to skip forward or void to continue - * normal file processing. + * @return void */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { if ( count( $parameters ) === 1 ) { - $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_strip_all_tags()` to strip all tags.'; - $this->phpcsFile->addWarning( $message, $stackPtr, 'StripTagsOneParameter' ); + $this->add_warning( $stackPtr, 'StripTagsOneParameter' ); } elseif ( isset( $parameters[2] ) ) { $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_kses()` instead to allow only the HTML you need.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'StripTagsTwoParameters' ); } } + + /** + * Process the function if no parameters were found. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * + * @return void + */ + public function process_no_parameters( $stackPtr, $group_name, $matched_content ) { + $this->add_warning( $stackPtr ); + } + + /** + * Add a warning if the function is used at all. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $error_code Error code to use for the warning. + * + * @return void + */ + private function add_warning( $stackPtr, $error_code = 'Used' ) { + $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_strip_all_tags()` to strip all tags.'; + $this->phpcsFile->addWarning( $message, $stackPtr, $error_code ); + } } diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc index b599439e..44d674a7 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc @@ -31,3 +31,6 @@ STRIP_TAGS( 'Test', $html ); // Warning. \strip_tags( 'Test' . ', ' . 'HTML', ); // Warning. strip_tags( 'Test, String', $html, ); // Warning. Strip_Tags( $string ); // Warning. + +// The function should always be flagged, even during live coding (missing required parameter). +strip_tags(); diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php index 0dff19b1..37aedc07 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php @@ -37,6 +37,7 @@ public function getWarningList() { 31 => 1, 32 => 1, 33 => 1, + 36 => 1, ]; } } From 043943d83a4fedfc2b0c4fa447422ed8641a6a02 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 17 Jul 2025 19:19:54 +0200 Subject: [PATCH 3/5] Functions/StripTags: document handling of PHP 5.6 argument unpacking This is flagged under the `StripTagsOneParameter` error code. --- WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc | 2 ++ WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php | 1 + 2 files changed, 3 insertions(+) diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc index 44d674a7..1695cf39 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc @@ -34,3 +34,5 @@ Strip_Tags( $string ); // Warning. // The function should always be flagged, even during live coding (missing required parameter). strip_tags(); + +strip_tags(...$params); // PHP 5.6 argument unpacking. diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php index 37aedc07..37672646 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php @@ -38,6 +38,7 @@ public function getWarningList() { 32 => 1, 33 => 1, 36 => 1, + 38 => 1, ]; } } From 1318d5620cf67e7df779e7c867e4065388a05df0 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 17 Jul 2025 19:30:23 +0200 Subject: [PATCH 4/5] 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. --- .../Sniffs/Functions/StripTagsSniff.php | 10 ++++++++-- .../Tests/Functions/StripTagsUnitTest.inc | 8 ++++++++ .../Tests/Functions/StripTagsUnitTest.php | 5 +++++ 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php index 4090cbfd..6be47507 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php @@ -9,6 +9,7 @@ namespace WordPressVIPMinimum\Sniffs\Functions; +use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** @@ -46,11 +47,16 @@ class StripTagsSniff extends AbstractFunctionParameterSniff { * @return void */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - if ( count( $parameters ) === 1 ) { + $string_param = PassedParameters::getParameterFromStack( $parameters, 1, 'string' ); + $allowed_tags_param = PassedParameters::getParameterFromStack( $parameters, 2, 'allowed_tags' ); + + if ( $string_param !== false && $allowed_tags_param === false ) { $this->add_warning( $stackPtr, 'StripTagsOneParameter' ); - } elseif ( isset( $parameters[2] ) ) { + } elseif ( $allowed_tags_param !== false ) { $message = '`strip_tags()` does not strip CSS and JS in between the script and style tags. Use `wp_kses()` instead to allow only the HTML you need.'; $this->phpcsFile->addWarning( $message, $stackPtr, 'StripTagsTwoParameters' ); + } else { + $this->add_warning( $stackPtr ); } } diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc index 1695cf39..ebe6e738 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc @@ -36,3 +36,11 @@ Strip_Tags( $string ); // Warning. strip_tags(); strip_tags(...$params); // PHP 5.6 argument unpacking. + +// Safeguard correct handling of function calls using PHP 8.0+ named parameters. +strip_tags(allowed_tags: $allowed, string: $html ); // Warning. +strip_tags(string: $html); // Warning. +strip_tags(allowed_tags: $allowed); // Warning. Invalid function call, but that's not the concern of this sniff. + +\strip_tags(string: $html, allowed_tag: $allowed); // Warning (mind: deliberate typo in param name). +strip_tags(html: $html); // Warning (mind: deliberately using incorrect param name). diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php index 37672646..e459f6a8 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php @@ -39,6 +39,11 @@ public function getWarningList() { 33 => 1, 36 => 1, 38 => 1, + 41 => 1, + 42 => 1, + 43 => 1, + 45 => 1, + 46 => 1, ]; } } From 914a244845a7c6e7fb3c0ddd0ee7dded93174407 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Thu, 17 Jul 2025 19:45:11 +0200 Subject: [PATCH 5/5] Functions/StripTags: add support for handling PHP 8.1 first class callables 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 https://github.com/WordPress/WordPress-Coding-Standards/pull/2544 --- .../Sniffs/Functions/StripTagsSniff.php | 14 ++++++++++++++ .../Tests/Functions/StripTagsUnitTest.inc | 2 ++ .../Tests/Functions/StripTagsUnitTest.php | 1 + 3 files changed, 17 insertions(+) diff --git a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php index 6be47507..5c1bb62b 100644 --- a/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Functions/StripTagsSniff.php @@ -74,6 +74,20 @@ public function process_no_parameters( $stackPtr, $group_name, $matched_content $this->add_warning( $stackPtr ); } + /** + * Process the function if it is used as a first class callable. + * + * @param int $stackPtr The position of the current token in the stack. + * @param string $group_name The name of the group which was matched. + * @param string $matched_content The token content (function name) which was matched + * in lowercase. + * + * @return void + */ + public function process_first_class_callable( $stackPtr, $group_name, $matched_content ) { + $this->add_warning( $stackPtr ); + } + /** * Add a warning if the function is used at all. * diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc index ebe6e738..2636026c 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.inc @@ -44,3 +44,5 @@ strip_tags(allowed_tags: $allowed); // Warning. Invalid function call, but that' \strip_tags(string: $html, allowed_tag: $allowed); // Warning (mind: deliberate typo in param name). strip_tags(html: $html); // Warning (mind: deliberately using incorrect param name). + +add_action('my_action', strip_tags(...)); // PHP 8.1 first class callable. diff --git a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php index e459f6a8..cd5ebf6c 100644 --- a/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Functions/StripTagsUnitTest.php @@ -44,6 +44,7 @@ public function getWarningList() { 43 => 1, 45 => 1, 46 => 1, + 48 => 1, ]; } }