Skip to content

Commit 95afb67

Browse files
rodrigoprimojrfnl
rodrigoprimo
and
jrfnl
committed
Apply suggestions from code review
Co-authored-by: Juliette <[email protected]>
1 parent f384fb0 commit 95afb67

File tree

4 files changed

+57
-55
lines changed

4 files changed

+57
-55
lines changed

‎WordPress/Docs/WP/OptionAutoloadStandard.xml‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,8 @@
77
<![CDATA[
88
When using `add_option()`, `update_option()`, `wp_set_options_autoload()`,
99
`wp_set_option_autoload()`, or `wp_set_option_autoload_values()`, it is recommended to
10-
explicitly set the autoload value to ensure predictable behavior. This value determines whether
10+
explicitly set the autoload parameter to ensure predictable behavior. This parameter determines whether
1111
the option is automatically loaded on every page load, which can impact site performance.
12-
13-
Check https://felix-arntz.me/blog/autoloading-wordpress-options-efficiently-and-responsibly/ for
14-
more information on when to set autoload to `true` or `false`.
1512
]]>
1613
</standard>
1714
<standard>
@@ -43,13 +40,15 @@ add_option(
4340
</code_comparison>
4441
<standard>
4542
<![CDATA[
46-
Using 'yes' or 'no' is not recommended. Those values are deprecated. Instead, use
47-
`true`|`false`|`null` if using `add_option()` or `update_option()` or `true`|`false` if using
48-
`wp_set_option_autoload_values()`, `wp_set_option_autoload()`, or `wp_set_options_autoload()`.
43+
When calling the `add_option()` or `update_option()` functions, use `true`, `false` or `null`
44+
as the value for the autoload parameter. For the `wp_set_option_autoload_values()`,
45+
`wp_set_option_autoload()`, or `wp_set_options_autoload()` functions, use `true` or `false`.
46+
47+
Using 'yes' or 'no' as the value for the autoload parameter is deprecated since WordPress 6.6.0.
4948
]]>
5049
</standard>
5150
<code_comparison>
52-
<code title="Valid: Using boolean values.">
51+
<code title="Valid: Using a boolean value.">
5352
<![CDATA[
5453
update_option(
5554
'my_option',
@@ -58,7 +57,7 @@ update_option(
5857
);
5958
]]>
6059
</code>
61-
<code title="Invalid: Using deprecated values.">
60+
<code title="Invalid: Using a deprecated value.">
6261
<![CDATA[
6362
update_option(
6463
'my_option',
@@ -70,10 +69,10 @@ update_option(
7069
</code_comparison>
7170
<standard>
7271
<![CDATA[
73-
It is not recommended to use internal-only values ('auto', 'auto-on', 'auto-off', 'on', 'off')
74-
in plugin or theme code. Instead, use `true`|`false`|`null` if using `add_option()` or
75-
`update_option()` or `true`|`false` if using `wp_set_option_autoload_values()`,
76-
`wp_set_option_autoload()`, or `wp_set_options_autoload()`.
72+
Use `true`|`false`|`null` if using `add_option()` or `update_option()` or `true`|`false` if
73+
using `wp_set_option_autoload_values()`, `wp_set_option_autoload()`, or
74+
`wp_set_options_autoload()`. It is strongly discouraged to use internal-only values ('auto',
75+
'auto-on', 'auto-off', 'on', 'off') in plugin or theme code.
7776
]]>
7877
</standard>
7978
<code_comparison>
@@ -114,7 +113,7 @@ wp_set_options_autoload(
114113
115114
wp_set_option_autoload(
116115
'my_option',
117-
<em>false</em>
116+
<em>true</em>
118117
);
119118
]]>
120119
</code>

‎WordPress/Sniffs/WP/OptionAutoloadSniff.php‎

Lines changed: 34 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
3030
/**
3131
* The phrase to use for the metric recorded by this sniff.
3232
*
33+
* @since 3.2.0
34+
*
3335
* @var string
3436
*/
3537
const METRIC_NAME = 'Value of the `$autoload` parameter in the option functions';
@@ -42,7 +44,7 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
4244
*
4345
* @var string[]
4446
*/
45-
protected static $valid_values_add_and_update = array( 'true', 'false', 'null' );
47+
private $valid_values_add_and_update = array( 'true', 'false', 'null' );
4648

4749
/**
4850
* List of valid values for the `$autoload` parameter in the wp_set_options_autoload(),
@@ -52,7 +54,7 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
5254
*
5355
* @var string[]
5456
*/
55-
protected static $valid_values_other_functions = array( 'true', 'false' );
57+
private $valid_values_other_functions = array( 'true', 'false' );
5658

5759
/**
5860
* List of deprecated values for the `$autoload` parameter.
@@ -61,7 +63,7 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
6163
*
6264
* @var string[]
6365
*/
64-
protected static $deprecated_values = array( 'yes', 'no' );
66+
private $deprecated_values = array( 'yes', 'no' );
6567

6668
/**
6769
* Internal-use only values for `$autoload` that cannot be fixed automatically by the sniff.
@@ -70,7 +72,7 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
7072
*
7173
* @var string[]
7274
*/
73-
protected static $internal_values_non_fixable = array( 'auto', 'auto-on', 'auto-off' );
75+
private $internal_values_non_fixable = array( 'auto', 'auto-on', 'auto-off' );
7476

7577
/**
7678
* Internal-use only values for `$autoload` that can be fixed automatically by the sniff.
@@ -79,16 +81,16 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
7981
*
8082
* @var string[]
8183
*/
82-
protected static $internal_values_fixable = array( 'on', 'off' );
84+
private $internal_values_fixable = array( 'on', 'off' );
8385

8486
/**
8587
* List of replacements for fixable values.
8688
*
8789
* @since 3.2.0
8890
*
89-
* @var string[]
91+
* @var array<string, string>
9092
*/
91-
protected static $fixable_values = array(
93+
private $fixable_values = array(
9294
'yes' => 'true',
9395
'no' => 'false',
9496
'on' => 'true',
@@ -102,7 +104,7 @@ final class OptionAutoloadSniff extends AbstractFunctionParameterSniff {
102104
*
103105
* @var string[]
104106
*/
105-
protected static $autoload_is_optional = array( 'add_option', 'update_option' );
107+
private $autoload_is_optional = array( 'add_option', 'update_option' );
106108

107109
/**
108110
* The group name for this group of functions.
@@ -215,7 +217,7 @@ public function process_no_parameters( $stackPtr, $group_name, $function_name )
215217
*
216218
* @return void
217219
*/
218-
protected function handle_wp_set_option_autoload_values( array $options_param, $stackPtr ) {
220+
private function handle_wp_set_option_autoload_values( array $options_param, $stackPtr ) {
219221
$array_token = $this->phpcsFile->findNext(
220222
Tokens::$emptyTokens,
221223
$options_param['start'],
@@ -266,15 +268,16 @@ protected function handle_wp_set_option_autoload_values( array $options_param, $
266268
* @since 3.2.0
267269
*
268270
* @param int $stackPtr The position of the current token in the stack.
269-
* @param string $function_name The name of the function being checked.
271+
* @param string $function_name The token content (function name) which was matched
272+
* in lowercase.
270273
*
271274
* @return void
272275
*/
273-
protected function maybe_display_missing_autoload_warning( $stackPtr, $function_name ) {
276+
private function maybe_display_missing_autoload_warning( $stackPtr, $function_name ) {
274277
$this->phpcsFile->recordMetric( $stackPtr, self::METRIC_NAME, 'param missing' );
275278

276279
// Only display a warning for the functions in which the `$autoload` parameter is optional.
277-
if ( in_array( $function_name, self::$autoload_is_optional, true ) ) {
280+
if ( in_array( $function_name, $this->autoload_is_optional, true ) ) {
278281
$this->phpcsFile->addWarning(
279282
'It is recommended to always pass the `$autoload` parameter when using %s() function.',
280283
$stackPtr,
@@ -290,13 +293,13 @@ protected function maybe_display_missing_autoload_warning( $stackPtr, $function_
290293
* @since 3.2.0
291294
*
292295
* @param array $autoload_info Information about the autoload value (start and end tokens and
293-
* the clean and raw value).
294-
* @param string $function_name The name of the function being checked.
296+
* the clean value).
297+
* @param string $function_name The token content (function name) which was matched
298+
* in lowercase.
295299
*
296300
* @return void
297301
*/
298-
protected function check_autoload_value( array $autoload_info, $function_name ) {
299-
// Find the first and second param non-empty tokens (the second token might not exist).
302+
private function check_autoload_value( array $autoload_info, $function_name ) {
300303
$param_first_token = $this->phpcsFile->findNext(
301304
Tokens::$emptyTokens,
302305
$autoload_info['start'],
@@ -312,9 +315,9 @@ protected function check_autoload_value( array $autoload_info, $function_name )
312315

313316
$normalized_value = strtolower( $autoload_info['clean'] );
314317

315-
if ( T_NS_SEPARATOR === $this->tokens[ $param_first_token ]['code']
318+
if ( \T_NS_SEPARATOR === $this->tokens[ $param_first_token ]['code']
316319
&& $param_second_token
317-
&& in_array( strtolower( $this->tokens[ $param_second_token ]['content'] ), self::$valid_values_add_and_update, true )
320+
&& in_array( strtolower( $this->tokens[ $param_second_token ]['content'] ), $this->valid_values_add_and_update, true )
318321
) {
319322
// Ensure the sniff handles correctly `true`, `false` and `null` when they are
320323
// namespaced (preceded by a backslash).
@@ -323,18 +326,18 @@ protected function check_autoload_value( array $autoload_info, $function_name )
323326
$normalized_value = substr( $normalized_value, 1 );
324327
}
325328

326-
if ( in_array( $function_name, self::$autoload_is_optional, true ) ) {
327-
$valid_values = self::$valid_values_add_and_update;
329+
if ( in_array( $function_name, $this->autoload_is_optional, true ) ) {
330+
$valid_values = $this->valid_values_add_and_update;
328331
} else {
329-
$valid_values = self::$valid_values_other_functions;
332+
$valid_values = $this->valid_values_other_functions;
330333
}
331334

332335
if ( in_array( $normalized_value, $valid_values, true ) ) {
333336
$this->phpcsFile->recordMetric( $param_first_token, self::METRIC_NAME, $normalized_value );
334337
return;
335338
}
336339

337-
if ( in_array( $this->tokens[ $param_first_token ]['code'], array( T_VARIABLE, T_STRING ), true )
340+
if ( in_array( $this->tokens[ $param_first_token ]['code'], array( \T_VARIABLE, \T_STRING ), true )
338341
&& 'null' !== strtolower( $this->tokens[ $param_first_token ]['content'] )
339342
) {
340343
// Bail early if the first non-empty token in the parameter is T_VARIABLE or T_STRING as
@@ -344,7 +347,7 @@ protected function check_autoload_value( array $autoload_info, $function_name )
344347
}
345348

346349
if ( $param_second_token
347-
&& ! in_array( $this->tokens[ $param_first_token ]['code'], array( T_ARRAY, T_OPEN_SHORT_ARRAY ), true )
350+
&& ! isset( Collections::arrayOpenTokensBC()[ $this->tokens[ $param_first_token ]['code'] ] )
348351
) {
349352
// Bail early if the parameter has two or more non-empty tokens and the second token is
350353
// not an array opener as this means an undetermined param value or a value that is not
@@ -355,7 +358,7 @@ protected function check_autoload_value( array $autoload_info, $function_name )
355358

356359
$autoload_value = TextStrings::stripQuotes( $autoload_info['clean'] );
357360

358-
$known_discouraged_values = array_merge( self::$deprecated_values, self::$internal_values_non_fixable, self::$internal_values_fixable );
361+
$known_discouraged_values = array_merge( $this->deprecated_values, $this->internal_values_non_fixable, $this->internal_values_fixable );
359362

360363
if ( in_array( $autoload_value, $known_discouraged_values, true ) ) {
361364
$metric_value = $autoload_value;
@@ -365,15 +368,15 @@ protected function check_autoload_value( array $autoload_info, $function_name )
365368

366369
$this->phpcsFile->recordMetric( $param_first_token, self::METRIC_NAME, $metric_value );
367370

368-
if ( in_array( $autoload_value, self::$deprecated_values, true ) ) {
371+
if ( in_array( $autoload_value, $this->deprecated_values, true ) ) {
369372
$message = 'The use of `%s` as the value of the `$autoload` parameter is deprecated. Use `%s` instead.';
370373
$error_code = 'Deprecated';
371-
$data = array( $autoload_info['clean'], self::$fixable_values[ $autoload_value ] );
372-
} elseif ( in_array( $autoload_value, self::$internal_values_fixable, true ) ) {
374+
$data = array( $autoload_info['clean'], $this->fixable_values[ $autoload_value ] );
375+
} elseif ( in_array( $autoload_value, $this->internal_values_fixable, true ) ) {
373376
$message = 'The use of `%s` as the value of the `$autoload` parameter is discouraged. Use `%s` instead.';
374377
$error_code = 'InternalUseOnly';
375-
$data = array( $autoload_info['clean'], self::$fixable_values[ $autoload_value ] );
376-
} elseif ( in_array( $autoload_value, self::$internal_values_non_fixable, true ) ) {
378+
$data = array( $autoload_info['clean'], $this->fixable_values[ $autoload_value ] );
379+
} elseif ( in_array( $autoload_value, $this->internal_values_non_fixable, true ) ) {
377380
$message = 'The use of `%s` as the value of the `$autoload` parameter is discouraged.';
378381
$error_code = 'InternalUseOnly';
379382
$data = array( $autoload_info['clean'] );
@@ -391,7 +394,7 @@ function ( $value ) {
391394
$data = array( $autoload_info['clean'], $valid_values_string );
392395
}
393396

394-
if ( in_array( $autoload_value, array_keys( self::$fixable_values ), true ) ) {
397+
if ( in_array( $autoload_value, array_keys( $this->fixable_values ), true ) ) {
395398
$fix = $this->phpcsFile->addFixableWarning(
396399
$message,
397400
$param_first_token,
@@ -400,7 +403,7 @@ function ( $value ) {
400403
);
401404

402405
if ( $fix ) {
403-
$this->phpcsFile->fixer->replaceToken( $param_first_token, self::$fixable_values[ $autoload_value ] );
406+
$this->phpcsFile->fixer->replaceToken( $param_first_token, $this->fixable_values[ $autoload_value ] );
404407
}
405408

406409
return;

‎WordPress/Tests/WP/OptionAutoloadUnitTest.inc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ $ok = wp_set_option_autoload_values(
4545
);
4646

4747
/*
48-
* These should all be ignored by the sniff as the autoload value is undetermined or not easy to
49-
* determine.
48+
* These should all be ignored by the sniff as the autoload value can't be reliably
49+
* determined.
5050
*/
5151
$ignored = add_option( 'my_option', 'my_value', '', $some_variable );
5252
$ignored = add_option( 'my_option', 'my_value', '', $obj->yes );
@@ -71,8 +71,8 @@ $ignored = wp_set_option_autoload_values(
7171
);
7272

7373
/*
74-
* These are ignored because it is not easy to determine the option names and the autoload values or
75-
* the `$options` parameter is empty.
74+
* These are ignored because the option names and the autoload values can't be reliably determined
75+
* or the `$options` parameter is empty.
7676
*/
7777
$ignored = wp_set_option_autoload_values( $options );
7878
$ignored = wp_set_option_autoload_values( options: $options );
@@ -82,7 +82,7 @@ $ignored = wp_set_option_autoload_values(options: array());
8282
$ignored = wp_set_option_autoload_values(options: []);
8383

8484
/*
85-
* Invalid function calls that are ignored by the sniff as a mandatory parameter is missing
85+
* Invalid function calls that are ignored by the sniff as a mandatory parameter is missing.
8686
*/
8787
$ignored = wp_set_option_autoload_values();
8888
$ignored = wp_set_options_autoload();

‎WordPress/Tests/WP/OptionAutoloadUnitTest.inc.fixed‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -45,8 +45,8 @@ $ok = wp_set_option_autoload_values(
4545
);
4646

4747
/*
48-
* These should all be ignored by the sniff as the autoload value is undetermined or not easy to
49-
* determine.
48+
* These should all be ignored by the sniff as the autoload value can't be reliably
49+
* determined.
5050
*/
5151
$ignored = add_option( 'my_option', 'my_value', '', $some_variable );
5252
$ignored = add_option( 'my_option', 'my_value', '', $obj->yes );
@@ -71,8 +71,8 @@ $ignored = wp_set_option_autoload_values(
7171
);
7272

7373
/*
74-
* These are ignored because it is not easy to determine the option names and the autoload values or
75-
* the `$options` parameter is empty.
74+
* These are ignored because the option names and the autoload values can't be reliably determined
75+
* or the `$options` parameter is empty.
7676
*/
7777
$ignored = wp_set_option_autoload_values( $options );
7878
$ignored = wp_set_option_autoload_values( options: $options );
@@ -82,7 +82,7 @@ $ignored = wp_set_option_autoload_values(options: array());
8282
$ignored = wp_set_option_autoload_values(options: []);
8383

8484
/*
85-
* Invalid function calls that are ignored by the sniff as a mandatory parameter is missing
85+
* Invalid function calls that are ignored by the sniff as a mandatory parameter is missing.
8686
*/
8787
$ignored = wp_set_option_autoload_values();
8888
$ignored = wp_set_options_autoload();

0 commit comments

Comments
 (0)