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

WordPress Trac noreply at wordpress.org
Mon Apr 12 21:02:12 UTC 2021


#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:90 hellofromTonya]:
 > Marking [https://github.com/WordPress/wordpress-develop/pull/1100 PR
 1100] ready for commit. cc @SergeyBiryukov.

 Thanks for the PR!

 * I don't see how `get_site_option( 'limited_email_domains' )` can return
 `null` in normal workflow, so it seems a bit random to specifically guard
 against `null` here. Even if the option does not exist, it returns
 `false`. So it looks like the only way for that to happen is if someone
 specifically filters the value to `null`. Instead of targeting `null`, I
 think the existing check for a truthy value would be enough here.
 * That said, I agree with some other points of the PR, we can check for
 `is_array()` before calling `implode()`. We should also do it consistently
 for the `banned_email_domains` and `illegal_names` options, though.
 * The `str_replace()` call was added in
 [https://mu.trac.wordpress.org/changeset/1285 mu:1285] /
 [https://mu.trac.wordpress.org/ticket/426 mu:#426] when converting
 `limited_email_domains` from an input field to a textarea. This would
 benefit from a comment to avoid any future confusion of why it is applied
 for `limited_email_domains`, but not for `banned_email_domains`.

 So something like [attachment:51423.wp-admin-network-settings.diff] would
 be my suggestion here.

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


More information about the wp-trac mailing list