[wp-trac] [WordPress Trac] #51423: Fix function argument type issues reported by PHPStan

[WordPress Trac]

#51423: Fix function argument type issues reported by PHPStan
-------------------------------------------------+-------------------------
 Reporter:  SergeyBiryukov                       |       Owner:
                                                 |  hellofromTonya
     Type:  task (blessed)                       |      Status:  assigned
 Priority:  normal                               |   Milestone:  5.8
Component:  General                              |     Version:
 Severity:  normal                               |  Resolution:
 Keywords:  php8 has-patch has-unit-tests        |     Focuses:  coding-
  commit                                         |  standards
-------------------------------------------------+-------------------------

Comment (by SergeyBiryukov):

 Replying to [comment:94 hellofromTonya]:
 > How can the code guard and be robust, stable, and performant?
 >
 > If the edge case values are removed and the code guards itself while
 staying in context, it could check for `empty()` in the first conditional
 guard and skip the prep code altogether. Edgy case falsey values are
 converted into and empty string. In this context, that's okay as a string
 0 is not an email domain.
 >
 > PR is updated with this guarding pattern for the 3 instances.

 That looks better to me, thanks! As demonstrated by [attachment:51423
 .type-casting-vs-empty-check-test.php] over a million iterations, it is
 indeed about three times more performant than  unnecessary type casting:
 {{{
 type casting: 1.03507 seconds
 empty() & is_array() checks: 0.34766 seconds
 }}}

 However, one other consideration I have when making this kind of changes
 is whether the code can also be as concise as possible while remaining
 easy to understand and without a significant performance degradation.

 Since it's a relatively rarely accessed settings page and the value is
 only displayed once per page load and does not affect performance in a
 noticeable way, I think the performance difference is negligible here, so
 I would prefer a more compact code over more verbose (1 line vs. 6 lines).

 What do you think? If you feel strongly the performance difference, we can
 go with the `empty()` and `is_array()` checks as you suggest. Otherwise,
 keeping the type casting and just addressing the initial `str_replace()`
 issue seems like a good compromise.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/51423#comment:95>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform