Make WordPress Core

Opened 7 weeks ago

Closed 2 weeks ago

#64703 closed defect (bug) (fixed)

Code Modernization: Replace void in PHPDoc union return types with null in class-wpdb.php

Reported by: apermo's profile apermo Owned by: westonruter's profile westonruter
Milestone: 7.1 Priority: normal
Severity: normal Version: 4.3
Component: Database Keywords: has-patch
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Following up on #64694 which fixed paginate_links(), this ticket addresses 3 instances in class-wpdb.php where @return annotations incorrectly use void as part of a union type.

The |void pattern was introduced in WordPress 4.3 via [32654] for prepare() and check_connection(), and in WordPress 5.5 via [47944] for get_row().

Why this matters

In PHP's type system, void means "the function does not return a value" and cannot be part of a union type (this is a compile error in PHP 8.0+). When a function uses bare return; or falls off the end without returning, the actual runtime value is null. Therefore @return Type|void should be @return Type|null.

This affects:

  • Static analysis tools (PHPStan, Psalm)
  • IDE autocompletion and type inference
  • Developer expectations about return values

Backward Compatibility

This is a safe, non-breaking change. As demonstrated by @westonruter in the #64694 PR review, return; and return null; are identical at runtime across every PHP version from 4.3.0 through 8.5.3: https://3v4l.org/3KQC8

Proposed Changes

For each instance:

  1. Update the @return annotation to replace void with null (or remove |void where the function always returns a typed value)
  2. Change bare return; statements to return null;
  3. Update description text to say "null" instead of "void" / "nothing"

Affected Functions

Function Line Current Recommendation Since
prepare() 1456 string|void string|null 4.3
check_connection() 2120 bool|void bool (void incorrect — always returns bool or terminates) 4.3
get_row() 3059 array|object|null|void array|object|null (void redundant — already includes null) 5.5

Note on prepare(): Uses bare return; on error paths. The change to return null; is safe, but the error-path behavior should be reviewed to confirm null is the intended signal for "no query".

See #64694 for prior art.

Change History (10)

This ticket was mentioned in PR #11009 on WordPress/wordpress-develop by @apermo.


7 weeks ago
#1

  • Keywords has-patch added

Co-Authored-By: xateman

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

## Use of AI Tools

Used AI for research, documentation and for the replacements. Everything was reviewed by myself and @xateman before opening this PR.

#2 @apermo
7 weeks ago

Props: @xate

Co-Authored and Reviewed

#3 @SergeyBiryukov
7 weeks ago

  • Description modified (diff)

#4 in reply to: ↑ description ; follow-up: @SergeyBiryukov
7 weeks ago

Hi there, thanks for the ticket!

Replying to apermo:

Function Line Current Recommendation Since
check_connection() 2120 bool|void bool (void incorrect — always returns bool or terminates) 4.3

Not sure if that's correct, as wpdb::check_connection() can still return nothing if it proceeds to the dead_db() call. As it stands, the proposed change causes a PHPStan warning (as seen on GitHub):

Method wpdb::check_connection() should return bool but return statement is missing.

Perhaps an explicit return false should be added at the end?

#5 in reply to: ↑ 4 @apermo
7 weeks ago

Replying to SergeyBiryukov:

Perhaps an explicit return false should be added at the end?

@westonruter suggested adding a @return never to dead_db(), and that solved the issue. While the return false; wouldn't have hurt either, that solution is cleaner.

#6 @westonruter
7 weeks ago

  • Keywords commit added
  • Milestone changed from Awaiting Review to 7.0
  • Owner set to westonruter
  • Status changed from new to reviewing

@westonruter commented on PR #11009:


6 weeks ago
#8

I've cherry-picked the never return types into https://github.com/WordPress/wordpress-develop/pull/11151 since they directly fix issues for PHPStan rule level 1.

#9 @audrasjb
4 weeks ago

  • Milestone changed from 7.0 to 7.1

As per today's 7.0 pre-RC1 bug scrub:
As we're a few hours from 7.0 RC1, I'm moving this ticket to milestone 7.1 to give it more time for review.

#10 @westonruter
2 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 62177:

Code Quality: Replace void with proper return types in wpdb and related functions.

Replace void in union return types with null, false, or never as appropriate, and add explicit return null statements where methods previously fell through without a return value.

Methods updated in wpdb: prepare(), print_error(), check_connection(), get_row(), get_col_info(), bail(), check_database_version().

Also adds @return never to dead_db() and fixes the @phpstan-return syntax for wp_die().

Developed in https://github.com/WordPress/wordpress-develop/pull/11009

Props apermo, westonruter, xate, mukesh27, SergeyBiryukov.
Fixes #64703.

Note: See TracTickets for help on using tickets.