"No space after opening parenthesis is prohibited" when using traits

[felixarntz]

I'm using traits in my code, and obviously WordPress doesn't. :)

The sniffer rules give give me the following error:
No space after opening parenthesis is prohibited

This happens for every line with a use statement for a trait, where I need to resolve a method conflict. For example:

use Meta_Submodule_Trait, Settings_Submodule_Trait {
    Meta_Submodule_Trait::get_meta_fields as protected _get_meta_fields;
}

(in the above example the error is always thrown for the first line)

This is valid PHP, but causes an issue, probably because the codesniffer hasn't yet considered such cases. Not sure whether I should just disable this, or whether the sniff could be adjusted.

[jrfnl]

Related to #764.

@felixarntz Could you give me a more complete code sample ? The current one gives a syntax error so is not a valid test case.

[felixarntz]

This is basically what the class looks like:

class Access_Control extends Submodule implements Meta_Submodule_Interface, Settings_Submodule_Interface {
    use Meta_Submodule_Trait, Settings_Submodule_Trait {
        Meta_Submodule_Trait::get_meta_fields as protected _get_meta_fields;
    }

    [functions etc]
}

[jrfnl]

@felixarntz Ok, thanks. I've been able to reproduce the issue.

For the short term, I'd suggest disabling that particular error message using:

<!-- Disable it selectively for a few files: -->
<rule ref="WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterOpenParenthesis">
	<exclude-pattern>*/something*.php</exclude-pattern>
</rule>

<!-- Or disable it completely: -->
<exclude name="WordPress.WhiteSpace.ControlStructureSpacing.NoSpaceAfterOpenParenthesis"/>

For the longer term, I think we need to create a dedicated sniff for use statement whitespace as there are so many variations of how use statements can be laid out because of its different uses, that the WhiteSpace.ControlStructure sniff which currently handles this, is not really suited for handling this.

As input for the dedicated sniff - it should handle all cases, i.e.:

1. Import/alias classes, traits, constants, etc. in namespaces,
2. Insert traits in classes,
3. Inherit variables in closures.

And let's not forget the variations within that with use function / use const / group use statements etc.

Refs:

• http://php.net/manual/en/language.namespaces.importing.php
• http://php.net/manual/en/language.oop5.traits.php
• http://php.net/manual/en/functions.anonymous.php

[jrfnl]

Ok, I've been gathering my thoughts around this sniff and there's quite some things to check for by the looks of it.

So far I've come up with the following "rules":

Generic:

• There should be exactly one space between use, function, const keywords and the start of the classname.
• If an alias is given, there should be exactly one space before and one space after the as keyword and the alias should be on the same line as the "thing" being aliased.
• If private, public, protected or final keywords are used, again surrounded by exactly one space (except when they are directly followed by a semi-colon).
• Multi-use statements: no space before the comma, exactly one space or a new line after.
• Multi-use statement: subsequent items should be indented one tab in from the use keyword.

Namespace use statements:

• Class names should not start with a \ as that is a given.
• Group use statements:
  • There should be no space between the namespace separator and the brace.
  • There should be exactly one space or a new line after the open brace.
  • There should be exactly one space or a new line before the close brace.
  • Should be either truly single line or truly multi-line, i.e.:
    Single line: all items on one line and the open and close brace on that same line.
    Multi-line:
    • Open brace on same line as use keyword, each item and the close brace on their own line.
    • Close brace aligned with the use keyword.
    • Items indented one tab in from the use keyword.
  • Trailing comma's are allowed after the last item in a group use statement. (PHP 7.2)

use My\Full\NSname;
use My\Full\Classname as Another;
use function My\Full\functionName as func;
use const My\Full\CONSTANT;

use My\Full\Classname as Another,
	My\Full\NSname;

use some\namespace\{ ClassA, ClassB, ClassC as C };
use some\namespace\{
	ClassA,
	ClassB,
	ClassC as C
};

Closure use statements:

• There should be exactly one space between the function close parenthesis and the use keyword.
• There should be exactly one space between the use statement and the open parenthesis.
• There should be exactly one space on the inside of each of the use statement parenthesis.
• There should be exactly one space between the close parenthesis and the open brace.

function ( $quantity, $product ) use ( $tax, &$total ) {};

Trait use statements:

• There should be exactly one space before the open brace.
• Other rules around single/multi-line same as for namespaces.
• There should be exactly one space before and after the insteadof keyword.

class MyHelloWorld {
	use \Hello, World;
}

class MyHelloWorld {
	use \Hello,
		World;
}

class MyClass1 {
	use \HelloWorld { sayHello as protected; }
}

class MyClass2 {
	use HelloWorld {
		sayHello as private myPrivateHello;
	}
}

class SomeThing {
	use Meta_Submodule_Trait, \Settings_Submodule_Trait {
		Meta_Submodule_Trait::get_meta_fields as protected _get_meta_fields;
	}
}

