PHP_CodeSniffer icon indicating copy to clipboard operation
PHP_CodeSniffer copied to clipboard

Support multiline foreach in Squiz.ControlStructures.ForEachLoopDeclaration

Open Daimona opened this issue 2 years ago • 2 comments

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.

Daimona avatar Oct 25 '23 22:10 Daimona

@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.

jrfnl avatar Oct 26 '23 02:10 jrfnl

@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.

Daimona avatar Oct 26 '23 21:10 Daimona