Reconsider return types for `_get_cron_array()`.

[peterwilsoncc]

`_get_cron_array()` currently has multiple return types:

• false: The cron option is not set/is set incorrectly
• array: The cron option is set correctly and either does or does not contain any cron jobs.

As either type can be returned when there are no cron jobs, I think it's worth discussing whether the return types are correct, specifically:

• should the function always return an array
• should the function always return false when there are zero cron jobs

Either change would improve the consistency of the return type but be a technical break of backward compatibility.

My inclination is to always return an array as it's fair to expect delete_option( '_cron' ) zeros out the cron jobs in the same way that _set_cron_array( [] ) does.

I also think always returning an array when there is no option/no cron jobs is more consistent than returning a different type when there are zero registered jobs depending on how they were deleted.

Follow up to #3058, #53635.

[jrf]

I also think always returning an array when there is no option/no cron jobs is more consistent than returning a different type when there are zero registered jobs depending on how they were deleted.

@peterwilsoncc I concur, though changing the return type is a BC-break and may break code relying on the false return type.

FYI: we discussed this function and the return type in ​last weeks livestream about PHP 8.1.

[jrf]

Note: the BC break may be acceptable in this case as the function is marked as internal only.

[jrf]

For visibility - there is a ticket open to add tests to this function: https://core.trac.wordpress.org/ticket/53927

[peterwilsoncc]

Replying to jrf:

Note: the BC break may be acceptable in this case as the function is marked as internal only.

That's what I was thinking but hoping for thoughts from other committers such as yourself.

There are a ​few plugins in the repository using it directly, but a check of those with over 100,000 installs shows that many assume it's always an array and those that don't would be unaffected by such a change.

FYI: we discussed this function and the return type in last weeks livestream about PHP 8.1.

Yeah, I saw that while cleaning the house on the weekend. It's what prompted me to start thinking whether it should always return an array.

[jrf]

Replying to peterwilsoncc:

There are a ​few plugins in the repository using it directly, but a check of those with over 100,000 installs shows that many assume it's always an array and those that don't would be unaffected by such a change.

Verified and I concur, though it is painful to see that some of those huge plugins are doing it wrong.

Yeah, I saw that while cleaning the house on the weekend. It's what prompted me to start thinking whether it should always return an array.

That's wonderful to hear! Hoping that we'll be able to inspire many more improvements via those live streams in the future 💗

[peterwilsoncc]

In 52656:

Upgrade: Prevent warnings upgrading cron array.

An unvisited site may have an undefined cron array, resulting in _get_cron_array() returning the value false. Previously this would trigger warning in upgrade_590() as the function assumed _get_cron_array() would alway return an array.

No database version change is required as the upgrade routine was successful on sites with a cron array during 5.9.0. On sites without a cron array, the error has already been thrown if they are running db version 51917. This fix is only required for new sites or those upgrading that have skipped 5.9.0.

Follow up to [51917].

Props chrisvanpatten, kapilpaul, SergeyBiryukov.
Fixes #54906.
See #53940.

[peterwilsoncc]

In 52663:

Upgrade: Prevent warnings upgrading cron array.

An unvisited site may have an undefined cron array, resulting in _get_cron_array() returning the value false. Previously this would trigger warning in upgrade_590() as the function assumed _get_cron_array() would alway return an array.

No database version change is required as the upgrade routine was successful on sites with a cron array during 5.9.0. On sites without a cron array, the error has already been thrown if they are running db version 51917. This fix is only required for new sites or those upgrading that have skipped 5.9.0.

Follow up to [51917].

Props chrisvanpatten, kapilpaul, SergeyBiryukov, audrasjb.
Merges [52656] and [52662] to the 5.9 branch.
Fixes #54906.
See #53940.

[peterwilsoncc]

I think this can be done and _get_cron_array() always return an array.

It's a BC change but as zero scheduled tasks can currently either return false or an empty array I think it's an improvement.

[thakkarhardik]

Hi @peterwilsoncc, I've added a patch which will always return array instead of false on failure from _get_cron_array().

https://core.trac.wordpress.org/ticket/53940

[peterwilsoncc]

In 53791:

Cron API: Modify _get_cron_array() to always return an array.

Change the return type of _get_cron_array() to an empty array if the cron option is either missing or of an
unexpected type.

This change ensures the return value for no registered events is consistently an empty array. Previously the return
value could be either an empty array or false.

Props thakkarhardik, jrf, costdev.
Fixes #53940.

https://core.trac.wordpress.org/changeset/53791

[peterwilsoncc]

Thanks @thakkarhardik, I've committed your patch.

I ended up including a bunch of test changes alongside your fix to make sure the function was working as intended.