Skip to content

Commit 7b288b2

Browse files
authored
Merge pull request #872 from Automattic/feature/performance-taxonomymetainoptions-various-improvements
2 parents 70e5569 + ae8557a commit 7b288b2

File tree

3 files changed

+109
-31
lines changed

3 files changed

+109
-31
lines changed

WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php

Lines changed: 37 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -10,16 +10,19 @@
1010
namespace WordPressVIPMinimum\Sniffs\Performance;
1111

1212
use PHP_CodeSniffer\Util\Tokens;
13-
use WordPressVIPMinimum\Sniffs\Sniff;
13+
use PHPCSUtils\Utils\PassedParameters;
14+
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
1415

1516
/**
1617
* Restricts the implementation of taxonomy term meta via options.
1718
*/
18-
class TaxonomyMetaInOptionsSniff extends Sniff {
19+
class TaxonomyMetaInOptionsSniff extends AbstractFunctionParameterSniff {
1920

2021
/**
2122
* List of options_ functions
2223
*
24+
* @deprecated 3.1.0 This property should never have been public.
25+
*
2326
* @var array<string>
2427
*/
2528
public $option_functions = [
@@ -45,34 +48,47 @@ class TaxonomyMetaInOptionsSniff extends Sniff {
4548
];
4649

4750
/**
48-
* Returns an array of tokens this test wants to listen for.
51+
* The group name for this group of functions.
4952
*
50-
* @return array<int|string>
53+
* @var string
5154
*/
52-
public function register() {
53-
return [ T_STRING ];
54-
}
55+
protected $group_name = 'option_functions';
5556

5657
/**
57-
* Process this test when one of its tokens is encountered
58+
* Functions this sniff is looking for.
5859
*
59-
* @param int $stackPtr The position of the current token in the stack passed in $tokens.
60+
* @var array<string, true> Keys are the target functions, value irrelevant.
61+
*/
62+
protected $target_functions = [
63+
'get_option' => true,
64+
'add_option' => true,
65+
'update_option' => true,
66+
'delete_option' => true,
67+
];
68+
69+
70+
/**
71+
* Process the parameters of a matched function.
72+
*
73+
* @param int $stackPtr The position of the current token in the stack.
74+
* @param string $group_name The name of the group which was matched.
75+
* @param string $matched_content The token content (function name) which was matched
76+
* in lowercase.
77+
* @param array $parameters Array with information about the parameters.
6078
*
6179
* @return void
6280
*/
63-
public function process_token( $stackPtr ) {
64-
65-
if ( in_array( $this->tokens[ $stackPtr ]['content'], $this->option_functions, true ) === false ) {
81+
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
82+
$target_param = PassedParameters::getParameterFromStack( $parameters, 1, 'option' );
83+
if ( $target_param === false ) {
84+
// Missing (required) target parameter. Probably live coding, nothing to examine (yet). Bow out.
6685
return;
6786
}
6887

69-
$openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
70-
71-
if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) {
72-
return;
73-
}
88+
$param_start = $target_param['start'];
89+
$param_end = ( $target_param['end'] + 1 ); // Add one to include the last token in the parameter in findNext searches.
7490

75-
$param_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $openBracket + 1, null, true );
91+
$param_ptr = $this->phpcsFile->findNext( Tokens::$emptyTokens, $param_start, $param_end, true );
7692

7793
if ( $this->tokens[ $param_ptr ]['code'] === T_DOUBLE_QUOTED_STRING ) {
7894
foreach ( $this->taxonomy_term_patterns as $taxonomy_term_pattern ) {
@@ -101,7 +117,9 @@ public function process_token( $stackPtr ) {
101117
}
102118

103119
$object_operator = $this->phpcsFile->findNext( Tokens::$emptyTokens, $variable_name + 1, null, true );
104-
if ( $this->tokens[ $object_operator ]['code'] !== T_OBJECT_OPERATOR ) {
120+
if ( $this->tokens[ $object_operator ]['code'] !== T_OBJECT_OPERATOR
121+
&& $this->tokens[ $object_operator ]['code'] !== T_NULLSAFE_OBJECT_OPERATOR
122+
) {
105123
return;
106124
}
107125

Lines changed: 64 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,66 @@
11
<?php
22

3-
get_option( "taxonomy_rating_$obj->term_id" ); // NOK.
4-
delete_option( "taxonomy_rating_{$obj->term_id}" ); // NOK.
5-
get_option( "taxonomy_rating_$tag_id" ); // NOK.
6-
update_option( 'taxonomy_rating_' . $category_id ); // NOK.
7-
get_option( "taxonomy_rating_{$cat_id}" ); // NOK.
8-
add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK.
3+
/*
4+
* Not the sniff target.
5+
*/
6+
use function add_option;
7+
8+
my\ns\get_option( "taxonomy_rating_$tag_id" );
9+
$this->delete_option( "taxonomy_rating_$tag_id" );
10+
$this?->update_option( "taxonomy_rating_$tag_id" );
11+
MyClass::get_option( "taxonomy_rating_$tag_id", );
12+
echo DELETE_OPTION;
13+
namespace\add_option( "taxonomy_rating_$tag_id" );
14+
15+
function &update_option() {}
16+
17+
/*
18+
* These should all be okay.
19+
*/
20+
// Incomplete function call, should be ignored by the sniff.
21+
$incorrect_but_ok = get_option();
22+
$incorrect_but_ok = update_option();
23+
24+
// Ignore as undetermined.
25+
Get_Option( $option );
26+
\add_option( $obj->get_optionname() );
27+
update_option( OPTION_NAME, );
28+
\DELETE_OPTION( MyClass::$option_name );
29+
30+
get_option(...$params); // PHP 5.6 argument unpacking, parameter value unknown.
31+
32+
// Looks like a function call, but is a PHP 8.0+ class instantiation via an attribute.
33+
#[Get_Option('taxonomy_rating_')]
34+
function foo() {}
35+
36+
array_walk($options, \update_option(...),); // PHP 8.1 first class callable.
37+
38+
// Ignore as not touching taxonomy term patterns.
39+
\get_option( 'option_name' );
40+
add_option( 'option_' . $post_id, );
41+
update_Option( "prefix_{$setting}" );
42+
add_option( 'option_' . MY_CONSTANT, );
43+
add_option( 'foo_' . $obj->$propname );
44+
update_option( 'foo_' . $obj->page_id ); // NOK.
45+
46+
47+
/*
48+
* These should all be flagged.
49+
*/
50+
get_option( "taxonomy_rating_$obj->term_id", ); // NOK.
51+
\delete_option( "taxonomy_rating_{$obj->term_id}" ); // NOK.
52+
get_option( /*comment*/ "taxonomy_rating_$tag_id" ); // NOK.
53+
UpDate_OPTION( 'taxonomy_rating_' . $category_id /*comment*/ , ); // NOK.
54+
\Get_Option( "taxonomy_rating_{$cat_id}" ); // NOK.
55+
add_option( 'taxonomy_rating_' . $obj->term_id ); // NOK.
56+
57+
58+
// Safeguard handling of function calls using PHP 8.0+ named parameters.
59+
get_option(default_value: false); // OK, well, not really, missing required $option param, but that's not the concern of this sniff.
60+
add_option(value: "$tag_id", option: 'option_name'); // OK.
61+
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.
62+
63+
add_option(value: $value, option: "$tag_id" ); // Bad.
64+
65+
// Safeguard handling of PHP 8.0+ nullsafe object operator.
66+
get_option( 'taxonomy_rating_' . $obj?->term_id ); // NOK.

WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,14 @@ public function getErrorList() {
3232
*/
3333
public function getWarningList() {
3434
return [
35-
3 => 1,
36-
4 => 1,
37-
5 => 1,
38-
6 => 1,
39-
7 => 1,
40-
8 => 1,
35+
50 => 1,
36+
51 => 1,
37+
52 => 1,
38+
53 => 1,
39+
54 => 1,
40+
55 => 1,
41+
63 => 1,
42+
66 => 1,
4143
];
4244
}
4345
}

0 commit comments

Comments
 (0)