From 7f766304b0654cee7c1dfa8e548f65fce197e05c Mon Sep 17 00:00:00 2001
From: Rodrigo Primo <rodrigosprimo@gmail.com>
Date: Wed, 28 Aug 2024 02:05:46 -0300
Subject: [PATCH] =?UTF-8?q?=E2=9C=A8=20New=20WordPress.WP.GetMetaSingle=20?=
 =?UTF-8?q?sniff=20(#2465)?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

* ✨ New WordPress.WP.GetMetaSingle sniff
  * 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.

---------

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>
---
 WordPress-Extra/ruleset.xml                  |   5 +
 WordPress/Docs/WP/GetMetaSingleStandard.xml  |  39 ++++
 WordPress/Sniffs/WP/GetMetaSingleSniff.php   | 195 +++++++++++++++++++
 WordPress/Tests/WP/GetMetaSingleUnitTest.inc |  48 +++++
 WordPress/Tests/WP/GetMetaSingleUnitTest.php |  53 +++++
 5 files changed, 340 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 ab0e8db3b..53da8be01 100644
--- a/WordPress-Extra/ruleset.xml
+++ b/WordPress-Extra/ruleset.xml
@@ -187,6 +187,11 @@
 	<!-- Detect use of double negative `!!`. -->
 	<rule ref="Universal.CodeAnalysis.NoDoubleNegative"/>
 
+	<!-- Flags calls to get_*_meta() and get_metadata*() functions
+		 that include the $[meta_]key parameter but omit the $single parameter.
+		 https://github.com/WordPress/WordPress-Coding-Standards/issues/2459 -->
+	<rule ref="WordPress.WP.GetMetaSingle"/>
+
 	<!--
 	#############################################################################
 	Code style sniffs for more recent PHP features and syntaxes.
diff --git a/WordPress/Docs/WP/GetMetaSingleStandard.xml b/WordPress/Docs/WP/GetMetaSingleStandard.xml
new file mode 100644
index 000000000..0d1a9778e
--- /dev/null
+++ b/WordPress/Docs/WP/GetMetaSingleStandard.xml
@@ -0,0 +1,39 @@
+<?xml version="1.0"?>
+<documentation xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+    xsi:noNamespaceSchemaLocation="https://phpcsstandards.github.io/PHPCSDevTools/phpcsdocs.xsd"
+    title="Get Meta Function Single Parameter"
+    >
+    <standard>
+    <![CDATA[
+    When calling the `get_(comment|post|site|term|user)_meta()`, `get_metadata()`,
+    `get_metadata_default()`, and `get_metadata_raw()` functions, if the `$[meta_]key` parameter is
+    passed, the `$single` parameter should be passed as well to make it explicit whether the
+    functions should return a single value (the first value) or an array of values stored for the
+    meta key.
+
+    Note that these functions may still return `false` for an invalid ID. They may also return an
+    empty string if `$single` is `true` or an empty array if `$single` is `false` for a valid but
+    non-existing ID. This can be confusing as these might be valid values for a meta value.
+    ]]>
+    </standard>
+    <code_comparison>
+        <code title="Valid: Get meta functions called either without the `$[meta_]key` parameter; or with both the `$[meta_]key` and the `$single` parameters.">
+        <![CDATA[
+$admin_color = get_user_meta(
+    $user_id,
+    <em>'admin_color'</em>,
+    <em>true</em>
+);
+$post_meta = get_metadata( 'post', $post_id<em></em> );
+        ]]>
+        </code>
+        <code title="Invalid: Get meta function called with the `$[meta_]key` parameter, but without the `$single` parameter.">
+        <![CDATA[
+$admin_color = get_user_meta(
+    $user_id,
+    <em>'admin_color'</em>
+);
+        ]]>
+        </code>
+    </code_comparison>
+</documentation>
diff --git a/WordPress/Sniffs/WP/GetMetaSingleSniff.php b/WordPress/Sniffs/WP/GetMetaSingleSniff.php
new file mode 100644
index 000000000..243f27250
--- /dev/null
+++ b/WordPress/Sniffs/WP/GetMetaSingleSniff.php
@@ -0,0 +1,195 @@
+<?php
+/**
+ * WordPress Coding Standard.
+ *
+ * @package WPCS\WordPressCodingStandards
+ * @link    https://github.com/WordPress/WordPress-Coding-Standards
+ * @license https://opensource.org/licenses/MIT MIT
+ */
+
+namespace WordPressCS\WordPress\Sniffs\WP;
+
+use PHPCSUtils\Utils\PassedParameters;
+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
+ * bugs.
+ *
+ * @link https://github.com/WordPress/WordPress-Coding-Standards/issues/2459
+ *
+ * @since 3.2.0
+ */
+final class GetMetaSingleSniff extends AbstractFunctionParameterSniff {
+
+	/**
+	 * The phrase to use for the metric recorded by this sniff.
+	 *
+	 * @var string
+	 */
+	const METRIC_NAME = 'get_*meta() function called with $single parameter';
+
+	/**
+	 * The group name for this group of functions.
+	 *
+	 * @since 3.2.0
+	 *
+	 * @var string
+	 */
+	protected $group_name = 'get_meta';
+
+	/**
+	 * List of functions this sniff should examine.
+	 *
+	 * {@internal Once support for PHP < 5.6 is dropped, it is possible to create two class constants
+	 * 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/
+	 * @link https://developer.wordpress.org/reference/functions/get_metadata_raw/
+	 * @link https://developer.wordpress.org/reference/functions/get_post_meta/
+	 * @link https://developer.wordpress.org/reference/functions/get_site_meta/
+	 * @link https://developer.wordpress.org/reference/functions/get_term_meta/
+	 * @link https://developer.wordpress.org/reference/functions/get_user_meta/
+	 *
+	 * @since 3.2.0
+	 *
+	 * @var array<string, array<string, array<string, int|string>>> 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(
+			'condition'   => array(
+				'param_name' => 'key',
+				'position'   => 2,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 3,
+			),
+		),
+		'get_metadata'         => array(
+			'condition'   => array(
+				'param_name' => 'meta_key',
+				'position'   => 3,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 4,
+			),
+		),
+		'get_metadata_default' => array(
+			'condition'   => array(
+				'param_name' => 'meta_key',
+				'position'   => 3,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 4,
+			),
+		),
+		'get_metadata_raw'     => array(
+			'condition'   => array(
+				'param_name' => 'meta_key',
+				'position'   => 3,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 4,
+			),
+		),
+		'get_post_meta'        => array(
+			'condition'   => array(
+				'param_name' => 'key',
+				'position'   => 2,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 3,
+			),
+		),
+		'get_site_meta'        => array(
+			'condition'   => array(
+				'param_name' => 'key',
+				'position'   => 2,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 3,
+			),
+		),
+		'get_term_meta'        => array(
+			'condition'   => array(
+				'param_name' => 'key',
+				'position'   => 2,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 3,
+			),
+		),
+		'get_user_meta'        => array(
+			'condition'   => array(
+				'param_name' => 'key',
+				'position'   => 2,
+			),
+			'recommended' => array(
+				'param_name' => 'single',
+				'position'   => 3,
+			),
+		),
+	);
+
+	/**
+	 * Process the parameters of a matched function.
+	 *
+	 * @since 3.2.0
+	 *
+	 * @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 ) {
+		$condition   = $this->target_functions[ $matched_content ]['condition'];
+		$recommended = $this->target_functions[ $matched_content ]['recommended'];
+
+		$meta_key = PassedParameters::getParameterFromStack( $parameters, $condition['position'], $condition['param_name'] );
+		if ( ! is_array( $meta_key ) ) {
+			return;
+		}
+
+		$single = PassedParameters::getParameterFromStack( $parameters, $recommended['position'], $recommended['param_name'] );
+		if ( is_array( $single ) ) {
+			$this->phpcsFile->recordMetric( $stackPtr, self::METRIC_NAME, 'yes' );
+			return;
+		}
+
+		$this->phpcsFile->recordMetric( $stackPtr, self::METRIC_NAME, 'no' );
+
+		$tokens       = $this->phpcsFile->getTokens();
+		$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 also pass the $%s parameter to explicitly indicate whether a single value or multiple values are expected to be returned.',
+			$stackPtr,
+			'Missing',
+			$message_data
+		);
+	}
+}
diff --git a/WordPress/Tests/WP/GetMetaSingleUnitTest.inc b/WordPress/Tests/WP/GetMetaSingleUnitTest.inc
new file mode 100644
index 000000000..54ea2fecc
--- /dev/null
+++ b/WordPress/Tests/WP/GetMetaSingleUnitTest.inc
@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * Not the sniff target.
+ */
+my\ns\get_post_meta($a, $b);
+$this->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 );
+$ok = get_post_meta( $post_id, single: true, key: $meta_key );
+$ok = get_metadata( 'post', $post_id );
+$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 );
+
+// 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.
+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 );
+$warning = GET_METADATA( 'post', $post_id, $meta_key );
+$warning = get_metadata(
+    'post',
+    $post_id,
+    meta_key: $meta_key
+);
+$warning = get_metadata_raw( 'post', $post_id, $meta_key );
+$warning = get_metadata_default( 'post', $post_id, $meta_key );
diff --git a/WordPress/Tests/WP/GetMetaSingleUnitTest.php b/WordPress/Tests/WP/GetMetaSingleUnitTest.php
new file mode 100644
index 000000000..f1b328c39
--- /dev/null
+++ b/WordPress/Tests/WP/GetMetaSingleUnitTest.php
@@ -0,0 +1,53 @@
+<?php
+/**
+ * Unit test class for WordPress Coding Standard.
+ *
+ * @package WPCS\WordPressCodingStandards
+ * @link    https://github.com/WordPress/WordPress-Coding-Standards
+ * @license https://opensource.org/licenses/MIT MIT
+ */
+
+namespace WordPressCS\WordPress\Tests\WP;
+
+use PHP_CodeSniffer\Tests\Standards\AbstractSniffUnitTest;
+
+/**
+ * Unit test class for the GetMetaSingle sniff.
+ *
+ * @since 3.2.0
+ *
+ * @covers \WordPressCS\WordPress\Sniffs\WP\GetMetaSingleSniff
+ */
+final class GetMetaSingleUnitTest extends AbstractSniffUnitTest {
+
+	/**
+	 * Returns the lines where errors should occur.
+	 *
+	 * @return array<int, int> Key is the line number, value is the number of expected errors.
+	 */
+	public function getErrorList() {
+		return array();
+	}
+
+	/**
+	 * Returns the lines where warnings should occur.
+	 *
+	 * @return array<int, int> Key is the line number, value is the number of expected warnings.
+	 */
+	public function getWarningList() {
+		return array(
+			33 => 1,
+			34 => 1,
+			35 => 1,
+			36 => 1,
+			37 => 1,
+			38 => 1,
+			39 => 1,
+			40 => 1,
+			41 => 1,
+			42 => 1,
+			47 => 1,
+			48 => 1,
+		);
+	}
+}