[wp-trac] [WordPress Trac] #61648: WP_Debug_Data::debug_data() is overly complex

[WordPress Trac]

#61648: WP_Debug_Data::debug_data() is overly complex
-------------------------+-----------------------------
 Reporter:  apermo       |      Owner:  (none)
     Type:  enhancement  |     Status:  new
 Priority:  normal       |  Milestone:  Awaiting Review
Component:  Site Health  |    Version:  trunk
 Severity:  normal       |   Keywords:  has-patch
  Focuses:               |
-------------------------+-----------------------------
 The function {{{WP_Debug_Data::debug_data()}}} starts at line 37 and ends
 at line 1490, this does not yet include the 13 lines of phpDoc above
 itself, which results in a total of 1466 lines including documentation for
 a single function.

 This breaks with so many programming best practices. And even though I
 know the rule "Don't refactor, just because you can" I do think, that this
 one needed refactoring.

 The function does generate the complete output at /wp-admin/site-
 health.php?tab=debug, including all 12(13) accordions.

 As of writing this ticket, I have already refactored the function, so I
 can at this moment already point out some design flaws.

 Most noticeable is a hidden bug inside the function at line 1302. For
 documentation reasons I will create an additional trac ticket for this.
 But basically in line 1302 the value of
 {{{$parent_theme_auto_update_string}}} is overwritten by the content of
 {{{$auto_updates_string}}}, which was likely never found due to two
 reasons, it barely changed the result and due to the massive complexity of
 the function, no one ever ran into it by accident.

 Also the structure of the whole function is counter intuitive. In some
 cases the label and structure is created first and a several/hundreds
 lines later the fields will be populated.

 In other cases all is done in a single place.

 This makes extending the function within core already quite challenging.
 From what I've researched, this code grew inside a featured plugin for 2
 years before it was copied into WordPress core, without any noticeable
 refactoring. And after that, the number of lines and it's complexity grew
 by a few hundred lines.

 Another thing, which the current filter does not easily provide. If you
 want to inject own debug content through the filter
 {{{'debug_information'}}} you will either append your debug information to
 the end, or you are force to manually restructure the array, which could
 potentially create conflicts if plugin authors do not pay attention here.
 Having one author doing a poor job here, could potentially mean, that
 debug informations by others could be removed by the attempt to insert
 his/her content at a precise place.

 I am aware that this would be the plugin authors fault, but by using the
 filter from the beginning to inject all core debug information, will
 provide a safe, easy and robust way, for authors to inject their debug
 information at other places than at the very end.

 Due to the massive size of the function, it is absolutely impossible to
 ever write any reasonable unit test for this.

 In my PR I have done the following things:

 * Bumped compatibility to 7.2 by adding types where suitable to the file.
 * Fixed the mentioned bug
 * Refactored code to distinct function, so that nearly all accordions are
 handled by a single function. Only exception to this rule is
 active/inactive plugins, since these are heavily tied together.
 * using {{{add_filter( 'debug_information', CALLBACK );}}} to populate the
 debug information array, which provides a robust way to inject contents at
 places other than the end.

-- 
Ticket URL: <https://core.trac.wordpress.org/ticket/61648>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform