Add sniff to warn about missing `$autoload` parameter when calling relevant WordPress option functions

[felixarntz]

Is your feature request related to a problem?

Several option functions (e.g. add_option(), update_option()) have been supporting an $autoload parameter that is optional and has had a default value of 'yes'. This has led to notable problems where far too many options that plugins or themes add are being autoloaded, even if they're only needed on a few specific pages of the application.

WordPress 6.6 introduced changes in the API and how it should be used. While we can't require a parameter that used to be optional, it is now more than ever encouraged to always pass it. Going forward, not passing the parameter will be interpreted by core as implicit, i.e. core is able to determine whether or not to autoload the option by itself. So far this effectively still means an option without any provided value will be autoloaded, unless it is a particularly large option (the only condition implemented in WordPress core so far). But this current behavior to still mostly autoload by default is only for backward compatibility and could change in the future.

See https://make.wordpress.org/core/2024/06/18/options-api-disabling-autoload-for-large-options/ for additional context. And somewhat related: https://make.wordpress.org/core/2023/10/17/new-option-functions-in-6-4/

Describe the solution you'd like

WordPress-Extra should include a sniff that warns whenever the $autoload parameter is missing on add_option() or update_option() calls.

While they should typically use either true or false, passing null is fine too as explicit acknowledgement that core will be able to make the decision.

Nice to have

It used to be formally documented that the strings 'yes' and 'no' could be passed alternatively to true and false respectively. This is still supported and not formally deprecated, but it's also no longer encouraged, so it would be nice to clean that up over time. There are however no functional differences between passing one or the other variant, so it's certainly not as important as the main solution described above. But if it's trivial to include in the new sniff, it's worth considering.

Additional context (optional)

Just some additional context how to decide whether an option should be autoloaded or not:

• Use true if the option is retrieved in the majority of page loads, including frontend requests.
• Use false if the option is only retrieved in particular page loads, e.g. a specific page in the frontend or a specific admin screen.
• Use null to leave the decision whether or not to autoload the option to WordPress core.

One additional explanation for a somewhat common situation:

• If a plugin or theme uses several options only in a shared place (e.g. a specific admin screen), not autoloading them may intuitively feel like the wrong decision because it would make several database requests for them instead of a combined one. However, in this situation the options should still not be autoloaded because it would lead those options to be fetched in all kinds of other areas where they are never used.
• Instead, for such a scenario plugin developers should rely on the functions wp_prime_option_caches() or wp_prime_option_caches_by_group(), which were introduced in WordPress 6.4. With these functions, multiple options can be retrieved ("pre-loaded") from the database with a single request, without "polluting" the list of autoloaded options, which should remain reserved for options that are loaded across most of the WordPress site.

[jrfnl]

@felixarntz Thanks for opening this issue.

I think this is a valid feature request and I support this proposal.

I do have some questions though which will need to be answered to fully understand the specs for the new sniff.

1. Which functions should be analyzed by the sniff ?
   My guess is: add_option(), update_option(), but there may be additional functions which I'm not aware of ?
