close

Make WordPress Core

Opened 3 weeks ago

Last modified 10 days ago

#64703 reviewing defect (bug)

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.0 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 (8)

Image

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


3 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 Image @apermo
3 weeks ago

Props: @xate

Co-Authored and Reviewed

#3 Image @SergeyBiryukov
3 weeks ago

  • Description modified (diff)

#4 in reply to: ↑ description ; follow-up: Image @SergeyBiryukov
3 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 Image @apermo
2 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 Image @westonruter
2 weeks ago

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

Image

@westonruter commented on PR #11009:


10 days 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.

Note: See TracTickets for help on using tickets.