[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 hellofromTonya):

 Replying to [comment:93 SergeyBiryukov]:
 > My thinking when reading the current PR includes: "Checking for `null`?
 OK, but why would that value ever be `null`? Is that normal or some kind
 of back-compat? Do we need to account for that in other instances where
 it's used?" This kind of checks for specific edge cases could create
 confusion later and seem a bit hard to maintain the long run.

 Good point. Down the road, this could be confusing. As I read it now, it
 does cause me to stop when reading the code. Even with the comment, it
 causes one to stop and wonder.

 >Looking at this again, I'd like to make adjustments to my previous patch.
 Since we mostly care about taking an array of values here and converting
 it to a string, I think we can do exactly that, see 51423.wp-admin-
 network-settings.2.diff​.

 Interesting approach. It does retain the values. However, it's less
 performant, i.e. when falsey/empty does unnecessary type casting and
 replace/implode.

 >While I understand the intention to keep backward compatibility here, I
 think we should consider the context when making these changes and, if
 possible, write code that accounts for the intended values without being
 extra specific for edge cases or particular PHP versions.

 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.

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