Replace/remove the WordPress.WhiteSpace.ControlStructureSpacing sniff

[jrfnl]

The ControlStructureSpacing (after the removal of the function handling in #2328) is largely a copy of an upstream sniff (Squiz.WhiteSpace.ControlStructureSpacing) to allow for 1 space on the inside of parentheses instead of 0.

Historic context: this is one of the oldest sniff in WPCS and at the time it was added, not much else was available for control structures upstream, what was available wasn't very configurable, and it wasn't yet possible to include just one or two error codes from a particular sniff.

Since then, more control structure related sniffs have become available upstream, including some which are more configurable.
Additionally, it is now easier than it was then to include just one or two error codes from a sniff.

As there is nothing WordPress specific about spacing rules for control structures, it will lower the future maintenance burden to remove the sniff and defer to upstream sniffs to check control structures instead.

For example: most, if not all, upstream sniffs also already include support for "new" control structure related keywords like finally and match, while the WPCS native sniff does not (though that will be added in 3.0.0 via an upcoming PR).

What does the sniff do ?

The WPCS native sniff currently checks the following:

• Require one space after the control structure keyword.
  Note: there is duplication here with the Squiz.ControlStructures.ControlSignature sniff, which is also included in the WordPress-Core ruleset.
• For alternative control structure syntax: one space before the : acting as the scope opener. (configurable to forbid the space).
• Require one space before the open parenthesis (has overlap with the keyword spacing check).
• Require one space after the open parenthesis, but allows a new line for multi-line conditions.
• Require one space before the close parenthesis, but allows for a new line for multi-line conditions.
• Require one space between the close parenthesis and the scope opener.
  Note: here is another duplication with the Squiz.ControlStructures.ControlSignature sniff.
• Require the control structure open brace to be on the same line as the keyword for single-line conditions.
• [Optional, turned OFF by default] Forbid a blank line at the start of the control structure body.
• [Optional, turned OFF by default] Forbid a blank line at the end of the control structure body.
• [Optional, turned ON by default] Require a blank line after the end of the control structure unless the control structure end is followed by the start (i.e. if - else) or end of another scoped structure.

What does the sniff not do ? (but are things which are checks via other sniffs)

• Require new line after open brace, i.e. body starts on the line after the keyword - Squiz.ControlStructures.ControlSignature.
• Require elseif, not else if - PSR2.ControlStructures.ElseIfDeclaration.
• Require one space after close brace when followed by a related control structure (i.e. if - else) - Squiz.ControlStructures.ControlSignature.
• Check there is no space between the scope closer of a match control structure and the semi-colon - Squiz.WhiteSpace.SemicolonSpacing.

What does the sniff not do ? (but are things we should possible (probably) add checks for)

• Check the placement of first condition in a multi-line condition. Should this be on the line containing the keyword (PSR2/PEAR) or should this be on the line after (PSR12) ?
• Check the indent of conditions in a multi-line condition. Should be 1 tab (4 spaces) from the keyword indent, but is not checked.
• Check the placement of the close parenthesis for multi-line conditions. The "normalized" way of doing this is to have the close parenthesis on a new line, indented the same as the control structure keyword, but this is not checked and usage in WP Core is very inconsistent.
• Check for consistency in the placement of boolean operators in multi-line conditions.
  For readability, always having the operator either at the end of the line or the start of the line helps.
  For keeping divs limited when conditions change, the start of the line is probably the best choice.
  At the very least, having this consistent within the same control structure would already be an improvement.

Some of this was already proposed in the March 2020 Make post (section: "Complex control structure conditions"), but this didn't gain enough consensus.

What sniffs are available upstream ?

• Squiz.ControlStructures.ControlSignature - already in use.
• Squiz.WhiteSpace.ControlStructureSpacing - WPCS native sniff is close duplicate of this sniff.
• PSR2.ControlStructures.ControlStructureSpacing - checks spacing on inside of parentheses only, configurable to 1 space, but will enforce first condition on same line as keyword.
• PSR12.ControlStructures.ControlStructureSpacing - checks for spacing issue between the parentheses only; checks single line conditions via the PSR2 sniff, but doesn't allow it to be configured to enforce 1 space; check condition placement, indent, close parenthesis placement natively, but enforces first condition on next line from keyword.
• PEAR.ControlStructures.MultiLineCondition - largely the same as the PSR12 sniff, but enforces first condition to be on the same line as the keyword + allows for enforcing boolean operator to always be at the start of the line.
• PEAR.ControlStructures.ControlSignature - checks spacing of the keyword and parenthesis only, doesn't check try/catch/finally.
• PSR12.Controlstructures.BooleanOperatorPlacement - allows for enforcing consistent placement of boolean operators, whether this is per control structure or always "start of line"/"end of line" is configurable.

Why not do this now ?

Initial test runs against WP Core show that replacing the sniff with upstream sniffs would have a bigger impact than expected and that the code style used for control structures in WP Core is pretty inconsistent.

To make it more consistent, decisions need to be taken on what rules should be enforced. As we're close to the WPCS 3.0.0 release, now is not the time to do this.

Action plan

• Discuss in this ticket what rules we want to propose to make the formatting of control structures more consistent.
• Write and publish a Make post with such a proposal to gather community opinions and gain consensus of the wider community.
• If consensus can be reached:
  • Deprecate the WPCS native sniff.
  • Add upstream sniffs to replace its functionality and potentially add additional checks.
  • Make sure the deprecation notice is silenced for the WPCS native rulesets.
  • In a future major release, remove the WPCS native sniff.