Skip to content

Commit 16c798e

Browse files
committed
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.
1 parent 3bc61bd commit 16c798e

File tree

3 files changed

+86
-27
lines changed

3 files changed

+86
-27
lines changed

WordPressVIPMinimum/Sniffs/Performance/TaxonomyMetaInOptionsSniff.php

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

1212
use PHP_CodeSniffer\Util\Tokens;
13-
use WordPressVIPMinimum\Sniffs\Sniff;
13+
use WordPressCS\WordPress\AbstractFunctionParameterSniff;
1414

1515
/**
1616
* Restricts the implementation of taxonomy term meta via options.
1717
*/
18-
class TaxonomyMetaInOptionsSniff extends Sniff {
18+
class TaxonomyMetaInOptionsSniff extends AbstractFunctionParameterSniff {
1919

2020
/**
2121
* List of options_ functions
2222
*
23+
* @deprecated 3.1.0 This property should never have been public.
24+
*
2325
* @var array<string>
2426
*/
2527
public $option_functions = [
@@ -45,27 +47,37 @@ class TaxonomyMetaInOptionsSniff extends Sniff {
4547
];
4648

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

5656
/**
57-
* Process this test when one of its tokens is encountered
58-
*
59-
* @param int $stackPtr The position of the current token in the stack passed in $tokens.
57+
* Functions this sniff is looking for.
6058
*
61-
* @return void
59+
* @var array<string, true> Keys are the target functions, value irrelevant.
6260
*/
63-
public function process_token( $stackPtr ) {
61+
protected $target_functions = [
62+
'get_option' => true,
63+
'add_option' => true,
64+
'update_option' => true,
65+
'delete_option' => true,
66+
];
6467

65-
if ( in_array( $this->tokens[ $stackPtr ]['content'], $this->option_functions, true ) === false ) {
66-
return;
67-
}
6868

69+
/**
70+
* Process the parameters of a matched function.
71+
*
72+
* @param int $stackPtr The position of the current token in the stack.
73+
* @param string $group_name The name of the group which was matched.
74+
* @param string $matched_content The token content (function name) which was matched
75+
* in lowercase.
76+
* @param array $parameters Array with information about the parameters.
77+
*
78+
* @return void
79+
*/
80+
public function process_parameters( $stackPtr, $group_name, $matched_content, $parameters ) {
6981
$openBracket = $this->phpcsFile->findNext( Tokens::$emptyTokens, $stackPtr + 1, null, true );
7082

7183
if ( $this->tokens[ $openBracket ]['code'] !== T_OPEN_PARENTHESIS ) {
Lines changed: 53 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,55 @@
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.

WordPressVIPMinimum/Tests/Performance/TaxonomyMetaInOptionsUnitTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -32,12 +32,12 @@ 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,
4141
];
4242
}
4343
}

0 commit comments

Comments
 (0)