Follow up on review feedback for the dual-mode void conditional return types

[westonruter]

Follow-up to r63440 and r63441, addressing review feedback from @IanDelMar on #13359.

Five commits, each independently reviewable. Four act on points raised in that review; one corrects a claim made in the previous pull request's own description.

1. Every falsy $args scalar belongs in the undecidable branch

The tags whose display flag lives in an $args array also accept a query string, which PHPStan cannot read, so their conditional return types end in a third branch that leaves the type a union and reports nothing. That branch tested $args is ''|array, treating the empty string as the only scalar behaving like an empty argument set.

'0' behaves identically. parse_str( '0', $r ) yields array( 0 => '' ), so wp_parse_args( '0', $defaults ) sets no key the tag reads and the flag keeps its default — the call prints and returns nothing, exactly as '' does. It was resolving to the retrieval type instead, so consuming its meaningless result went unreported.

The branch is now ''|'0'|array, matching how the flag conditions themselves already spell the falsy set as false|0|''|'0'. Ten tags are affected: wp_list_authors(), wp_list_bookmarks(), wp_tag_cloud(), wp_list_comments(), paginate_comments_links(), wp_get_archives(), the_title_attribute(), wp_list_pages(), wp_page_menu() and wp_list_users() — the nine raised in review plus wp_get_archives(), which has the same branch and the same gap.

Every other call shape is unchanged. A query string carrying a visible flag and an $args built at runtime both stay unions and are still reported in neither mode; an array setting the flag still resolves to the retrieval type; an empty array or a truthy flag still resolves to void.

2. wp_dropdown_languages() bails on a falsy argument, not a missing one

The return description said nothing comes back "when the required id or name argument is missing". Neither is required and neither can go missing: both default to 'locale', so a caller omitting them gets a dropdown. The function bails on a falsy value the caller supplied.

The void in the union covers exactly that bail, and until now did nothing, since a plain union never resolves to void. A conditional makes it report where the bail is visible. '' and '0' are the falsy strings the arguments are documented to hold, mirroring how the sibling tags spell the falsy set of a bool|int flag.

The two shapes have to nest rather than combine. PHPStan cannot represent a union of two unsealed array shapes with different required keys and widens array{ id: ''|'0', ... }|array{ name: ''|'0', ... } to plain non-empty-array:

/** @var array{ id: ''|'0', ... }|array{ name: ''|'0', ... } $u */
\PHPStan\dumpType( $u );   // Dumped type: non-empty-array

That would ask whether $args has any keys at all, resolving every populated array — including all seven call sites in core — to void:

Call	Combined	Nested
array( 'id' => '' )	void ✔	void ✔
array( 'name' => '' )	void ✔	void ✔
array( 'id' => 'x', 'name' => 'y' )	void ✘	string ✔
array( 'echo' => false )	void ✘	string ✔
array( 'selected' => 'de_DE' )	void ✘	string ✔
array() / no argument	string ✔	string ✔

The two rows that look right in the combined column are coincidence: the empty array passes only by failing non-empty-array, and the two bail shapes pass only by being non-empty. src/ was swept for the same construct — no other conditional in core unions array shapes.

This also corrects the previous description, which filed wp_dropdown_languages() under "not dual-mode at all — there is no argument for a condition to switch on". There is one; it is the bail condition rather than the display flag.

3. the_date() and the_modified_date() say what they return

Both described their return as "String if retrieving." — naming the type rather than the content, and saying nothing about display mode, which is the one outcome the void exists to convey.

the_date() now names the date and the case where there isn't one: it builds its value only when is_new_day(), so a post sharing a date with the one before it retrieves an empty string. Naming the content without that caveat would promise a date the caller may not get, which is the correction already applied to next_posts(), previous_posts() and wp_register() in r63441. the_modified_date() always concatenates a date, so it takes the short form.

Also raised in review was edit_term_link(). As it stands in r63441 it already reads "HTML content when retrieving, null on failure or without the capability to edit the term. Nothing when displaying." — the same structure as the five title tags from single_post_title() through single_term_title(), so it is left alone.

4. void restored on two wpdb methods that never return a value

print_error() and check_database_version() documented void|false and void|WP_Error until r62177 replaced the void with null and added a trailing return null; to each. The premise was that void cannot belong to a union, which holds for PHP's native return types but not for PHPDoc — the premise r63441 corrected.

Both are the shape that keeps void elsewhere in core: the method either succeeds, with nothing to hand back, or reports a failure. print_error() returns false only when errors are suppressed or hidden; otherwise it prints and has no value to give. check_database_version() returns a WP_Error only when the server is too old, and its one caller, wp_check_mysql_version(), tests is_wp_error() and ignores the rest. Under null that meaningless value read as a legitimate one, which is what the void was there to deny — and what nineteen sibling functions documenting void|false, and seven documenting void|WP_Error, still say.

