Prefer strict PHP comparisons over loose PHP comparisons

[pikamander2]

My understanding is that it's usually better to use strict comparisons (===, !==) rather than loose comparisons (==, !=) because they're more explicit and therefore less prone to silly edge case bugs.

Although it wouldn't affect the logic of the code in some cases, it could still be helpful for knowing that no implicit type conversion is happening and therefore making certain things faster to debug. Plus, the process of converting loose comparisons might reveal some dormant bugs in the code that would have been difficult to find otherwise.

So, here are some examples of what I'm proposing:

Change this:

if ( $url && $url != network_home_url( , 'http' ) ) :

to this:

if ( $url && $url !== network_home_url( , 'http' ) ) :

And this:

if ( $signup->domain . $signup->path == ) {

To this:

if ( $signup->domain . $signup->path === ) {

And this:

if ( SITECOOKIEPATH != COOKIEPATH ) {

to this:

if ( SITECOOKIEPATH !== COOKIEPATH ) {

I'd be willing to do a lot of the conversions myself, but first I wanted to see if that's something that we're interested in.

[pikamander2]

Here are some past cases where converting loose comparisons (and using yoda conditions) seems to have been accepted:

• #46834
• #47201
• #46508

Just wanted to check if that's still desired and what all I should watch out for while doing so.

[pikamander2]

Use strict comparisons and yoda conditions

[pikamander2]

Use strict comparisons and yoda conditions

[pikamander2]

Use strict comparisons and yoda conditions

[pikamander2]

Use strict comparisons and yoda conditions

[pikamander2]

Use strict comparisons and yoda conditions

[pikamander2]

Use strict comparisons and yoda conditions

[pikamander2]

@SergeyBiryukov - Here are some strict comparison and yoda condition updates to the WordPress PHP files in the root directory.

[pikamander2]

@SergeyBiryukov @garrett-eclipse - Any updates on this? I can go through more files if these changes look good.

[SergeyBiryukov]

Just noting that wp-comments-post.php is already handled in [47028] / #49105.

[SergeyBiryukov]

In 47054:

Coding Standards: Use strict comparison and Yoda conditions in the root directory files.

Props pikamander2.
Fixes #48965.

[Otto42]

Possible backwards compatibility break on this:

​https://wordpress.org/support/topic/5-4-sending-post-via-email-results-in-no-title-for-the-post-title/

Specifically line 213 in wp-mail.php, where it detects a blank title. Seems that it wasn't really an empty string in some cases, but depended on it being empty enough.

[pikamander2]

@Otto42 - Yeah, that's not too surprising. My biggest fear with pushing these changes is that they might break production code.

On the flip side though, that's exactly why these kinds of changes are needed. Loose comparisons allow for a lot of poorly-defined behavior and weird interactions. The more comparisons we convert to strict, the easier it becomes to see what the function is expecting and can handle.

Pinging @SergeyBiryukov for thoughts.

[pikamander2]

I guess the simple fix here would be something like this:

<?php
if ( true === empty($post_title) )

@Otto42 - When applied to wp-mail.php, does that code fix your problem?

[Otto42]

I don't use wp-mail myself, I was just relaying from that support thread. You might consider asking that guy in the forum to change the code in his file to see if that solves his problem. If so, then it could be a reasonable fix for .1.

Edit: Also, I would just go with if ( empty($post_title) ) instead.

[MattyRob]

This is because $post_title = xmlrpc_getposttitle( $content ); is returning NULL instead of an empty string which is therefore failing the strict comparison as newly implemented.

Perhaps in this instance it may be better to use:
if ( empty( $post_title ) ) {

Instead of:
if ( '' === $post_title ) {

THat's seem to work in my basic testing.

[MattyRob]

Actually, thinking again using empty() would not allow for a post title of '0' (as in zero).

Perhaps we need a better check or revert the strict check

if ( '' === (string) $post_title ) {

[SergeyBiryukov]

Thanks everyone for highlighting the issue!

Since this ticket was closed on a completed milestone, I've created a new one for 5.4.1: #49853. Let's continue the discussion there.