From 4b47564bfe1cd85f6c570777c6148549b5950242 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Mon, 24 Jun 2024 11:43:56 -0300 Subject: [PATCH 01/11] =?UTF-8?q?=E2=9C=A8=20New=20WordPress.WP.GetMetaSin?= =?UTF-8?q?gle=20sniff?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This sniff warns when get_*_meta() and get_metadata*() functions are used with the $meta_key/$key param, but without the $single parameter. This could lead to unexpected behavior as an array will be returned, but a string might be expected. --- WordPress-Extra/ruleset.xml | 5 + WordPress/Docs/WP/GetMetaSingleStandard.xml | 33 ++++ WordPress/Sniffs/WP/GetMetaSingleSniff.php | 172 +++++++++++++++++++ WordPress/Tests/WP/GetMetaSingleUnitTest.inc | 32 ++++ WordPress/Tests/WP/GetMetaSingleUnitTest.php | 52 ++++++ 5 files changed, 294 insertions(+) create mode 100644 WordPress/Docs/WP/GetMetaSingleStandard.xml create mode 100644 WordPress/Sniffs/WP/GetMetaSingleSniff.php create mode 100644 WordPress/Tests/WP/GetMetaSingleUnitTest.inc create mode 100644 WordPress/Tests/WP/GetMetaSingleUnitTest.php diff --git a/WordPress-Extra/ruleset.xml b/WordPress-Extra/ruleset.xml index ab0e8db3b7..a1d36cc955 100644 --- a/WordPress-Extra/ruleset.xml +++ b/WordPress-Extra/ruleset.xml @@ -187,6 +187,11 @@ + + + - + diff --git a/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php b/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php index e50990e52b..ea78d46cf1 100644 --- a/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php +++ b/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php @@ -36,6 +36,10 @@ final class GetMetaFunctionSingleParameterSniff extends AbstractFunctionParamete /** * List of functions this sniff should examine. * + * Once support for PHP 5.4 is dropped, it is possible to create two private properties + * representing the two different signatures of get meta functions to remove the duplication + * of the name and position of the parameters. + * * @link https://developer.wordpress.org/reference/functions/get_comment_meta/ * @link https://developer.wordpress.org/reference/functions/get_metadata/ * @link https://developer.wordpress.org/reference/functions/get_metadata_default/ @@ -47,8 +51,10 @@ final class GetMetaFunctionSingleParameterSniff extends AbstractFunctionParamete * * @since 3.2.0 * - * @var array Key is function name, value is an array containing information - * about the name and position of the relevant parameters. + * @var array>> Key is function name, value is + * an array containing information + * about the name and position of + * the relevant parameters. */ protected $target_functions = array( 'get_comment_meta' => array( @@ -160,13 +166,14 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p return; } - $tokens = $this->phpcsFile->getTokens(); + $tokens = $this->phpcsFile->getTokens(); + $message_data = array( $condition['parameter'], $tokens[ $stackPtr ]['content'], $recommended['parameter'] ); $this->phpcsFile->addWarning( - 'When passing the $%s parameter to %s(), the $%s parameter must also be passed to make it explicit whether an array or a string is expected.', + 'When passing the $%s parameter to %s(), it is recommended to pass the $%s parameter as well to make it explicit whether an array or a string is expected.', $stackPtr, 'ReturnTypeNotExplicit', - array( $condition['parameter'], $tokens[ $stackPtr ]['content'], $recommended['parameter'] ) + $message_data ); } } diff --git a/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.inc b/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.inc index 80d47d4b02..54ea2fecc0 100644 --- a/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.inc +++ b/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.inc @@ -1,11 +1,17 @@ get_post_meta($a, $b); +MyClass::get_post_meta($a, $b); +echo GET_POST_META; +add_action('my_action', get_post_meta(...)); // PHP 8.1 first class callable. +/* + * These should all be okay. + */ $ok = get_post_meta( $post_id ); $ok = get_post_meta( $post_id, $meta_key, false ); $ok = get_post_meta( $post_id, single: true ); @@ -15,10 +21,20 @@ $ok = get_metadata( 'post', $post_id, $meta_key, true ); $ok = get_metadata( 'post', $post_id, single: true ); $ok = get_metadata( 'post', $post_id, single: true, meta_key: $meta_key ); -$warning = get_post_meta( $post_id, $meta_key ); -$warning = get_post_meta( $post_id, key: $meta_key ); +// Incorrect function calls, should be ignored by the sniff. +$incorrect_but_ok = get_post_meta(); +$incorrect_but_ok = get_post_meta( single: true, $post_id ); +$incorrect_but_ok = get_post_meta( single: true ); +$incorrect_but_ok = get_metadata( 'post' ); + +/* + * These should all be flagged with a warning. + */ +$warning = \get_post_meta( $post_id, $meta_key ); +implode(', ', get_post_meta( $post_id, $meta_key )); +if (get_post_meta( $post_id, key: $meta_key )) {} $warning = get_post_meta( $post_id, key: $meta_key, sinngle: true ); // Typo in parameter name. -$warning = get_comment_meta( $comment_id, $meta_key ); +echo get_comment_meta( $comment_id, $meta_key ); $warning = get_site_meta( $site_id, $meta_key ); $warning = get_term_meta( $term_id, $meta_key ); $warning = get_user_meta( $user_id, $meta_key ); diff --git a/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.php b/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.php index 98e415a81e..b1582c6a5e 100644 --- a/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.php +++ b/WordPress/Tests/WP/GetMetaFunctionSingleParameterUnitTest.php @@ -36,17 +36,18 @@ public function getErrorList() { */ public function getWarningList() { return array( - 18 => 1, - 19 => 1, - 20 => 1, - 21 => 1, - 22 => 1, - 23 => 1, - 24 => 1, - 25 => 1, - 26 => 1, - 31 => 1, - 32 => 1, + 33 => 1, + 34 => 1, + 35 => 1, + 36 => 1, + 37 => 1, + 38 => 1, + 39 => 1, + 40 => 1, + 41 => 1, + 42 => 1, + 47 => 1, + 48 => 1, ); } } From 168e7463c09deb02f27deeba82d724fd8a91498d Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Wed, 21 Aug 2024 15:47:37 -0300 Subject: [PATCH 05/11] Apply suggestions from code review Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com> --- .../Docs/WP/GetMetaFunctionSingleParameterStandard.xml | 7 +++---- .../Sniffs/WP/GetMetaFunctionSingleParameterSniff.php | 6 +++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/WordPress/Docs/WP/GetMetaFunctionSingleParameterStandard.xml b/WordPress/Docs/WP/GetMetaFunctionSingleParameterStandard.xml index 38d40ae8c0..123fa70a8e 100644 --- a/WordPress/Docs/WP/GetMetaFunctionSingleParameterStandard.xml +++ b/WordPress/Docs/WP/GetMetaFunctionSingleParameterStandard.xml @@ -7,12 +7,11 @@ - + ); ]]> - + phpcsFile->addWarning( 'When passing the $%s parameter to %s(), it is recommended to pass the $%s parameter as well to make it explicit whether an array or a string is expected.', $stackPtr, - 'ReturnTypeNotExplicit', + 'Missing', $message_data ); } From 980648708d9ac9cbe07f17bc75ce7a6daac550df Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 22 Aug 2024 11:34:35 -0300 Subject: [PATCH 06/11] Apply more suggestions from code review --- .../GetMetaFunctionSingleParameterSniff.php | 78 ++++++++++--------- 1 file changed, 42 insertions(+), 36 deletions(-) diff --git a/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php b/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php index e2d1d0f073..30efe9a79b 100644 --- a/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php +++ b/WordPress/Sniffs/WP/GetMetaFunctionSingleParameterSniff.php @@ -13,6 +13,8 @@ use WordPressCS\WordPress\AbstractFunctionParameterSniff; /** + * Warns when calls to get meta functions use the $[meta_]key param without the $single param. + * * Flags calls to get_(comment|post|site|term|user)_meta(), get_metadata(), get_metadata_default() * and get_metadata_raw() functions that include the $key/$meta_key parameter, but omit the $single * parameter. Omitting $single in this situation can result in unexpected return types and lead to @@ -59,82 +61,82 @@ final class GetMetaFunctionSingleParameterSniff extends AbstractFunctionParamete protected $target_functions = array( 'get_comment_meta' => array( 'condition' => array( - 'parameter' => 'key', - 'position' => 2, + 'param_name' => 'key', + 'position' => 2, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 3, + 'param_name' => 'single', + 'position' => 3, ), ), 'get_metadata' => array( 'condition' => array( - 'parameter' => 'meta_key', - 'position' => 3, + 'param_name' => 'meta_key', + 'position' => 3, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 4, + 'param_name' => 'single', + 'position' => 4, ), ), 'get_metadata_default' => array( 'condition' => array( - 'parameter' => 'meta_key', - 'position' => 3, + 'param_name' => 'meta_key', + 'position' => 3, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 4, + 'param_name' => 'single', + 'position' => 4, ), ), 'get_metadata_raw' => array( 'condition' => array( - 'parameter' => 'meta_key', - 'position' => 3, + 'param_name' => 'meta_key', + 'position' => 3, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 4, + 'param_name' => 'single', + 'position' => 4, ), ), 'get_post_meta' => array( 'condition' => array( - 'parameter' => 'key', - 'position' => 2, + 'param_name' => 'key', + 'position' => 2, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 3, + 'param_name' => 'single', + 'position' => 3, ), ), 'get_site_meta' => array( 'condition' => array( - 'parameter' => 'key', - 'position' => 2, + 'param_name' => 'key', + 'position' => 2, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 3, + 'param_name' => 'single', + 'position' => 3, ), ), 'get_term_meta' => array( 'condition' => array( - 'parameter' => 'key', - 'position' => 2, + 'param_name' => 'key', + 'position' => 2, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 3, + 'param_name' => 'single', + 'position' => 3, ), ), 'get_user_meta' => array( 'condition' => array( - 'parameter' => 'key', - 'position' => 2, + 'param_name' => 'key', + 'position' => 2, ), 'recommended' => array( - 'parameter' => 'single', - 'position' => 3, + 'param_name' => 'single', + 'position' => 3, ), ), ); @@ -156,21 +158,25 @@ public function process_parameters( $stackPtr, $group_name, $matched_content, $p $condition = $this->target_functions[ $matched_content ]['condition']; $recommended = $this->target_functions[ $matched_content ]['recommended']; - $meta_key = PassedParameters::getParameterFromStack( $parameters, $condition['position'], $condition['parameter'] ); + $meta_key = PassedParameters::getParameterFromStack( $parameters, $condition['position'], $condition['param_name'] ); if ( false === $meta_key ) { return; } - $single = PassedParameters::getParameterFromStack( $parameters, $recommended['position'], $recommended['parameter'] ); + $single = PassedParameters::getParameterFromStack( $parameters, $recommended['position'], $recommended['param_name'] ); if ( is_array( $single ) ) { return; } $tokens = $this->phpcsFile->getTokens(); - $message_data = array( $condition['parameter'], $tokens[ $stackPtr ]['content'], $recommended['parameter'] ); + $message_data = array( + $condition['param_name'], + $tokens[ $stackPtr ]['content'], + $recommended['param_name'], + ); $this->phpcsFile->addWarning( - 'When passing the $%s parameter to %s(), it is recommended to pass the $%s parameter as well to make it explicit whether an array or a string is expected.', + 'When passing the $%s parameter to %s(), it is recommended to also pass the $%s parameter to explicitly indicate whether a single value or multiple values are expected to be returned.', $stackPtr, 'Missing', $message_data From 20a3e3b36db4d92eaaed70daa06260c3364566e3 Mon Sep 17 00:00:00 2001 From: Rodrigo Primo Date: Thu, 22 Aug 2024 11:42:45 -0300 Subject: [PATCH 07/11] Rename sniff back to GetMetaSingle --- WordPress-Extra/ruleset.xml | 2 +- .../Docs/WP/GetMetaFunctionSingleParameterStandard.xml | 2 ++ ...ctionSingleParameterSniff.php => GetMetaSingleSniff.php} | 2 +- ...ingleParameterUnitTest.inc => GetMetaSingleUnitTest.inc} | 0 ...ingleParameterUnitTest.php => GetMetaSingleUnitTest.php} | 6 +++--- 5 files changed, 7 insertions(+), 5 deletions(-) rename WordPress/Sniffs/WP/{GetMetaFunctionSingleParameterSniff.php => GetMetaSingleSniff.php} (98%) rename WordPress/Tests/WP/{GetMetaFunctionSingleParameterUnitTest.inc => GetMetaSingleUnitTest.inc} (100%) rename WordPress/Tests/WP/{GetMetaFunctionSingleParameterUnitTest.php => GetMetaSingleUnitTest.php} (80%) diff --git a/WordPress-Extra/ruleset.xml b/WordPress-Extra/ruleset.xml index dcee9a5b4d..0401b9f459 100644 --- a/WordPress-Extra/ruleset.xml +++ b/WordPress-Extra/ruleset.xml @@ -190,7 +190,7 @@ - + + that include the $[meta_]key parameter but omit the $single parameter. + https://github.com/WordPress/WordPress-Coding-Standards/issues/2459 -->