Spacing for Return Types

[GaryJones]

For PHP 7, the format for adding return types appears to be of the format:

function foo( $arg ): array {

This format of characters and whitespace between ) and { can be seen in examples on:

• PHP Manual
• PHP Return Type RFC
• Draft PSR-12
• SitePoint example.
• This blog post
• And this blog post (which has no space after the return type.)

Just for a counter-argument, this post puts a space before the :.

Regardless, the current WPCS flags up ): as Expected one space after closing parenthesis. It doesn't flag up no space between the return type and {.

We, as WPCS, either need to make a decision that ): array { is correct for the WordPress community, or that particular combination of characters / tokens should be excluded from whitespace checks until TPTB have updated the coding standards.

[JDGrimes]

It seems pretty clear-cut to me that there should be a space before the opening bracket. I think it wold be really inconsistent for WP not to adopt that (54 years from now :-).

As far as the space after the closing parenthesis, I think we should just allow it to be configured either way, but not flag it by default.

[JDGrimes]

This error partly comes from an upstream sniff:

 3 | ERROR | [x] Expected 1 space after closing parenthesis; found 0
   |       |     (Generic.Functions.OpeningFunctionBraceKernighanRitchie.SpaceAfterBracket)

So partly this is because PHPCS doesn't fully support PHP 7 yet, I think. Although I also get this from our sniffs:

 3 | ERROR | [x] Space between opening control structure and closing
   |       |     parenthesis is required
   |       |     (WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterCloseParenthesis)

Also, it looks like WordPress.Arrays.ArrayDeclaration isn't compatible with PHP 7:

 1 | ERROR | [x] Expected 1 space after array opener, found 1.
   |       |     (WordPress.Arrays.ArrayDeclaration.SpaceAfterArrayOpener)
 1 | ERROR | [x] Missing space before array closer.
   |       |     (WordPress.Arrays.ArrayDeclaration.NoSpaceAfterOpenParenthesis)
 3 | ERROR | [x] There must be no space between the "array" keyword and
   |       |     the opening parenthesis
   |       |     (WordPress.Arrays.ArrayDeclaration.SpaceAfterKeyword)

[GaryJones]

Most of the rest of the PHP World uses no space for return types, but the WP historical code style would suggest using a space.

Always put spaces after commas, and on both sides of logical, comparison, string and assignment operators.
-- https://make.wordpress.org/core/handbook/best-practices/coding-standards/php/#space-usage

My preference, is that for something "relatively" new (and not yet found in WP core, or many plugins) that there's a good case for sticking with no-space:

• default auto-formatting by IDEs just works - many may not even have an option to include a space there,
• examples in WP and PHP tutorials match,
• possibly able to inherit a sniff from an existing standard to check this, instead of having to write a current one (which in turn means it could be added to a ruleset far quicker and adopted consistently in the community).

In terms of the space before the : for the return type, the Handbook explicitly says that the case in a switch block must have no space before the colon (and likewise for the CSS standards), so there is prior-art in WP core for having no space before colons (as well as the rest of the PHP community).

---

Along the same lines and with exactly the same arguments, there is also:

declare(strict_types=1);

declare( strict_types = 1 );

As well as the points I mentioned above for keeping with no-space, the extra argument here is that this isn't going to vary massively - there's only something like three possible values to go inside declare(), so it is very much a boilerplate flag, rather than logic that needs readability.

declare( strict_types = 0 ) would be redundant AFAIK, so the only realistic value is 1. That, therefore, doesn't make it logic in the intent that the existing Handbook sentence was written; the logic is "is there statement there or not", not what the parts of it are.

It's also always going to be at the top of the file, probably with a clear line space before any namespace or use statements, well away from any other blocks of code, so I can't see that readability is an issue.

I agree that exceptions make for a bad approach to writing standards.

But then, the existing Handbook already has inconsistencies. Space usage of ['a'] vs [ $a ], lack of spaces around type-casts like (array), trailing comma usage of single-line vs multi-line arrays etc. All of them are in the middle of business logic. What I'm referring to is a declare statement at the top of a file.

[pento]

For return types, no space before the colon. When colon is used as a separator between different things (in the ternary operator, for example), there should be spaces. When it's used as a "here is something related to the thing I'm coming after", (case statements and return types), there should be no space before, one space after when they're on the same line.

The other obvious use case of colon is as an alternative to {} on if statements, and loops - I would very much like to deprecate this usage at some point, so I don't think it's necessary to consider.

For declare(), use spaces. While it's usually used at the top of the file, that's not always the case - particularly with the ticks directive, it's not uncommon to see that changed at various points within the code.

[jrfnl]

My preference, is that for something "relatively" new (and not yet found in WP core, or many plugins) that there's a good case for sticking with no-space:

👍 for consistency with prior art here.

For declare(), use spaces.

As - for now - it is rarely used (encoding introduced in PHP 5.3.0, strict_types in PHP 7.0, only ticks available in PHP 5.2), I'd just as easily leave that one alone for now until a time when it becomes more relevant.

Personally I'd prefer no spaces as:

• in sync with examples everywhere
• distinguishes it from function calls - a declare statement is an execution directive

[GaryJones]

it is rarely used

ticks and encoding have not been used much, but the availability on strongly-typed behaviour in PHP 7 is only going to increase (strict types, scalar type declarations, return types etc.) as we move forward. Right now, all plugins and client work my team and I do is PHP 7.0+, and all files should be getting strict_types as they are worked on.

particularly with the ticks directive, it's not uncommon to see that changed at various points within the code.

I can't see it in core, and it's not something I've seen much use of at all - do ticks get much use in a WordPress environment? All the reasons for sticking with no-spaces vs the slight chance someone might be doing something with ticks + historical code style of function calls (which are different), seems a simple conclusion.

[GaryJones]

For return types, no space before the colon.

@pento Yay :-) Can that be added to the Handbook then, please?

[pento]

I can't see it in core, and it's not something I've seen much use of at all - do ticks get much use in a WordPress environment?

They're used a bit internally on WordPress.com, and always in the context of a ::tick_on(), ::tick_off() helper method, so they can be more readily controlled.

Can that be added to the Handbook then, please?

👍🏻

[tfrommen]

I just ran into the issue with an expected space before the return type colon, which I don't want to be there.

So, did I understand correctly that this will be fixed sometime soon?

For now, I will just exclude the according message:

<exclude name="WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterCloseParenthesis"/>

[jrfnl]

Reopening as - while PR #1421 fixes the false positive previously encountered -, there currently isn't any check in place for the desired spacing.

[dac514]

The rule is (currently) inconsistent.

I get:

Space between opening control structure and closing parenthesis is required
(WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterCloseParenthesis)

For:

function test2( array $foo ): \Generator {
    yield 1;
}

But no complaints for the exact same code without a function parameter:

function test1(): \Generator {
    yield 1;
}

[GaryJones]

I've experienced this as well, though I thought we fixed it.

@connerbw are you on the latest release of WPCS?

[dac514]

Sorry, we're on "wp-coding-standards/wpcs": "^0.14.0",
See: https://github.com/humanmade/coding-standards
I'll open the bug there.