[wp-trac] [WordPress Trac] #58874: Code Modernization: Consider using the null coalescing operator.

[WordPress Trac]

#58874: Code Modernization: Consider using the null coalescing operator.
-------------------------+-----------------------------------
 Reporter:  costdev      |      Owner:  (none)
     Type:  enhancement  |     Status:  new
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  General      |    Version:
 Severity:  normal       |   Keywords:  has-patch 2nd-opinion
  Focuses:               |
-------------------------+-----------------------------------
 == Overview
 There are currently around 500 instances of `isset( $var ) ? $var :
 'default'` in Core.

 In [https://make.wordpress.org/core/2020/03/20/updating-the-coding-
 standards-for-modern-php/ Updating the Coding Standards for modern PHP],
 the proposal states:
 > The spaceship `<=>`, null coalesce `??` and null coalesce equals `??=`
 operators can not be used in WordPress Core until the minimum PHP version
 has been raised to PHP 7.0 (spaceship and null coalesce) or PHP 7.4 (null
 coalesce equals).

 With [https://make.wordpress.org/core/2023/07/05/dropping-support-for-
 php-5/ the dropping of PHP 5 support in WordPress 6.3], we can now make
 use of the [https://www.php.net/manual/en/migration70.new-
 features.php#migration70.new-features.null-coalesce-op null coalescing
 operator].

 > The null coalescing operator (`??`) has been added as syntactic sugar
 for the common case of needing to use a ternary in conjunction with
 `isset()`. It returns its first operand if it exists and is not `null`;
 otherwise it returns its second operand.
 > [https://www.php.net/manual/en/migration70.new-features.php#migration70
 .new-features.null-coalesce-op PHP.net]

 This allows changes from:
 {{{#!php
 <?php

 $new_var = isset( $var ) ? $var : 'default';
 $new_var = isset( $arr['key']['subkey'][0] ) ? $arr['key']['subkey'][0] :
 'default';
 $new_var = isset( $obj->prop ) ? $obj->prop : 'default';
 }}}
 to:
 {{{#!php
 <?php

 $new_var = $var ?? 'default';
 $new_var = $arr['key']['subkey'][0] ?? 'default';
 $new_var = $obj->prop ?? 'default';
 }}}

 == Proposal

 This ticket proposes that we update these as the above instances often
 result in very long lines or very cumbersome conditions. This follows on
 from similar changes to use `str_starts_with()`, `str_ends_with()` and
 `str_contains()`, helps usher in the bump to a PHP 7 minimum for WordPress
 in a safe way compared to other PHP 7+ features, and promotes increased
 contribution as prospective contributors see WordPress not just enforcing
 a minimum PHP version or using new features in ''new'' code, but
 modernising its ''existing'' codebase.

 As we're very early in 6.4-alpha, making this change now is as "perfect"
 as we could hope to be considering this will invalidate ''some'' patches.
 However, given the verbosity of `isset()` ternaries, these usually occur
 on their own line with very little extra code, so the number of
 invalidated patches should be ''relatively'' low.

 == Considerations

 1. Backporting: This may add extra work if backports involve changing
 lines containing the affected `isset()` ternaries. This applies to
 security backports as well as WordPress 6.3 minor releases. However, our
 earlier changes to `str_starts_with()`, `str_ends_with()` and
 `str_contains()` had a greater risk of creating extra work.
 2. Invalidated patches: Patches that change lines containing the affected
 `isset()` ternaries will need a refresh. This ''is'' a negative, but it's
 also likely to be relatively straightforward to resolve for each patch.
 Our earlier changes to `str_starts_with()`, `str_ends_with()` and
 `str_contains()` risked invalidating many more patches compared to this
 proposal.
 3. Readability: While objectively this is a benefit for brevity,
 readability is subjective.
 4. [https://make.wordpress.org/core/handbook/contribute/code-refactoring/
 Code refactoring should not be done just because we can]: This page
 details several things needed for proposals such as this:

 - **Unit tests**, even if the code was not previously covered. We can’t
 afford regressions, and this will improve our test coverage.
   - The behaviour of the null coalescing operator is the same as `isset(
 $var ) ? $var : 'default'`. This proposal does not suggest changing any
 other instances at this time.
 - **Performance benchmarks**, before and after. We can’t afford
 regressions.
   - The behaviour and performance of the null coalescing operator is the
 same as `isset( $var ) ? $var : 'default'`. This proposal does not suggest
 changing any other instances at this time.
 - **Proper justification and clear rationale of changes are both
 necessary**. Too often it is impossible to determine the purpose,
 objective, or focus of these patches. Code should not be rewritten under
 the cloaks of readability, narrow personal opinion, or general
 subjectiveness.
   - Much like the changes to `str_starts_with()`, `str_ends_with()` and
 `str_contains()`, this provides brevity in the codebase, and per-instance
 has a much greater reduction in code.
   - It reduces our time reading and writing code - I appreciate there will
 be an adjustment period for some.
   - It provides a clear signal to prospective contributors and to
 extenders that WordPress is moving forward, encouraging participation and
 observation for future changes.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/58874>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform