[wp-trac] [WordPress Trac] #57427: WP_Locale doesn't initialize property arrays before using them
WordPress Trac
noreply at wordpress.org
Mon Jan 9 08:41:37 UTC 2023
#57427: WP_Locale doesn't initialize property arrays before using them
--------------------------+---------------------
Reporter: tyxla | Owner: (none)
Type: defect (bug) | Status: new
Priority: normal | Milestone: 6.2
Component: I18N | Version: 6.1
Severity: normal | Resolution:
Keywords: has-patch | Focuses:
--------------------------+---------------------
Comment (by tyxla):
I can't disagree with all the great points brought here.
> On the other hand I'm +1 for the patch/PR as marking something as array
in the docblock but then setting it to null is a pretty bad practice. Why
use the wrong type? There should be another way to indicate improper
usage.
This summarizes what I would have replied to your comment. We can talk
about load order, legacy usages, how the current architecture came to be,
obfuscating any improper usage, and so on, but the issue that this PR
addresses is in the very foundation and has nothing to do with all of
that. It addresses a type issue, but also a clear violation of the PHP
syntax, within the boundaries of the class. It also provides more safety
and makes that class future-proof in case any future refactoring causes a
difference in load or execution order.
> Hmmm, but it is not a module? It depends on other parts of WP, is not
designed to be self-contained, it is impossible to be used "stand alone",
and there is no reason to use it as such.
I see, and I think that's one of the problems with the original WordPress
architecture in general. Things often tend to be too tightly coupled with
each other, and not always for a good reason. I see your point about why
it's not supposed to be used standalone, but then why do we even have that
functionality abstracted to its separate class? Just for keeping things
tidy? I think that even makes things more confusing, as we fail to keep
the classes self-sufficient and to follow the single-responsibility
principle. I understand that there are historical reasons for how things
are right now, and that's why this PR suggests minimal changes at no real
cost.
I'm happy to let this go, but at this point, I am not convinced there are
any real downsides to landing it.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/57427#comment:7>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list