✨ New WordPress.WP.OptionAutoload sniff #2520

rodrigoprimo · 2025-02-26T23:27:54Z

This sniff warns users about missing $autoload parameter when calling relevant WordPress option functions.

It includes tests and sniff documentation in the XML format.

Sniff behavior

The sniff runs when the following functions are found in the code:

add_option( $option, $value = '', $deprecated = '', $autoload = null)
update_option( $option, $value, $autoload = null )
wp_set_options_autoload( $options, $autoload )
wp_set_option_autoload( $option, $autoload )
wp_set_option_autoload_values( $options )

For add_option() and update_option(), if the $autoload parameter is not passed, it simply recommends passing it. For the other three functions, the $autoload parameter (or $options parameter in the case of wp_set_option_autoload_values()) is mandatory, so it is not needed to check if the parameter is omitted.

If the $autoload parameter is passed and is true|false|null for add_option() and update_option() or true|false for wp_set_options_autoload(), wp_set_option_autoload() and wp_set_option_autoload_values(), the sniff stays silent as those are valid values.

If the $autoload parameter is passed and is 'yes'|'no', the sniff flags as discouraged parameter values and auto-fixes to true|false.

For the internal-use only values of the $autoload parameter, if the value is 'auto'|'auto-on'|'auto-off' the sniff flags it as unsupported without auto-fixing. If the value is 'on'|'off', the sniff flags it as unsupported and auto-fixes to true|false.

Any other value passed is flagged as unsupported, and a list of supported values is provided.

Open questions

I'm unsure about a few choices that I made in the XML documentation and could use some feedback on the following:

I opted to include a link to a document explaining how to decide which value to use for the $autoload parameter. Since I could not find something wp.org, I ended up linking to a post written by a core committer in their blog. I'm not sure if this is a problem or not.
Maybe combining the <standard> blocks for invalid, deprecated, and internal-only values in a single block would be better? I created a block for each, but there is some repetition in the block description and the valid examples.
I added a <standard> block without code examples at the top with a general description of what the sniff does before creating blocks for each of the warnings generated by the sniff.

dingo-d · 2025-02-27T10:09:24Z

I opted to include a link to a document explaining how to decide which value to use for the $autoload parameter. Since I could not find something wp.org, I ended up linking to a post written by a core committer in their blog. I'm not sure if this is a problem or not.

The only problem is that that is not official documentation. And if their site goes down, you have a dead link in the documentation.

I added a block without code examples at the top with a general description of what the sniff does before creating blocks for each of the warnings generated by the sniff.

Does the validation passes against the phpcsdocs.xsd?

jrfnl

@rodrigoprimo Thank you for working on this.

I've so far only done a first pass reading through it (without running the sniff/docs etc), but seeing the amount of remarks I've already left, I figured I should give you the chance to iterate on the current state before continuing my review.

WordPress/Sniffs/WP/OptionAutoloadSniff.php

WordPress/Tests/WP/OptionAutoloadUnitTest.inc.fixed

rodrigoprimo · 2025-02-28T21:10:02Z

