[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