[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