Make WordPress Core

Opened 5 weeks ago

Last modified 3 days ago

#63430 new enhancement

Coding Standards: replace isset() ternary with null coalescing

Reported by: seanwei's profile seanwei Owned by:
Milestone: 6.9 Priority: normal
Severity: minor Version:
Component: General Keywords: has-patch
Focuses: coding-standards Cc:

Description

When reviewing the source code, I noticed some ternary expressions of the form isset( $var ) ? $var : null.

Since PHP 7.0 introduced the null coalescing operator, and WordPress now requires at least PHP 7.2.24, we can safely replace these ternaries with the more concise $var ?? null syntax.

To ensure the changeset is correct, I'd prefer break the changes to bulk of files instead of fixing entire project in one PR.
I've changed manually with the help with IDE, and verify that the meaning of code is exactly the same.

Change History (7)

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


5 weeks ago
#1

  • Keywords has-patch added

When reviewing the source code, I noticed some ternary expressions of the form isset( $var ) ? $var : null.

Since PHP 7.0 introduced the (null coalescing operator)https://www.php.net/manual/en/migration70.new-features.php#migration70.new-features.null-coalesce-op, and WordPress now requires at least PHP 7.2.24, we can safely replace these ternaries with the more concise $var ?? null syntax.

To ensure the changeset is correct, I'd prefer break the changes to bulk of files instead of fixing entire project in one PR.
I've changed manually with the help with IDE, and verify that the meaning of code is exactly the same.

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

@getsyash commented on PR #8791:


5 weeks ago
#2

Updating isset( $var ) ? $var : null to the null coalescing operator ($var ?? null) definitely makes the code cleaner and more modern, especially now that WordPress supports PHP 7.2.24 and above.
A few thoughts:

  1. It would be helpful to include test coverage (if applicable) or indicate if existing tests already verify these code paths, just to be safe.
  2. Consider checking for any edge cases where isset() might have side effects (e.g. when used on arrays or in contexts with indirect references), though in most cases this replacement should be safe.

#3 @SergeyBiryukov
5 weeks ago

  • Milestone changed from Awaiting Review to 6.9

#4 @SergeyBiryukov
5 weeks ago

Hi there, welcome to WordPress Trac!

Thanks for the ticket, just noting that this was previously also suggested and discussed in #58874.

@krupalpanchal commented on PR #8791:


5 weeks ago
#5

Looks good! But, is this PR contains all isset() from the whole the WordPress code? Have to check this.

#6 @johnbillion
2 weeks ago

  • Version trunk deleted

This ticket was mentioned in Slack in #core by sean_wei. View the logs.


3 days ago

Note: See TracTickets for help on using tickets.