2. Are there (future) intentions to change the add_site_option() and update_site_option() (which currently don't support the $autoload parameter) ?
3. From what I understand the $autoload parameter used to accept true|false|'yes'|'no' as valid values and as of WP 6.6, it accepts null as well.
   Will older WP versions be able to handle a null value being passed ? Or is there a risk of "passing null to non-nullable" deprecation notices or TypeErrors if null is passed on WP < 6.6 ?
   If passing null is WP-cross-version compatible, we can include it in the recommendation. If not, we'll need a version check based on the minimum_wp_version property (set by the plugin/theme) to determine what recommendation can be given and/or adjust the warning message based on the minimum supported WP version.

Just some additional context how to decide whether an option should be autoloaded or not:

I think we'll need to have a careful think about the warning message text, but more than anything, I think this could be explained in more detail in the sniff XML documentation.

@felixarntz Would you be up to contributing these docs, or collaborating on them, once the sniff has been written ?

It used to be formally documented that the strings 'yes' and 'no' could be passed alternatively to true and false respectively. This is still supported and not formally deprecated, but it's also no longer encouraged, so it would be nice to clean that up over time. There are however no functional differences between passing one or the other variant, so it's certainly not as important as the main solution described above. But if it's trivial to include in the new sniff, it's worth considering.

Looking at commit WordPress/wordpress-develop@7d606a3, I see some more potential values for the option - 'on'|'off'|'auto'|'auto-on'|'auto-off' -, though those may only be intended for internal use (I haven't gone through the code in detail, just skimmed it for now). Could you clarify ?

And yes, it would be trivial for the sniff to flag anything which is non-true|false|null and with some more, less trivial, code, I think the sniff could even auto-fix 'yes' to true and 'no' to false, which should greatly help with cleaning these things up.

Along the same lines, $autoload parameter values which are only supposed to be for internal use and not for userland use, could also be flagged by the sniff.

This would mean the sniff logic would need to be along the following lines:

• If the $autoload parameter is not passed, recommend passing the parameter. Do not auto-fix as what the value should be should be a conscious decision by a developer.
• If the $autoload parameter is passed and is true|false, stay silent.
• TBD: If the $autoload parameter is passed and is null, stay silent with minimum WP 6.6, possibly flag for WP < 6.6 as unsupported until WP 6.6 ?
• If the $autoload parameter is passed and is 'yes'|'no', flag as discouraged parameter values and auto-fix.
• TBD: If the $autoload parameter is passed and is any of the internal-use only values, flag as unsupported and suggest replacing it with the appropriate true|false|null value and auto-fix (if possible ?).
• If any other value is passed, flag as unsupported value and point out that this means the default will be used. Do not auto-fix.

Does this sound correct to you ?

[felixarntz]

@jrfnl Replying to your questions:

Which functions should be analyzed by the sniff ?

It's only add_option() and update_option().

Are there (future) intentions to change the add_site_option() and update_site_option() (which currently don't support the $autoload parameter) ?

Not planned, no. They are predominantly for multisite network options, where there's no autoloading functionality like for regular options. On regular sites they pass through to regular options, so there's potential value here, but it's tricky from an API perspective and overall probably low impact, given how less common it is for those functions to be used.

Will older WP versions be able to handle a null value being passed ?

Yes, that won't be a problem at all from a functionality perspective. update_option() formally used to accept null prior to 6.6 already, and for add_option(), while not documented, it would result in the correct behavior prior to 6.6, which would be the same as passing 'yes' back then, which was the previous default. That's because add_option() handles the parameter value like this:

$autoload = ( 'no' === $autoload || false === $autoload ) ? 'no' : 'yes';

Would you be up to contributing these docs, or collaborating on them, once the sniff has been written ?

Definitely happy to help with that!

'on'|'off'|'auto'|'auto-on'|'auto-off' -, though those may only be intended for internal use (I haven't gone through the code in detail, just skimmed it for now). Could you clarify ?

All those values are internal values stored in the database, nothing to be concerned with for someone calling add_option() or update_option(). Brief explanation:

• 'on'|'off' will be stored if the developer explicitly passes true|false (or the old 'yes'|'no', just for BC).
• 'auto'|'auto-on'|'auto-off' will be stored if the developer doesn't pass anything or passes null, since WordPress core makes the decision based on various heuristics.

Along the same lines, $autoload parameter values which are only supposed to be for internal use and not for userland use, could also be flagged by the sniff.

I like that idea. Yeah, none of the additional above 'on'|'off'|'auto'|'auto-on'|'auto-off' should be passed to the add_option() or update_option() functions.

---

Your summary sounds great, plus taking the above into account on the TBD sections:

• Passing null is fine even prior to WP 6.6.
• For the internal values, I think 'on'|'off' can be auto-fixed to true|false. I'm not sure about 'auto'|'auto-on'|'auto-off' though since those are only for a decision by core. If someone passes them, I'd be worried they haven't understood how this works, so I think it's better in that case to only flag it without trying to auto-fix.

Let me know if you have any follow up questions.

[jrfnl]

Thanks for your response @felixarntz. That clarifies all questions I had.

@rodrigoprimo Is this something you may like to work on ? And if so, have you got time to do so ?

[rodrigoprimo]

@rodrigoprimo Is this something you may like to work on ? And if so, have you got time to do so ?

@jrfnl, yes, I would like to work on this new sniff. I can start next week.

[rodrigoprimo]

Updating the sniff summary that @jrfnl created with the latest information that @felixarntz provided:

• The sniff should run when the functions add_option() and update_option() are found in the code.
• If the $autoload parameter is not passed, recommend passing the parameter. Do not auto-fix as what the value should be should be a conscious decision by a developer.
• If the $autoload parameter is passed and is true|false|null, stay silent.
• If the $autoload parameter is passed and is 'yes'|'no', flag as discouraged parameter values and auto-fix to true|false.
• For the internal-use only values of the $autoload parameter, if the value is 'auto'|'auto-on'|'auto-off' flag as unsupported without auto-fix. If the value is 'on'|'off', flag it as unsupported and auto-fix to true|false.
• If any other value is passed, flag as unsupported value and point out that this means the default will be used. Do not auto-fix.

[felixarntz]

Thanks @rodrigoprimo for this summary, it looks accurate to me.

I just remembered the additional option autoloading related core functions introduced in WordPress 6.4:

• wp_set_options_autoload( $options, $autoload )
• wp_set_option_autoload( $option, $autoload )
• wp_set_option_autoload_values( $options ) (here the autoload values are within an associative $options array, so maybe trickier, if not impossible to lint)

I just opened a WordPress Core ticket to update the parameter documentation of those functions to as well no longer recommend 'yes'|'no': https://core.trac.wordpress.org/ticket/61929

While they probably aren't used too much in the ecosystem compared to add_option() and update_option(), we may want to warn and auto-fix 'yes'|'no' to true|false for those functions as well. WDYT?

cc @jrfnl

[jrfnl]

I just remembered the additional option autoloading related core functions introduced in WordPress 6.4:

* `wp_set_options_autoload( $options, $autoload )`
* `wp_set_option_autoload( $option, $autoload )`
* `wp_set_option_autoload_values( $options )` (here the autoload values are within an associative `$options` array, so maybe trickier, if not impossible to lint)

While they probably aren't used too much in the ecosystem compared to add_option() and update_option(), we may want to warn and auto-fix 'yes'|'no' to true|false for those functions as well. WDYT?

I concur and I think it would be good to include this in the sniff.

I don't think the wp_set_option_autoload_values( $options ) function will be a problem to handle as the array format is clearly defined, though it will need special casing in the sniff.

Caveat for all detection: if the values aren't being passed directly in the function call/hard-coded, we're out of luck as tracing variable definitions/function calls throughout the codebase is not something PHPCS is suitable for.
(finding the $options definition if defined within the same scope (= within the function body of the function which calls the wp_set_option_autoload_values() function) may be possible, but that's about it)

In other words: the below can be flagged and fixed:

add_option( $option, $value, '', 'yes' );
wp_set_option_autoload_values(
    array(
        'optionA' => 'no',
        'optionB' => 'yes'
    )
);

... but these can't be:

add_option( $option, $value, '' AUTOLOAD_VALUE );
wp_set_option_autoload_values($options_array);

[rodrigoprimo]

Including those three functions sounds like a good idea to me as well.

We discussed generating a warning when the $autoload parameter is missing for calls to add_option() and update_option() as this parameter is optional.

The same parameter is mandatory for the wp_set_options_autoload() and wp_set_option_autoload() functions, so I believe that for those two functions when the $autoload parameter is missing, the sniff should generate an error. What do you think?

[felixarntz]

The same parameter is mandatory for the wp_set_options_autoload() and wp_set_option_autoload() functions, so I believe that for those two functions when the $autoload parameter is missing, the sniff should generate an error. What do you think?

I personally think for those functions we don't necessarily have to sniff for the parameter missing on those functions. In those cases it would already cause a PHP error anyway because it is technically required. So for wp_set_options_autoload() and wp_set_option_autoload(), I think WPCS should only focus on whether the values given are the recommended booleans. If none are given, this will already fail with WordPress core alone.

[rodrigoprimo]

I think that is a fair point and a good option as well.

I prefer slightly the error as it seems to me as a more consistent behavior from the sniff perspective. The sniff will always generate something (a warning or a error) when the $autoload parameter is missing instead of a warning when an optional $autoload parameter is missing and nothing when a mandatory parameter is missing. That being said, I'm fairly new to the world of sniffs.

What is your take on this, @jrfnl?

[jrfnl]

I agree with @felixarntz on this.

It is not the job of the sniff to flag PHP errors. That's a case of scope creep for this sniff, but also for sniffs in general.
Some examples of prior art:

WordPress-Coding-Standards/WordPress/Sniffs/PHP/IniSetSniff.php

Lines 148 to 151 in 7f76630

	if ( false === $option_param || false === $value_param ) {
	// Missing required param. Not the concern of this sniff. Bow out.
	return;
	}

WordPress-Coding-Standards/WordPress/Sniffs/DateTime/CurrentTimeTimestampSniff.php

Lines 77 to 81 in 7f76630

	$type_param = PassedParameters::getParameterFromStack( $parameters, 1, 'type' );
	if ( false === $type_param ) {
	// Type parameter not found. Bow out.
	return;
	}