Fixing the non yoda conditions in ms-functions.php

[debarghyabanerjee]

There are two functions inside ms-functions.php, where if check doesn't have Yoda condition check implemented and also strict comparison is not added.

The two functions are:

1. newuser_notify_siteadmin();
2. newblog_notify_siteadmin();

The if check is as follows:

<?php
if ( is_email( $email ) == false ) {
    return false;
}

Trac Ticket: Core-63032

## Problem Statement:

• In the ms-functions.php file, there are two functions — newuser_notify_siteadmin() and newblog_notify_siteadmin() —that perform email validation using if statements without employing Yoda conditions or strict comparisons. This can potentially lead to issues with robustness and consistency in the validation logic.

## Fix:

• To enhance the code quality and align with WordPress coding standards, the following updates have been made:

if ( ! is_email( $email ) )

[audrasjb]

Thanks for opening this ticket and reporting the issue.
Moving to 6.7.

[SergeyBiryukov]

Hi there, welcome back to WordPress Trac!

Looks good, and brings some consistency with other is_email() checks in core.

[jrf]

Note: neither of these conditions would need yoda conditions as there is no comparison with a variable.

And while I would much prefer to change this to a strict === false, I agree that considering the return type of is_email() is string|false AND that the return value of the function is filterable, changing this to the loosy ! ... is the more WordPressy thing to do.

Having said that, it would be better to improve the condition to safeguard the code after it more properly and change it to something like:

<?php
$is_email = is_email( $email );
if ( ! is_string( $is_email ) || empty ( $is_email)  ) {

[debarghyabanerjee]

Hi @jrf, I was going through the core's codebase and found that in most of the places,

<?php
if ( ! is_email( $email ) ) {

is used, so in my opinion we should keep this to keep the check consistent across the codebase, however I can change the checks as per your suggestion, please let me know, what are your thoughts on this. Thanks.

[davidbaumwald]

With 6.7 Beta 1 releasing in a few hours, this is being moved to 6.8 given recent momentum towards a resolution.

@audrasjb If you or any committer feels the remaining work can be resolved in time for Beta 1 and wishes to assume ownership, feel free to pull this back into the milestone.