WordPress / WordPress-Coding-Standards

PHP_CodeSniffer rules (sniffs) to enforce WordPress coding conventions
MIT License
2.54k stars 479 forks source link

Proposal: Add rules about formatting of the spread operator #1762

Closed jrfnl closed 1 year ago

jrfnl commented 5 years ago

Is your feature request related to a problem?

PHP 5.6 introduced the spread operator ... for packing arguments in function declarations (variadic functions) and unpacking them in function calls.

PHP 7.4 will introduce unpacking of arrays using the spread operator.

Refs:

At this moment WPCS has no formatting rules for the spread operator.

Describe the solution you'd like

I would like to propose adding the following rules regarding the spread operator to the WP Coding Standards handbook:

// Correct.
function foo( &...$spread ) {
    bar( ...$spread );

    bar(
        [ ...$foo ],
        ...array_values( $keyed_array )
    );
}

// Incorrect.
function fool( &   ... $spread ) {
    bar(...
             $spread );

    bar(
        [...  $foo ],.../*comment*/array_values( $keyed_array )
    );
}

If this proposal is accepted, we can then add sniff(s) to WPCS to check these rules automatically and - except for issues with comments - auto-fix them.

Notes for the WPCS Implementation:

/cc @pento

dingo-d commented 5 years ago

Should we put in this sniff the check, in the case of multiple arguments used with the spread operator, that the variadic argument is always placed on the last place?

// Correct.
function foo( $required, $optional = null, ...$variadic ) {
    // Code goes here
}

// Incorrect. Will output a fatal error.
function foo( $required, ...$variadic, $optional = null ) {
    // Code goes here
}
jrfnl commented 5 years ago

Should we put in this sniff the check, in the case of multiple arguments used with the spread operator, that the variadic argument is always placed on the last place?

Good point. In my opinion, no, this should not be covered here. This topic is about the formatting of the spread operator, i.e. whitespace and such.

Parse errors are covered by the Generic.PHP.Syntax sniff which is included in WordPress-Extra.

If we would want to check the order of parameters, I think a more generic CodeAnalysis.FunctionDeclarationParameterOrder sniff would be more appropriate.

Such a sniff could then check:

If such a sniff would be desired, a separate issue should be opened about it and we'd need to check if anything of the above is already covered and if there are any upstream sniffs available we could use for this.

pento commented 5 years ago

I agree with rules 1, 2, and 3. 🎉

jrfnl commented 5 years ago

FYI: The upstream PR for rule 2 has been merged and a sniff for rule 3 has also been added. Both will be included in PHPCS 3.5.0.

jrfnl commented 4 years ago

FYI: PHPCS 3.5.0 has been released.

jrfnl commented 4 years ago

FYI: Based on the PHPCS 3.5.0 changelog, rule 3 is covered via the Squiz.Functions.FunctionDeclarationArgumentSpacing which is already included in the WordPress-Core ruleset.

Squiz.Functions.FunctionDeclarationArgumentSpacing now checks for no space after a reference operator

Similar for rule 2, though only in the case of function declarations, so adding the PHPCS 3.5.0 Generic.WhiteSpace.SpreadOperatorSpacingAfter sniff is still needed.

Squiz.Functions.FunctionDeclarationArgumentSpacing now checks for no space after a variadic operator

Src: https://github.com/squizlabs/PHP_CodeSniffer/releases/tag/3.5.0

jrfnl commented 4 years ago

If we would want to check the order of parameters, I think a more generic CodeAnalysis.FunctionDeclarationParameterOrder sniff would be more appropriate.

@dingo-d The PEAR.Functions.ValidDefaultValue sniff partially covers this. Also see: https://github.com/squizlabs/PHP_CodeSniffer/issues/2719

Note: that sniff is currently not included in any of the WPCS rulesets.