Add sniff to delete_option() + add_option

[GaryJones]

What problem would the enhancement address for VIP?

As explained at https://github.com/woocommerce/woocommerce/pull/27696 there is a race condition when delete_option('foo') is followed by add_option('foo', '...').

Describe the solution you'd like

Having a sniff that looked for a delete_option() call followed by an add_option() immediately or at some point within the same scope, for the same option key, could flag this race condition and suggest using update_option() instead.

What code should be reported as a violation?

function reset_settings() {
	$defaults = get_default_settings();
	delete_option( 'my_settings' );
	add_option( 'my_settings', $defaults );
}

Likely to need some consideration of an option key that is using a variable.

What code should not be reported as a violation?

• add_option() in a different scope.
• different option key (including different variable).

[rebeccahum]

or at some point within the same scope

Do you have any examples on what would be considered in scope?

[GaryJones]

Within the same function or method call.

[rebeccahum]

Starting to work on this one roughly... I believe that the add_option() call with the same option name would have to immediately follow the delete_option(). Being in the same function or method call may be too broad of a scope? (I'm not sure.)

E.g. the sniff would not flag this as a race condition:

function reset_settings() {
	$defaults = get_default_settings();
	delete_option( 'my_settings' );

	// a whole bunch of other stuff is done here before the option gets added back

	add_option( 'my_settings', $defaults );
}

Or this one:

function reset_settings() {
	$defaults = get_default_settings();
	delete_option( 'my_settings' );
	add_option( 'settings', [] );
	add_option( 'my_settings', $defaults );
}

[GaryJones]

Can certainly start with looking at the calls being in consecutive statements.