Neither can carry a conditional: both switch on object state rather than on an argument, so the distinction is for the reader. Removing the explicit return null; changes nothing at runtime, since falling off the end returns null anyway, and void in the union is what licenses it.

The rest of r62177 stands. prepare() and get_row() return a null their callers genuinely consume, check_connection() and bail() were corrected in the other direction because their old void branches die rather than return, and get_col_info() is documented mixed.

5. null named in the tags whose retrieval branch can return it

Fifteen dual-mode tags document a null in their return description while the @return tag lists only string|void, so the tag omits a value the prose promises and the conditional @phpstan-return already states.

The three-way union was rejected when those annotations were written, on the grounds that no such form appeared elsewhere in core. That was wrong. Immediately before r62177 the tree held three: WP_Block_Type::__get() as string|string[]|null|void, wpdb::get_row() as array|object|null|void, and one in WP_Theme_JSON as null|void. All three were removed by the same campaign that replaced void in unions on the mistaken premise. The idiom existed; it was erased by the error being undone here.

Thirteen tags become string|null|void — single_post_title(), post_type_archive_title(), single_cat_title(), single_tag_title(), single_term_title(), wp_get_archives(), get_calendar(), edit_term_link(), the_title(), the_title_attribute(), wp_list_comments(), wp_update_php_annotation() and twentytwenty_site_description() — and two become string|string[]|null|void: wp_tag_cloud() and paginate_comments_links(). Values first, then null, then void, following the order the tree used before r62177.

The test is the conditional's retrieval branch, not the wording, so null is named only where a caller can observe it. wp_list_pages(), wp_page_menu(), wp_list_authors(), wp_list_bookmarks() and wp_list_users() look like the same shape but are left alone: their null sits only in the undecidable branch covering a query string or an $args built at runtime, which is the union of both modes rather than a value retrieval mode can hand back. Their retrieval branch is plain string, so string|void is already complete.

Left open

One point from the review is deliberately not acted on. Of the thirty-one dual-mode return descriptions, twenty-three name the flag ("Calendar HTML when $display is false … Nothing otherwise."), six use retrieving/displaying ("Title when retrieving … Nothing when displaying."), and two name a second dimension as well. Folding the six into the majority form is a defensible normalization, but both phrasings are accurate and it would touch five functions beyond the one raised. Happy to add it if wanted.

The suggestion to declare @param true $deprecated_echo on trackback_url(), as php-stubs/wordpress-stubs does to signal a deprecated argument, is also left for its own ticket. It conflicts with the conditional return, which then reports Condition "true is true" in conditional return type is always true. on the function itself, so adopting it means removing the conditional — and it reclassifies every existing trackback_url( false ) call as an argument-type error, which is a policy decision about deprecated arguments across core rather than a detail of this change.

Verification

• A temporary probe file confirms each behavior claimed above: '0' now resolves to plain void on all ten tags and is reported when consumed, while query strings carrying a visible flag, dynamic $args, and arrays setting the flag are unchanged in both modes. The wp_dropdown_languages() conditional resolves correctly in all six shapes tabulated above, and the combined form was verified to fail as described.
• phpstan-diff --changed --staged --base=HEAD is clean on every commit, as enforced by the pre-commit hook.
• PHPCS reports no new errors. The warnings on general-template.php are the pre-existing $wpdb->prepare() ones in wp_get_archives(), on untouched lines.
• PHPUnit Tests_DB passes for the wpdb change: 651 tests, 985 assertions, 2 skipped. Removing the explicit return null; is a no-op at runtime.
• tests/phpstan/baselines/return.missing.neon stays deleted — void in the union is what licenses falling off the end.

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

Use of AI Tools

AI assistance: Yes
Tool(s): Claude Code
Model(s): Opus 5
Used for: Evaluating the review feedback, verifying each claim against PHPStan, and drafting the annotations and this description. The PHPStan, PHPCS and PHPUnit verification was run against the working tree, and the result has been reviewed and is owned by me.

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Core Committers: Use this line as a base for the props when committing in SVN:

Props westonruter, marian1.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[IanDelMar]

Better use false|0|0.0|''|'0'|[]|null. While the docs say that $args['id'] and $args['name'] should be string, other values may still be passed. In those cases, PHPStan may incorrectly infer a string return type.

This may apply to other conditional return types as well.

[IanDelMar]

Folding the six into the majority form is a defensible normalization, but both phrasings are accurate and it would touch five functions beyond the one raised.

Yes, that's what I meant. It does not follow the "under this condition, the return type is ..." pattern. "when retrieving" requires looking up when retrieval actually happens.

@param true $deprecated_echo on trackback_url(), as php-stubs/wordpress-stubs does to signal a deprecated argument, is also left for its own ticket. It conflicts with the conditional return

There is no other way to express this in the DocBlock. Doing it differently would require a PHPStan rule. And because it is a "conflict" the example added @phpstan-ignore conditionalType.alwaysTrue. There is no need to remove the conditional.