squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.63k stars 1.48k forks source link

Support multiline foreach in Squiz.ControlStructures.ForEachLoopDeclaration #3907

Open Daimona opened 8 months ago

Daimona commented 8 months ago

The Squiz.ControlStructures.ForEachLoopDeclaration sniff fails to account for multiline foreach loops when checking the space around the as keyword. If you simply enable the sniff and run it on this code (with tabWidth = 4):

function myTest() {
    foreach ( $array
        as $el ) {

    }
}

it says:

 202 | ERROR   | [x] Expected 1 space before "as"; 8 found
     |         |     (Squiz.ControlStructures.ForEachLoopDeclaration.SpacingBeforeAs)

And similarly, for this code:

function myTest() {
    foreach ( $array as
        $el ) {

    }
}
 202 | ERROR   | [x] Expected 1 space after "as"; 0 found
     |         |     (Squiz.ControlStructures.ForEachLoopDeclaration.SpacingAfterAs)

Regardless of where the as keyword is placed, there is no way to have a multiline foreach assignment with this sniff enabled. This is especially problematic if any side of as is particularly long (e.g., a function call, or an array destructuring expression).

The Squiz.ControlStructures.ForLoopDeclaration sniff provides the ignoreNewlines option to ignore multi-line for conditions. It would be nice to have something similar here.

jrfnl commented 8 months ago

@Daimona If you want multi-line control structures, the Squiz sniff may not be your best option. I suggest checking out the PSR12.ControlStructures.ControlStructureSpacing sniff or the PEAR.ControlStructures.MultiLineCondition sniffs instead.

Daimona commented 8 months ago

@Daimona If you want multi-line control structures, the Squiz sniff may not be your best option. I suggest checking out the PSR12.ControlStructures.ControlStructureSpacing sniff or the PEAR.ControlStructures.MultiLineCondition sniffs instead.

Thank you for the suggestion. Unfortunately, the only thing that I wanted to use the ForEachLoopDeclaration for is the spacing around the as keyword :-/ And the Squiz.ControlStructures.ForEachLoopDeclaration sniff is the only one I could find in the base standards which checks that.