class Aliased_Talker {
	use A, B {
		B::smallTalk insteadof A;
		A::bigTalk insteadof B;
		B::bigTalk as talk;
	}
}

class Foo {
	use A, B, C {
		C::bar insteadof A, B;
	}
}

@WordPress-Coding-Standards/wpcs-collaborators Opinions ?

[jrfnl]

One more - for keyword which are not identified as such [*] and therefore don't trigger the Generic.PHP.Lowercasekeywords sniff: all keywords should be lowercase.

[*] PHPCS changes the function and const keywords to T_STRING to prevent sniffs looking for T_FUNCTION and T_CONST from triggering on these keywords when used in use statements.

[GaryJones]

I like most of it, but I wonder if some of the subtle variants described in https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md (Draft) might be worth using?

i.e. No spaces inside group use statements, traits on single line only, prescribed order of where each of these file-top items should go, etc.?

[jrfnl]

@GaryJones

I like most of it, but I wonder if some of the subtle variants described in https://github.com/php-fig/fig-standards/blob/master/proposed/extended-coding-style-guide.md (Draft) might be worth using?

Just been reading through it. Except for one of two things regarding spacing inside parenthesis and braces, it looks to be mostly in line with what I wrote above already.

What I wrote down above is largely based on making things consistent with other WP rules regarding:

• space usage
• indentation similar to arrays

Regarding your suggestions (sorry for the lack of links, but the markdown of the fig doc does not allow for easy linking to sections - php-fig/fig-standards#911):

i.e. No spaces inside group use statements,

What do you mean by that ? I can't find that anywhere in the document. Or do you mean: no blank lines ?

Or, alternatively, do you mean: no spaces inside the parenthesis for closure use statement ? In that case, that seems completely inconsistent with other parenthesis use in WP.

traits on single line only,

That seems counter-intuitive and inconsistent to me, especially when you look at the next example for importing traits and using as and insteadof. For this sniff, I'd rather not force it either way at this moment.

prescribed order of where each of these file-top items should go, etc.?

I like the "class use statement order" part - is that what you mean ? but am not sure if this would not go a bit too far for WP at this moment.

One or more class-based use import statements.
One or more function-based use import statements.
One or more constant-based use import statements.

I also like the blank line directive:

each of the blocks below (in this comment above) MUST be separated by a single blank line, and MUST NOT contain a blank line

This does seem like something which should be handled by a separate sniff though as the re-ordering of statements is quite complex. Maybe add this as a separate sniff with auto-fixer, but have everything be warnings, not errors ? And maybe not even add it to a ruleset, but make it opt-in (though it will ,of course, automatically be added to the WordPress ruleset)

For closures, I also like this part:

Argument lists and variable lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument or variable per line.

When the ending list (whether of arguments or variables) is split across multiple lines, the closing parenthesis and opening brace MUST be placed together on their own line with one space between them.

That's basically the same principle as outlined above for group use statements, but for multi-line closure opening declarations.

[JDGrimes]

I like the rules @jrfnl has outlined.

In regard to what @GaryJones said:

No spaces inside group use statements

use some\namespace\{ClassA, ClassB, ClassC as C};

No spaces inside the braces of the group use statements for namespaces does not feel right. In fact, it wouldn't feel right to me having a single-line group use statement at all. I'd probably always split it across multiple lines, because the braces feel like a control structure, and that is the rules for control structures.

// I wouldn't do this:
use some\namespace\{ ClassA, ClassB, ClassC as C };

// Because I wouldn't do this:
if ( $var ) { return false; }

They are two different things of course, but it just feels inconsistent. Same thing here:

// I wouldn't do this:
class MyClass1 {
	use \HelloWorld { sayHello as protected; }
}

// I'd do this:
class MyClass1 {
	use \HelloWorld { 
		sayHello as protected; 
	}
}

[GaryJones]

I'm easily swayed by the counter offers, since PSR-12 is, at best, draft, and even if it did make it through to deprecated PSR-2, it would still require take-up across the PHP community for it to be worthwhile.

file-top items

I meant this bit:

Each block MUST be in the order listed below, although blocks that are not relevant may be omitted.

• File-level docblock.
• One or more declare statements.
• The namespace declaration of the file.
• One or more class-based use import statements.
• One or more function-based use import statements.
• One or more constant-based use import statements.
• The remainder of the code in the file.

I can't imagine that function-based and constant-based use statements are that prevalent amongst plugins and themes, to the point where giving a warning / error for an incorrect order would cause massive upset in the community. The rest of the order seems standard anyway. Whether it's enforced now with sniffs, or later, just getting agreement and into the Handbook is, as always, the first step.

[jrfnl]

Found yet another one we need to check for: for group use namespace imports:

• There should be no space between the close brace and the colon.

[GaryJones]

Noting here that the PSR12 standard inside PHP_CodeSniffer itself may start to handle some of this.

[jrfnl]

The actual bug was fixed via #1926 which replaced the WPCS native utility functions with PHPCSUtils ones.

The rules for traits and other use statements will be addressed separately.