From 16c798ec6317448445a1cfa4c79f469e3a77dacc Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 30 Jul 2025 05:49:29 +0200 Subject: [PATCH 1/3] Performance/TaxonomyMetaInOptions: extend AbstractFunctionParameterSniff As things were, the determination of whether or not a `T_STRING` is a call to any of the global WP options functions, was severely flawed. By switching the sniff over to be based on the WordPressCS `AbstractFunctionParameterSniff` class, this flaw is mitigated. Includes adding a slew of additional tests, some of which (line 8 - 13, line 50, 51) are specific to the flaw being addressed in this commit. Additionally, the tests have been made more comprehensive and varied by: * Testing against false positives for calls to methods or namespaced function calls (= the issue being addressed in this PR). * Ensure function import `use` statements are not flagged. We're not interested in those. * Safeguarding that function calls using PHP 5.6+ argument unpacking are not flagged. * Safeguarding handling of PHP 8.0+ attribute class using the same name as a target function. * Safeguarding that the functions are not flagged when used as a PHP 8.1+ first class callable. * Adding tests with more variations: - Non-lowercase function call(s). - Fully qualified function calls. - Use PHP 7.3+ trailing comma's in a few function calls. - Comments in unexpected places. --- .../TaxonomyMetaInOptionsSniff.php | 42 ++++++++----- .../TaxonomyMetaInOptionsUnitTest.inc | 59 +++++++++++++++++-- .../TaxonomyMetaInOptionsUnitTest.php | 12 ++-- 3 files changed, 86 insertions(+), 27 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php index 4541d60a..c2adf1ac 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php @@ -10,16 +10,18 @@ namespace WordPressVIPMinimum\Sniffs\Performance; use PHP_CodeSniffer\Util\Tokens; -use WordPressVIPMinimum\Sniffs\Sniff; +use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** * Restricts the implementation of taxonomy term meta via options. */ -class TaxonomyMetaInOptionsSniff extends Sniff { +class TaxonomyMetaInOptionsSniff extends AbstractFunctionParameterSniff { /** * List of options_ functions * + * @deprecated 3.1.0 This property should never have been public. + * * @var array */ public $option_functions = [ @@ -45,27 +47,37 @@ class TaxonomyMetaInOptionsSniff extends Sniff { ]; /** - * Returns an array of tokens this test wants to listen for. + * The group name for this group of functions. * - * @return array + * @var string */ - public function register() { - return [ T_STRING ]; - } + protected $group_name = 'option_functions'; /** - * Process this test when one of its tokens is encountered - * - * @param int $stackPtr The position of the current token in the stack passed in $tokens. + * Functions this sniff is looking for. * - * @return void + * @var array Keys are the target functions, value irrelevant. */ - public function process_token( $stackPtr ) { + protected $target_functions = [ + 'get_option' => true, + 'add_option' => true, + 'update_option' => true, + 'delete_option' => true, + ]; - if ( in_array( $this->tokens[ $stackPtr ]['content'], $this->option_functions, true ) === false ) { - return; - } + /** + * Process the parameters of a matched function. + * + * @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. + * @param array $parameters Array with information about the parameters. + * + * @return void + */ + public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) { diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc index 6e67c82b..a3beb493 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc @@ -1,8 +1,55 @@ term_id" ); // NOK. -delete_option( "taxonomy_rating_{$obj->term_id}" ); // NOK. -get_option( "taxonomy_rating_$tag_id" ); // NOK. -update_option( 'taxonomy_rating_' . $category_id ); // NOK. -get_option( "taxonomy_rating_{$cat_id}" ); // NOK. -add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK. \ No newline at end of file +/* + * Not the sniff target. + */ +use function add_option; + +my\ns\get_option( "taxonomy_rating_$tag_id" ); +$this->delete_option( "taxonomy_rating_$tag_id" ); +$this?->update_option( "taxonomy_rating_$tag_id" ); +MyClass::get_option( "taxonomy_rating_$tag_id", ); +echo DELETE_OPTION; +namespace\add_option( "taxonomy_rating_$tag_id" ); + +function &update_option() {} + +/* + * These should all be okay. + */ +// Incomplete function call, should be ignored by the sniff. +$incorrect_but_ok = get_option(); +$incorrect_but_ok = update_option(); + +// Ignore as undetermined. +Get_Option( $option ); +\add_option( $obj->get_optionname() ); +update_option( OPTION_NAME, ); +\DELETE_OPTION( MyClass::$option_name ); + +get_option(...$params); // PHP 5.6 argument unpacking, parameter value unknown. + +// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute. +#[Get_Option('taxonomy_rating_')] +function foo() {} + +array_walk($options, \update_option(...),); // PHP 8.1 first class callable. + +// Ignore as not touching taxonomy term patterns. +\get_option( 'option_name' ); +add_option( 'option_' . $post_id, ); +update_Option( "prefix_{$setting}" ); +add_option( 'option_' . MY_CONSTANT, ); +add_option( 'foo_' . $obj->$propname ); +update_option( 'foo_' . $obj->page_id ); // NOK. + + +/* + * These should all be flagged. + */ +get_option( "taxonomy_rating_$obj->term_id", ); // NOK. +\delete_option( "taxonomy_rating_{$obj->term_id}" ); // NOK. +get_option( /*comment*/ "taxonomy_rating_$tag_id" ); // NOK. +UpDate_OPTION( 'taxonomy_rating_' . $category_id /*comment*/ , ); // NOK. +\Get_Option( "taxonomy_rating_{$cat_id}" ); // NOK. +add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK. diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php index dff27d68..1fa15375 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php @@ -32,12 +32,12 @@ public function getErrorList() { */ public function getWarningList() { return [ - 3 => 1, - 4 => 1, - 5 => 1, - 6 => 1, - 7 => 1, - 8 => 1, + 50 => 1, + 51 => 1, + 52 => 1, + 53 => 1, + 54 => 1, + 55 => 1, ]; } } From 1f24aadb0c68fa0c59cd9a2240d9a7e8adb186dd Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 30 Jul 2025 05:57:19 +0200 Subject: [PATCH 2/3] Performance/TaxonomyMetaInOptions: add support for handling PHP 8.0+ function calls using named parameters Includes tests. --- .../Performance/TaxonomyMetaInOptionsSniff.php | 12 ++++++++---- .../Performance/TaxonomyMetaInOptionsUnitTest.inc | 8 ++++++++ .../Performance/TaxonomyMetaInOptionsUnitTest.php | 1 + 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php index c2adf1ac..8c451dd0 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php @@ -10,6 +10,7 @@ namespace WordPressVIPMinimum\Sniffs\Performance; use PHP_CodeSniffer\Util\Tokens; +use PHPCSUtils\Utils\PassedParameters; use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** @@ -78,13 +79,16 @@ class TaxonomyMetaInOptionsSniff extends AbstractFunctionParameterSniff { * @return void */ public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) { - $openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true ); - - if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) { + $target_param = PassedParameters::getParameterFromStack( $parameters, 1, 'option' ); + if ( $target_param === false ) { + // Missing (required) target parameter. Probably live coding, nothing to examine (yet). Bow out. return; } - $param_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $openBracket + 1, null, true ); + $param_start = $target_param['start']; + $param_end = ( $target_param['end'] + 1 ); // Add one to include the last token in the parameter in findNext searches. + + $param_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_start, $param_end, true ); if ( $this->tokens[ $param_ptr ]['code'] === T_DOUBLE_QUOTED_STRING ) { foreach ( $this->taxonomy_term_patterns as $taxonomy_term_pattern ) { diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc index a3beb493..a6868479 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc @@ -53,3 +53,11 @@ get_option( /*comment*/ "taxonomy_rating_$tag_id" ); // NOK. UpDate_OPTION( 'taxonomy_rating_' . $category_id /*comment*/ , ); // NOK. \Get_Option( "taxonomy_rating_{$cat_id}" ); // NOK. add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK. + + +// Safeguard handling of function calls using PHP 8.0+ named parameters. +get_option(default_value: false); // OK, well, not really, missing required $option param, but that's not the concern of this sniff. +add_option(value: "$tag_id", option: 'option_name'); // OK. +update_option(autoload: true, optin: "$tag_id", value: 10,); // OK, well, not really, typo in param name, but that's not the concern of the sniff. + +add_option(value: $value, option: "$tag_id" ); // Bad. diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php index 1fa15375..ed4fc304 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php @@ -38,6 +38,7 @@ public function getWarningList() { 53 => 1, 54 => 1, 55 => 1, + 63 => 1, ]; } } From ae8557a81e828ef140d486ff6281c7c83611186c Mon Sep 17 00:00:00 2001 From: jrfnl Date: Wed, 30 Jul 2025 06:15:26 +0200 Subject: [PATCH 3/3] Performance/TaxonomyMetaInOptions: add support for handling PHP 8.0+ nullsafe object operator Includes test. --- .../Sniffs/Performance/TaxonomyMetaInOptionsSniff.php | 4 +++- .../Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc | 3 +++ .../Tests/Performance/TaxonomyMetaInOptionsUnitTest.php | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php index 8c451dd0..b4a10f33 100644 --- a/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php +++ b/WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php @@ -117,7 +117,9 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p } $object_operator = $this->phpcsFile->findNext( Tokens::$emptyTokens, $variable_name + 1, null, true ); - if ( $this->tokens[ $object_operator ]['code'] !== T_OBJECT_OPERATOR ) { + if ( $this->tokens[ $object_operator ]['code'] !== T_OBJECT_OPERATOR + && $this->tokens[ $object_operator ]['code'] !== T_NULLSAFE_OBJECT_OPERATOR + ) { return; } diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc index a6868479..a89e6ec0 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.inc @@ -61,3 +61,6 @@ add_option(value: "$tag_id", option: 'option_name'); // OK. update_option(autoload: true, optin: "$tag_id", value: 10,); // OK, well, not really, typo in param name, but that's not the concern of the sniff. add_option(value: $value, option: "$tag_id" ); // Bad. + +// Safeguard handling of PHP 8.0+ nullsafe object operator. +get_option( 'taxonomy_rating_' . $obj?->term_id ); // NOK. diff --git a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php index ed4fc304..cc57df21 100644 --- a/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php +++ b/WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php @@ -39,6 +39,7 @@ public function getWarningList() { 54 => 1, 55 => 1, 63 => 1, + 66 => 1, ]; } }