[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:92 hellofromTonya]:
 > Checking for truthy would result in different value if the the returned
 filtered option was a string 0 or 0.0. [https://3v4l.org/oYFA8 See it in
 action here]. I know that's a very very very far edge case. But here we
 were seeking to not change the existing behavior for backwards
 compatibility for any 3rd parties that are filtering the option.

 Thanks for the additional details! 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.

 We already have a truthy check here, so I think we can just move the
 `str_replace()` call into it.

 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.

 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 [attachment:"51423.wp-
 admin-network-settings.2.diff"].

 Here's the current code: https://3v4l.org/kmiZo
 Here's the proposed code: https://3v4l.org/VTVcF

 As you can see, the results almost exactly the same, except for an empty
 array, where the former code inadvertently passes an array instead of a
 string further down, so the proposed code fixes that as well.

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