Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiline foreach in Squiz.ControlStructures.ForEachLoopDeclaration #3907

Open
Daimona opened this issue Oct 25, 2023 · 2 comments
Open

Comments

@Daimona
Copy link
Contributor

Daimona commented Oct 25, 2023

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
Copy link
Contributor

jrfnl commented Oct 26, 2023

@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
Copy link
Contributor Author

Daimona commented Oct 26, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants