Make WordPress Core

Opened 9 months ago

Last modified 6 months ago

#58948 new enhancement

Use Elvis operator (`:?`) to replace repeating ternary patterns (`a ? a : b`)

Reported by: ayeshrajans's profile ayeshrajans Owned by:
Milestone: Awaiting Review Priority: normal
Severity: trivial Version: 6.3
Component: General Keywords: has-patch close
Focuses: coding-standards Cc:

Description

This is a somewhat big change, but a simple one at that.

Replaces a ? a : b patterns of ternary statements to use the Elvis operator a ?: b.

Null coalescing operator (??) was added in PHP 7.0, which WordPress now requires as the minimum version. However, to make this ticket focused and easier to review, this ticket only proposes the Elvis operator.

On tainted lines, it follows WordPress CS to use uppercase constants (NULL instead of null, for example), and removes a few cases of unnecessary braces. Apart from them, the diff output should be quite minimal and straight forward.

Perhaps, the review will be easier with word-diff (git show --word-diff), or on GitHub, where it highlights word diffs in the line-diff mode.

Thank you.

Change History (7)

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


9 months ago
#1

This is a somewhat big change, but a simple one at that.

Replaces a ? a : b patterns of ternary statements to use the Elvis operator a ?: b.

Null coalescing operator (??) was added in PHP 7.0, which WordPress now requires as the minimum version. However, to make this ticket focused and easier to review, this ticket only proposes the Elvis operator.

On tainted lines, it follows WordPress CS to use uppercase constants (NULL instead of null, for example), and removes a few cases of unnecessary braces. Apart from them, the diff output should be quite minimal and straight forward.

Perhaps, the review will be easier with word-diff (git show --word-diff), or on GitHub, where it highlights word diffs in the line-diff mode.

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

#2 follow-up: @ayeshrajans
9 months ago

I wasn't aware of this before, but it looks like wpcs disallows Elvis operator at the moment. I will read the issues/PRs in https://github.com/WordPress/WordPress-Coding-Standards to see if there is a good reason for it, or try see if we get them allowed. PHPCS task fails otherwise.

#3 @swissspidy
9 months ago

  • Focuses performance removed

A make/core blog post proposal is currently being worked on to talk about adopting new syntax. I suggest waiting for that to be published.

#4 in reply to: ↑ 2 @jrf
9 months ago

Replying to ayeshrajans:

I wasn't aware of this before, but it looks like wpcs disallows Elvis operator at the moment. I will read the issues/PRs in https://github.com/WordPress/WordPress-Coding-Standards to see if there is a good reason for it, or try see if we get them allowed. PHPCS task fails otherwise.

Just checking if something is "truthy" (while used a LOT in WP), is generally speaking not the best of practices and as soon as a check is made more specific - isset( $a ) && is_int( $a ) && $a > 10 - a short ternary will break.

That's why it is forbidden and there is no intention to change this rule.

#5 in reply to: ↑ description @SergeyBiryukov
9 months ago

Replying to ayeshrajans:

On tainted lines, it follows WordPress CS to use uppercase constants (NULL instead of null, for example)

I believe the standards actually recommend lower case here, as seen on the PR:

Error: TRUE, FALSE and NULL must be lowercase; expected "null" but found "NULL"

See [19687] / #16302 for historical reference.

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


7 months ago

#7 @jrf
6 months ago

  • Keywords close added

I'm going to suggest closing this ticket as introducing these changes is against the WP Coding Standards.

Note: See TracTickets for help on using tickets.