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

PSR12 - foreach - ControlStructure #3047

Open
SpazzMarticus opened this issue Aug 7, 2020 · 2 comments
Open

PSR12 - foreach - ControlStructure #3047

SpazzMarticus opened this issue Aug 7, 2020 · 2 comments

Comments

@SpazzMarticus
Copy link

SpazzMarticus commented Aug 7, 2020

Hi,

in the PSR-12 standard foreach is defined only as:

A foreach statement looks like the following. Note the placement of parentheses, spaces, and braces.

<?php

foreach ($iterable as $key => $value) {
    // foreach body
}

Splitting - like for if, elseif, else, switch, case, while, do-while and for - is not defined:

... Expressions in parentheses MAY be split across multiple lines, where each subsequent line is indented at least once. ...

Given these three formatting styles only the first one yields no errors:

<?php

$array = ['a', 'b', 'c'];
foreach (
    array_filter($array, function ($a) {
        return $a === 'b';
    }) as $data
) {
    //code
}

foreach (array_filter($array, function ($a) { //line 12
    return $a === 'b'; 
}) as $data) { //line 14
    //code
}

foreach (array_filter( //line 18
    $array,
    function ($a) {
        return $a === 'b';
    }) as $data) { //line 22
    //code
}

PHP 7.4.7 on Windows, with CodeSniffer 3.5.5:

 12 | ERROR | [x] The first expression of a multi-line control structure must be on the line after
    |       |     the opening parenthesis
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.FirstExpressionLine)
 14 | ERROR | [x] Each line in a multi-line control structure must be indented at least once;
    |       |     expected at least 4 spaces, but found 0
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.LineIndent)
 14 | ERROR | [x] The closing parenthesis of a multi-line control structure must be on the line after
    |       |     the last expression
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.CloseParenthesisLine)
 18 | ERROR | [x] The first expression of a multi-line control structure must be on the line after
    |       |     the opening parenthesis
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.FirstExpressionLine)
 22 | ERROR | [x] Multi-line function call not indented correctly; expected 0 spaces but found 4
    |       |     (PSR2.Methods.FunctionCallSignature.Indent)
 22 | ERROR | [x] Closing parenthesis of a multi-line function call must be on a line by itself
    |       |     (PSR2.Methods.FunctionCallSignature.CloseBracketLine)
 22 | ERROR | [x] The closing parenthesis of a multi-line control structure must be on the line after
    |       |     the last expression
    |       |     (PSR12.ControlStructures.ControlStructureSpacing.CloseParenthesisLine)

Is the first formatting style valid PSR-12 code?

@gsherwood
Copy link
Member

I don't know why the MR (php-fig/fig-standards#900) didn't include this for foreach statements as it didn't explicitly exclude them and the discussion revolved around IF statements mainly.

I'm not sure I want to back out of allowing this for all control structures as it's likely to cause some BC breaks and a discussion about the author's intention, which I can't speak for.

Will leave this open in case anyone cares to comment, but I wont take any action at this time. Thanks for bringing it up though.

@gsherwood gsherwood added this to Idea Bank in PHPCS v3 Development via automation Oct 19, 2020
@gsherwood gsherwood moved this from Idea Bank to Track in PHPCS v3 Development Oct 19, 2020
@gsherwood gsherwood moved this from Track to Idea Bank in PHPCS v3 Development Oct 28, 2020
@jwittorf
Copy link

jwittorf commented Feb 22, 2023

I'll dig up this issue because I'm currently comparing different tools like PHP_CodeSniffer and PHP CS Fixer. I'm using PSR-12 as a reference to check how precise the tools check for and fix the code according to PSR-12.

Since there is nothing saying "foreach multi-line statements MUST follow the same guidelines and principles as if statements, see examples below", neither of @SpazzMarticus code-snippets would be correct. Furthermore, they are missing the => $value part.

According to the currently accepted PSR-12, the only valid way is using foreach ($iterable as $key => $value) - period.

I therefore propose to add some input to the foreach section as described in the second paragraph.

Some of my personal ideas:

<?php

$iterableArray = [
    "one",
    "two",
    "three",
];

// this makes no sense (1-2)
foreach (
    $iterableArray as $i => $value) {
    // code
}

foreach (
    $iterableArray as $i => $value
) {
    // code
}



// this could make sense (3-4)
foreach (
    $iterableArray
    as $i => $value
) {
    // code
}

foreach (
    $iterableArray as $i
    => $value
) {
    // code
}



// how about this? (5-6)
// a different approach, similar to the for-loop
foreach (
    $iterableArray
    as $i =>
    $value
) {
    // code
}

foreach (
    $iterableArray
    as $i
    => $value
) {
    // code
}

I'm curious about further input, although the issue is a bit older.

/edit: Whops, I though this was already in the PSR-repository ... I'll start a discussion on their mailing list.

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

No branches or pull requests

3 participants