#63430 (Coding Standards: replace isset() ternary with null coalescing) – WordPress Trac

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

9 months 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:

9 months 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:

It would be helpful to include test coverage (if applicable) or indicate if existing tests already verify these code paths, just to be safe.
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
9 months ago

Milestone changed from Awaiting Review to 6.9

#4 @SergeyBiryukov
9 months 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:

9 months ago #5

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

#6 @johnbillion
9 months ago

Version trunk deleted

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

8 months ago

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

4 months ago

#9 @wildworks
4 months ago

This ticket was featured on today's 6.9 Bug Scrub.

It looks like there are a huge number of areas that need to be updated. Try searching using the following regular expression in your code editor.

isset\(\s*([^)]+?)\s*\)\s*\?\s*\1\s*:\s*([^\n;]+)

In my environment, 701 items and 208 files were found.

Updating all of these in a single PR may be a bit risky, so we may want to consider how to split the work up.

#10 @jorbin
4 months ago

Milestone changed from 6.9 to Future Release

Punting to Future Release due to the impending 6.9 beta1 and lack of recent progress.

I would also question the value of doing this in general and how it aligns with the core stance on refactoring

#11 @SergeyBiryukov
7 weeks ago

Milestone changed from Future Release to 7.0

#12 @SergeyBiryukov
7 weeks ago

Owner set to SergeyBiryukov
Resolution set to fixed
Status changed from new to closed

In 61403:

Code Modernization: Replace some isset() ternary checks with null coalescing.

Since PHP 7.0 introduced the null coalescing operator, and WordPress now requires at least PHP 7.2.24, isset( $var ) ? $var : null ternary checks can be safely replaced with the more concise $var ?? null syntax.

As some new code using the null coalescing operator has already been introduced into core in recent releases, this commit continues with the code modernization by implementing incremental changes for easier review.

Props seanwei, getsyash, krupalpanchal, wildworks, jorbin, SergeyBiryukov.
Fixes #63430. See #58874.

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

7 weeks ago #13

Keywords has-unit-tests added

This is a follow-up up to r61403.

I used PHPStorm's regex search and replace with the pattern isset$\s*(.+?)\s*$\s*\?\s*\1(\s*): and the replacement $1$2??.

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

#14 @westonruter
7 weeks ago

Proposed follow-up in #58874:

Now that [61403] has been committed to fix #63430, which is a subset of what this ticket is all about, I've followed up with PR 10654. I did a regex replacement to update the remaining instances of isset() ternaries to use the null coalescing operator as well. So it's basically it's a refresh of PR 4886. If there aren't any objections, I'll commit.

#15 @westonruter
5 weeks ago

In 61424:

Code Modernization: Update tests to use null coalescing operator in place of isset() in ternaries.

Developed as a subset of https://github.com/WordPress/wordpress-develop/pull/10654

Follow-up to [61404], [61403].

See #58874, #63430.

#16 @westonruter
5 weeks ago

Resolution fixed deleted
Status changed from closed to reopened

Re-opening because there are still many instances of the ternary which can be replaced, although this is also more-or-less a duplicate of #58874.

#17 @westonruter
5 weeks ago

In 61429:

Code Modernization: REST API: Use null coalescing operator in place of isset() in ternaries.

Developed as a subset of https://github.com/WordPress/wordpress-develop/pull/10654
Initially developed in https://github.com/WordPress/wordpress-develop/pull/4886

Follow-up to [61424], [61404], [61403].