Skip to content

Commit

Permalink
✨ New WordPress.WP.GetMetaSingle sniff (#2465)
Browse files Browse the repository at this point in the history
* ✨ 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 <[email protected]>
  • Loading branch information
rodrigoprimo and jrfnl authored Aug 28, 2024
1 parent 0d24267 commit 7f76630
Show file tree
Hide file tree
Showing 5 changed files with 340 additions and 0 deletions.
5 changes: 5 additions & 0 deletions WordPress-Extra/ruleset.xml
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
39 changes: 39 additions & 0 deletions WordPress/Docs/WP/GetMetaSingleStandard.xml
Original file line number Diff line number Diff line change
@@ -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>
195 changes: 195 additions & 0 deletions WordPress/Sniffs/WP/GetMetaSingleSniff.php
Original file line number Diff line number Diff line change
@@ -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
);
}
}
48 changes: 48 additions & 0 deletions WordPress/Tests/WP/GetMetaSingleUnitTest.inc
Original file line number Diff line number Diff line change
@@ -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 );
53 changes: 53 additions & 0 deletions WordPress/Tests/WP/GetMetaSingleUnitTest.php
Original file line number Diff line number Diff line change
@@ -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,
);
}
}

0 comments on commit 7f76630

Please sign in to comment.