Thanks for your review, @dingo-d and @jrfnl! I believe I addressed all of your comments except two that are still pending, and I will need a bit more time (#2520 (comment) and #2520 (comment)).

The only problem is that that is not official documentation. And if their site goes down, you have a dead link in the documentation.

That makes sense. As @jrfnl suggested, I removed the paragraph that included this link.

Does the validation passes against the phpcsdocs.xsd?

I believe so. This is how I tested:

$ xmllint --schema phpcsdocs.xsd WordPress/Docs/WP/OptionAutoloadStandard.xml --noout 
WordPress/Docs/WP/OptionAutoloadStandard.xml validates

Let me know if I'm missing something.

rodrigoprimo · 2025-03-12T18:38:16Z

@jrfnl, I addressed the two points that were pending. Could you please take another look at this PR when you get a chance?

WordPress/Docs/WP/OptionAutoloadStandard.xml

WordPress/Sniffs/WP/OptionAutoloadSniff.php

jrfnl

Hi @rodrigoprimo thanks for making those initial updates.

I've now done a proper code review (with the exception of the tests). Hope this helps!

WordPress/Docs/WP/OptionAutoloadStandard.xml

jrfnl · 2025-07-04T14:26:11Z

WordPress/Docs/WP/OptionAutoloadStandard.xml

+    </standard>
+    <code_comparison>
+        <code title="Valid: Using a boolean value or `null`">
+        <![CDATA[


Should the valid example have an example from both function groups ?

jrfnl · 2025-07-04T14:29:46Z

WordPress/Docs/WP/OptionAutoloadStandard.xml

+    Using 'yes' or 'no' is deprecated since WordPress 6.6.0. Using 'auto', 'auto-on', 'auto-off',
+    'on', or 'off' in plugin/theme code is strongly discouraged as those values are meant for
+    internal-use only.


I wonder if we're not giving people ideas by explicitly listing the internal values.... ?
Maybe the second sentence should be removed ? Or maybe just mention something along the lines of "Plugin/theme code should also not use values for the parameter which are for WP-Core internal use only."

I opted to add the sentence that you suggested to mention that plugin/theme code should not use internal values, while not mentioning the values directly to avoid giving people ideas.

WordPress/Docs/WP/OptionAutoloadStandard.xml

jrfnl · 2025-07-04T14:34:51Z

WordPress/Docs/WP/OptionAutoloadStandard.xml

+        ]]>
+        </code>
+    </code_comparison>
+    <standard>


Would it make sense to move this <standard> block up ? This is about the parameter being missing, while the first block also talks about that, so it kind of feels like having this above the block about the values reads more fluently.

I'm not sure I understand your suggestion. As far as I can see, the first block does not mention the parameter being missing for add_option() and update_option(), so I'm not sure why it would read better if this <standard> block were moved up.

My reasoning for ordering the blocks the way they are was:

First block: introduces what the sniff is about and what the autoload parameter is.

Second block: explains the rule about the parameter value that applies to all the functions that this sniff cares about.

Third block: explains the rule about the parameter missing that only applies to add_option() and update_option().

I think I prefer the current order, but I'm happy to reconsider if I'm missing something.

WordPress/Sniffs/WP/OptionAutoloadSniff.php

jrfnl · 2025-07-04T15:54:24Z

WordPress/Sniffs/WP/OptionAutoloadSniff.php

+
+		$autoload_value = TextStrings::stripQuotes( $autoload_info['clean'] );
+
+		$known_discouraged_values = array_merge( $this->deprecated_values, $this->internal_values_non_fixable, $this->internal_values_fixable );


Any particular reason to use array_merge() instead of array union (+) ? The arrays being merged are perfectly suitable for use with array union.

jrfnl · 2025-07-04T15:57:49Z

WordPress/Sniffs/WP/OptionAutoloadSniff.php

+			$data       = array( $autoload_info['clean'], $this->fixable_values[ $autoload_value ] );
+		} elseif ( isset( $this->internal_values_fixable[ $autoload_value ] ) ) {
+			$message    = 'The use of `%s` as the value of the `$autoload` parameter is discouraged. Use `%s` instead.';
+			$error_code = 'InternalUseOnly';


I'm not sure I like the same error code being used for two - closely related, but still different - errors here.... (line 397 + 401)

Maybe the second error code (line 401) could be something like InternalUseOnlyAuto (as they values which will be flagged all start with "auto") ?

And maybe this one should then be InternalUseOnlyOnOff ? (to keep things consistent)

WordPress/Sniffs/WP/OptionAutoloadSniff.php

rodrigoprimo · 2025-07-04T21:31:14Z

Thanks for taking another look at this PR, @jrfnl. I started replying to your feedback and implementing the changes, but I was unable to finish everything today. I won't be able to look into this next week. The following week, I will address the comments that I was unable to address today.

This sniff warns users about missing `$autoload` parameter when calling relevant WordPress option functions. The sniff runs when the following functions are found in the code: - add_option( $option, $value = '', $deprecated = '', $autoload = null) - update_option( $option, $value, $autoload = null ) - wp_set_options_autoload( $options, $autoload ) - wp_set_option_autoload( $option, $autoload ) - wp_set_option_autoload_values( $options ) For `add_option()` and `update_option()`, if the `$autoload` parameter is not passed, it simply recommends passing it. For the other three functions, the `$autoload` parameter (or `$options` parameter in the case of `wp_set_option_autoload_values()`) is mandatory, so it is not needed to check if the parameter is omitted. If the `$autoload` parameter is passed and is `true|false|null` for `add_option()` and `update_option()` or `true|false` for `wp_set_options_autoload()`, `wp_set_option_autoload()` and `wp_set_option_autoload_values()`, the sniff stays silent as those are valid values. If the `$autoload` parameter is passed and is `'yes'|'no'`, the sniff flags as discouraged parameter values and auto-fixes to `true|false`. For the internal-use only values of the `$autoload` parameter, if the value is `'auto'|'auto-on'|'auto-off'` the sniff flags it as unsupported without auto-fixing. If the value is `'on'|'off'`, the sniff flags it as unsupported and auto-fixes to `true|false`. Any other value passed is flagged as an unsupported value and a list of supported values is provided. The sniff ignores the function call when it is not able to determine the value of the `$autoload` parameter. For example, when a variable or a constant is passed.

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

…he rest of the sniff code

This allows to use isset() instead of in_array()

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

…tax errors

…t_option_functions`

…ional

rodrigoprimo · 2025-07-04T21:40:02Z

Rebased without changes to fix an unrelated failing test. 39524bc is the first commit addressing the latest feedback.

…n discouraged values

…xable internal values

…st to reach that condition

…t the condition on OptionAutoloadSniff.php:247 correctly

rodrigoprimo · 2025-07-15T17:48:17Z

@jrfnl, thanks again for your review! I finished addressing all the comments that I was not able to address before, and now this PR is ready for another review. Just a reminder that 39524bc is the first commit addressing the latest feedback, as I had to rebase without changes to fix an unrelated failing test.

Also, just noting here that I had to force push the last commit as the build was failing due to a temporary PHPStan error. Documenting here in case we start seeing this error more often (https://github.com/rodrigoprimo/WordPress-Coding-Standards/actions/runs/16300169675/job/46032411241):

==> Setup Tools
✓ composer Added composer 2.8.10
✗ phpstan 403: rate limit exceeded

rodrigoprimo · 2025-07-15T17:53:00Z

What about moving it into the condition ?

Regarding the comment quoted above, I'm not sure why I cannot reply directly to it, but it was addressed, and now the param missing metric will only be recorded for add_option() and update_option().

…e review

rodrigoprimo added Component: Extra Type: Enhancement labels Feb 26, 2025

jrfnl requested changes Feb 28, 2025

View reviewed changes

jrfnl added the Status: Awaiting feedback label Feb 28, 2025

rodrigoprimo force-pushed the new-option-autoload branch from 9d70828 to a3da583 Compare February 28, 2025 20:47

rodrigoprimo mentioned this pull request Mar 6, 2025

Check if sniffs using AbstractFunctionParameterSniff are handling variable unpacking correctly #2521

Open

18 tasks

rodrigoprimo force-pushed the new-option-autoload branch from cd264e6 to fc98b8e Compare March 12, 2025 18:34

rodrigoprimo removed the Status: Awaiting feedback label Mar 12, 2025

rodrigoprimo commented Jul 4, 2025

View reviewed changes

WordPress/Docs/WP/OptionAutoloadStandard.xml Outdated Show resolved Hide resolved

WordPress/Sniffs/WP/OptionAutoloadSniff.php Outdated Show resolved Hide resolved

jrfnl requested changes Jul 4, 2025

View reviewed changes

jrfnl added the Status: Awaiting feedback label Jul 4, 2025

rodrigoprimo and others added 15 commits July 4, 2025 18:31

Apply suggestions from code review

f9f2b95

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Attemp to improve the XML documentation by combining standards

f18b9ed

Fix unintentional coding standard violation in a test case

18c8a31

Update some tests to have more function name case variations

1b5b8bf

Update some tests to have more whitespaces, comments and annotations

5ac7e1c

Use false === isset() instead of ! isset() for consistency with t…

d5ec3a2

…he rest of the sniff code

Improve sniff performance by changing the format of some properties

a777a97

This allows to use isset() instead of in_array()

Apply suggestions from code review

39524bc

Co-authored-by: Juliette <663378+jrfnl@users.noreply.github.com>

Remove unused parameter

62532db

Rename test case file to be able to add more test case files with syn…

60a85c9

…tax errors

Handle case when $array_token is false + add a test for it

c8bee3d

Improve sniff XML doc following suggestions from code review

f506c0b

Add link to external blog post about when to use the autoload flag

5b58aba

Add backticks to all function calls in the docblocks

e85d37f

rodrigoprimo added 4 commits July 4, 2025 18:31

Rename property valid_values_other_functions to `valid_values_wp_se…

d6dbd92

…t_option_functions`

Explain in the docblock the array properties where all values are true

c17ed01

Ensure all @param array tags explain the structure of the array

21a5350

Only record param missing metric when the autoload parameter is opt…

325f3ba

…ional

rodrigoprimo force-pushed the new-option-autoload branch from f80b867 to 325f3ba Compare July 4, 2025 21:35

rodrigoprimo added 6 commits July 15, 2025 11:25

Use array union instead of array_merge() to create the list of know…

4944d88

…n discouraged values

Use one error code for fixable internal values and another for non-fi…

2b48e41

…xable internal values

Update if condition to explicitly check what is expected and add a te…

4c7b209

…st to reach that condition

Add another test without whitespaces surrounding the parameter to tes…

2a5815a

…t the condition on OptionAutoloadSniff.php:247 correctly

Add a comment explaining why null needs special treatment on line 361

6c930ea

Explicitly check if $param_second_token is an integer

4c7d770

rodrigoprimo force-pushed the new-option-autoload branch from 31270ce to 4c7d770 Compare July 15, 2025 17:43

rodrigoprimo removed the Status: Awaiting feedback label Jul 15, 2025

Move the third block of the documentation up as suggestion during cod…

e13b23b

…e review


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

		$known_discouraged_values = array_merge( $this->deprecated_values, $this->internal_values_non_fixable, $this->internal_values_fixable );

Uh oh!

✨ New WordPress.WP.OptionAutoload sniff #2520

Are you sure you want to change the base?

✨ New WordPress.WP.OptionAutoload sniff #2520

Uh oh!

Conversation

rodrigoprimo commented Feb 26, 2025

Sniff behavior

Open questions

Uh oh!

dingo-d commented Feb 27, 2025

Uh oh!

jrfnl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rodrigoprimo commented Feb 28, 2025

Uh oh!

rodrigoprimo commented Mar 12, 2025

Uh oh!

Uh oh!

Uh oh!

jrfnl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rodrigoprimo commented Jul 4, 2025

Uh oh!

rodrigoprimo commented Jul 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rodrigoprimo commented Jul 15, 2025

Uh oh!

rodrigoprimo commented Jul 15, 2025

Uh oh!

Uh oh!

rodrigoprimo commented Jul 4, 2025 •

edited

Loading