shortcode_parse_atts() result for no attributes does not match the documentation

[SergeyBiryukov]

Background: #59206.

The shortcode_parse_atts() documentation says:

 * @return array|string Array of attribute values keyed by attribute name.
 *                      Returns empty array if there are no attributes.
 *                      Returns the original arguments string if it cannot be parsed.

The middle part (empty array if there are no attributes) appears to be incorrect. In my testing, the function returns an empty string in that case.

Do we want to update the results to match the documentation, or just correct the documentation?

Some history:

• [6659] introduced the shortcode_parse_atts() function.
• [8196] / #7184 added the initial documentation.
• [34744] / #23307 expanded it for empty values.
• [55711] / #57840 adjusted it to the current wording.

[TobiasBg]

Correct, it's an empty string. For more: #26927

[oglekler]

From a logical point, it is better when the return value is consistent. In many cases change of return value can introduce a regression or need of change to plugins/themes, but because of each returning in an array attribute needs to be checked before usage anyway, I cannot imagine a regression in this case.
So, I am suggesting to return empty array if there are no actual attributes.

• Adds conditional to shortcode_parse_atts() to return an empty array if there are no attributes on the shortcode.
• Reorders @return options on shortcode_parse_atts() doc block to reflect order of if statement for clarity.

Trac ticket: https://core.trac.wordpress.org/ticket/59249

[nicolefurlan]

I've added a PR with the change that @oglekler suggested 👍

[nicolefurlan]

Testing Instructions

To test this change, we should verify that shortcodes continue to generate the appropriate markup on the frontend both with and without attributes.

Steps to Test

1. Create a post or page and add shortcodes with and without attributes. WordPress has several built-in shortcodes that we can use for testing: ​https://developer.wordpress.org/plugins/shortcodes/#built-in-shortcodes

e.g.

[caption id="id" caption_id="caption_id" align="aligncenter" width="300" caption="caption text" class="class"]Caption with attributes[/caption]
[caption]Caption without attributes[/caption]

Expected Results

• Shortcodes should continue to generate the appropriate markup on the frontend both with and without attributes.

[swissspidy]

Is this a duplicate of #45929?

Edit: looks the other way around

[swissspidy]

#59613 was marked as a duplicate.

[anonymized_14808221]

The patch here still does not grant consistent output and adds an additional unnecessary if check when we can just do this:
https://core.trac.wordpress.org/ticket/59613
​https://github.com/WordPress/wordpress-develop/pull/5480

There is no reason why we should return a string in any case. It disrupts dependant code, which then needs to "juggle" whether or not it receives expected input.
Please see the linked trac (and pr) which has been marked as duplicate.

The patch committed here would nor resolve the issues this problem causes in dependant code.

[nicolefurlan]

@SergeyBiryukov or @swissspidy do you have any thoughts on #comment:11?

@aaroncampbell pinging you as well as the component maintainer.

I'm happy to update the PR whenever we can reach a consensus.

Since this patch possibly needs an update and definitely still needs testing, I'm moving it to 6.5.

[swissspidy]

A simplified patch like ​https://github.com/WordPress/wordpress-develop/pull/5480 looks good. Just needs correct docblock adjustments and ideally unit tests.

[swissspidy]

#45929 was marked as a duplicate.

I don't see harm in always returning an empty array here.

Reverts https://core.trac.wordpress.org/changeset/56488 because it's no longer needed.

Trac ticket: https://core.trac.wordpress.org/ticket/59249

[swissspidy]

In 57597:

Shortcodes: Always return an array in shortcode_parse_atts().

Previously, shortcode_parse_atts() would return the input (an empty string) if a shortcode had no attributes, even though the documentation said otherwise.

Always returning an (empty) array reduces confusion and improves developer experience as the return value does not have to be manually checked in the shortcode itself.

Props: nicolefurlan, swissspidy, johnbillion, bedas.
Fixes #59249.

Committed in https://core.trac.wordpress.org/changeset/57597

Committed in https://core.trac.wordpress.org/changeset/57597

[desrosj]

Wondering if we should call this out in the miscellaneous dev note? Even if just by a bullet point, it could help someone out that's relying on the incorrect behavior.

[swissspidy]

Sure why not 👍 Either a misc dev note or the field guide I suppose