[wp-trac] [WordPress Trac] #41057: Update PHP codebase per WordPress PHP Coding Standards

[WordPress Trac]

#41057: Update PHP codebase per WordPress PHP Coding Standards
----------------------------+-----------------------
 Reporter:  netweb          |       Owner:  pento
     Type:  task (blessed)  |      Status:  accepted
 Priority:  normal          |   Milestone:  4.9
Component:  General         |     Version:
 Severity:  normal          |  Resolution:
 Keywords:                  |     Focuses:
----------------------------+-----------------------

Comment (by pento):

 This is looking really good @jrf! There don't appear to be any major
 issues that I've run into so far, most of it is just tweaking.

 I've submitted a handful of reports for indentation issues to the WPCS
 repo: [https://github.com/WordPress-Coding-Standards/WordPress-Coding-
 Standards/issues/1118 WPCS#1118], [https://github.com/WordPress-Coding-
 Standards/WordPress-Coding-Standards/issues/1119 WPCS#1119],
 [https://github.com/WordPress-Coding-Standards/WordPress-Coding-
 Standards/issues/1120 WPCS#1120], [https://github.com/WordPress-Coding-
 Standards/WordPress-Coding-Standards/issues/1121 WPCS#1121].

 Array elements aren't aligning at the `=>`. It's mentioned (and shown in
 the examples) in the [https://make.wordpress.org/core/handbook/best-
 practices/coding-standards/php/#indentation Handbook], but I can make that
 more explicit if necessary. More
 [https://wordpress.slack.com/archives/C5VCTJGH3/p1504491187000089?thread_ts=1504483321.000002&cid=C5VCTJGH3
 info in Slack], and [https://github.com/WordPress-Coding-Standards
 /WordPress-Coding-Standards/issues/1122 WPCS#1122].

 What's the condition for splitting a long array declaration into multiple
 lines? The arrays at the top of `admin-ajax.php` and `class-wp-list-
 table.php` are split correctly, but those in `class-wp.php` are not.

 There are a couple of instances of a comment associated with the `while`
 of a `do/while` being indented, when it probably shouldn't be (the
 behaviour is not defined in the handbook), but I'm not worried about them
 - two instances in `compat.php` isn't worth getting excited about.

 The inline XML in `xmlrpc.php` is currently indented with spaces, at 2
 spaces per level. This isn't being correctly fixed - instead of replacing
 each 2 space indent with a tab, it's replacing groups of 4 spaces with a
 tab. Again, I'm not worried about this - we can manually fix it. A quick
 read `wp-includes/feed-*.php` shows all of them being indented correctly.

 `WP_Admin_Bar::_render_item()` probably needs to be rewritten, because
 that kind of abuse of white space collapsing is just not on. Conforming to
 the coding standards breaks the HTML output.

 Generally, I'm finding everything to be looking good, and much easier to
 read. The bits that are still hard to read are the long lines that we
 don't have a standard for splitting up. The usually fall into these
 categories:

 - Multiple conditions in an `if()`
 - Concatenating several strings together
 - Embedded HTML, usually with multiple attributes
 - HTML inside a string
 - DB queries in a string

 For the first two, I think we can just define a line length to split at,
 then they'll obey their relevant indent rules.

 For embedded HTML, I don't know if we can automatically split - adding
 whitespace between elements can cause layout bugs, but adding whitespace
 inside of elements (ie, putting element attributes on separate lines)
 won't.

 For the last two, I don't think we can sanely automatically fix them, but
 we could certainly throw a warning.

 I'm leaning towards using a modified version of the Docs line split rule -
 split as close to column 100 as possible, do not go over column 120. I'm
 open to arguments for or against this, the exact numbers to split at, and
 whether a split should cause all "units" in that line to split. For
 example:

 If you're splitting an `if` condition, should you split just at the split
 point, at every logical operator in the current scope, or at every logical
 operator?

 Given this example (and assuming the split point is at `$d`):

 `if ( $a || ( $b && $c ) || $d )`

 Which of these should it become?

 {{{
 if ( $a || ( $b && $c )
         || $d )
 }}}

 {{{
 if ( $a
         || ( $b && $c )
         || $d )
 }}}

 {{{
 if ( $a
         || ( $b
                 && $c )
         || $d )
 }}}

 Same thing for string concatenation. Split at every `.`, or just the ones
 we need to?

 And for HTML elements, should we split at every attribute, or just the
 ones we need to? Should we put newlines between all elements where there
 was previously no whitespace, just a specific set of elements, or should
 we throw a warning? (cc @peterwilsoncc for your opinion on adding
 whitespace, and what kind of layout issues that might cause.)

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