Spaces around Control Structures do not appear to be checked

[yorkshire-pudding]

See https://docs.backdropcms.org/php-standards#control-structures

While the standard shows this:

if (condition1 || condition2) {
  action1;
}
elseif (condition3 && condition4) {
  action2;
}
else {
  defaultaction;
}

Doing this is currently not flagged:

if(condition1 || condition2){
  action1;
}elseif(condition3 && condition4){
  action2;
}else{
  defaultaction;
}

The standard says:

Control statements should have one space between the control keyword and opening parenthesis, to distinguish them from function calls.

But perhaps it also needs to explicitly state about the space between condition parentheses and the curly braces as shown in the example.

[indigoxela]

@yorkshire-pudding many thanks for reporting.

Our custom phpcs standard implementation is incomplete, as we had to start fresh to get compatible with phpcs 3.x. PRs are welcome. Co-maintainers, too. 😉

[yorkshire-pudding]

I had a look at the code, but I don't understand it (yet). I'd be happy to have a go at the PR but not sure how to approach it. I thought looking at Whitespace/ScopeClosingBraceSniff would be the best place to start, but working out how it is working is a bit beyond me.

[indigoxela]

We're using several generic rules for white space stuff, that already ship with phpcs.

https://github.com/backdrop-ops/phpcs/blob/main/Backdrop/ruleset.xml
It's possible to replace those with something custom, but that requires quite some effort (to understand and to implement).

Their definitions can be found in their repo (and other directories).

Personally, I ended up as maintainer of this repo here simply because I started working on our standard ruleset. And nobody else did. So please don't expect full expertise from my side. 😉

[yorkshire-pudding]

I wondered whether adding Squiz.Whitespace.ControlStructureSpacing would do it but it doesn't seem to make any difference. I can't workout what is actually looking for.

[yorkshire-pudding]

Also tried: Squiz.ControlStructures.ControlSignature and this seems to mostly work but it doesn't like our standard of putting else and elseif on a new line. It wants just a space.

I have done a PR using a modified version of this in the Backdrop spec but just removing:

        // Single space after closing brace.
        $found = 1;
        if ($tokens[($closer + 1)]['code'] !== T_WHITESPACE) {
            $found = 0;
        } else if ($tokens[$closer]['line'] !== $tokens[$stackPtr]['line']) {
            $found = 'newline';
        } else if ($tokens[($closer + 1)]['content'] !== ' ') {
            $found = $tokens[($closer + 1)]['length'];
        }

        if ($found !== 1) {
            $error = 'Expected 1 space after closing brace; %s found';
            $data  = [$found];

            if ($phpcsFile->findNext(Tokens::$commentTokens, ($closer + 1), $stackPtr) !== false) {
                // Comment found between closing brace and keyword, don't auto-fix.
                $phpcsFile->addError($error, $closer, 'SpaceAfterCloseBrace', $data);
                return;
            }

            $fix = $phpcsFile->addFixableError($error, $closer, 'SpaceAfterCloseBrace', $data);
            if ($fix === true) {
                if ($found === 0) {
                    $phpcsFile->fixer->addContent($closer, ' ');
                } else {
                    $phpcsFile->fixer->replaceToken(($closer + 1), ' ');
                }
            }
        }

[indigoxela]

Hey, that's quick progress here. 😄 I wonder, which (predefined) rules we currently use, this PR is able to replace.

[indigoxela]

I did some testing – this works great! 👍 And it doesn't seem to overlap with anything we have so far. 🎉

A tiny comment, and I'd love to merge this.

[yorkshire-pudding]

Done