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

PSR2: Multiline foreach #461

Closed
Pittiplatsch opened this issue Jan 26, 2015 · 11 comments
Closed

PSR2: Multiline foreach #461

Pittiplatsch opened this issue Jan 26, 2015 · 11 comments

Comments

@Pittiplatsch
Copy link

Similar to #460, code like

foreach (
    [
        'foo'           => 'bar',
        'foobaz'        => 'bazzy',
    ] as $key => $value
) {
}

is ok for PSR2,
and I tend to believe that even

foreach (
    [
        'foo'           => 'bar',
        'foobaz'        => 'bazzy',
    ]
    as $key => $value
) {
}

is allowed.

Both versions however throw multiple errors, complaining mostly about spare spaces.

@aik099
Copy link
Contributor

aik099 commented Jan 26, 2015

What PHP_CodeSniffer version are you using? If not latest, then please try it.

@Pittiplatsch
Copy link
Author

I'm using v2.2 stable.

@gsherwood
Copy link
Member

is ok for PSR2

What makes you think this code is valid in PSR-2? The example code looks pretty clear and states Note the placement of parentheses, spaces, and braces: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md#55-foreach

@Pittiplatsch
Copy link
Author

IMO it's just the same problem as in #460 - the mighty newline needed to wrap long statements as those shown in the intial comment.

@chrisemerson
Copy link

I had put this on #460 before realising it was closed - my mistake - but I'll put it here instead.

I was about to come and open an issue about the same thing. Have read through these comments:

What makes you think this code is valid in PSR-2? The example code looks pretty clear

The example code is just that - an example - not an exhaustive list of possible permutations I think. Identical wording regarding there not being a space after the opening parenthesis is used for method definitions too, and the example code for that shows that a new line is fine, therefore a literal space character is meant by the word 'space' in that context, rather than any whitespace character.

Under 4.3. methods:

There MUST NOT be a space after the opening parenthesis

and in some example code below:

public function aVeryLongMethodName(
    ClassTypeHint $arg1,
    &$arg2,
    array $arg3 = []
) {
    // method body
}

Under 5. Control Structures:

There MUST NOT be a space after the opening parenthesis

The wording is identical, and the example in methods clearly shows that a literal space character is meant, not any white space character. Why would identical wording mean different things in different parts of the document?

@gsherwood
Copy link
Member

The example code is just that - an example - not an exhaustive list of possible permutations I think.

In the complete absence of any words, the example code is all I have to go off. All the example code in the control structure sections follows the same general style, so this is the style PSR-2 defines and is the style PHPCS tries to enforce.

For a comment about how relaxing all the rules defeats the purpose of the coding standard all together, read this: #460 (comment)

@chrisemerson
Copy link

You haven't addressed the second part of my comment at all though - the wording used in a different place combined with the example there shows that this is a correct interpretation of the wording.

@gsherwood
Copy link
Member

the wording used in a different place combined with the example there shows that this is a correct interpretation of the wording.

I don't agree with you.

The methods section you pointed to has the same example, with no newline after the open brace. It has another section dedicated to multi-line definitions, and even some errata that has been added after first release to clarify that bit.

@chrisemerson
Copy link

I think your interpretation of the rules based on which example they are closest to in the document is a HUGE assumption indeed, and there is nothing in the text at all that suggests the rules stated should only be followed for single line declaration. It has ADDITIONAL information on multi-line definitions, but these don't override the rules on methods in general, they supplement them.

The idea that identical wording in the same document should mean different things in different places is extremely unlikely.

With regards to the post you linked to - if that is a consequence of allowing this, then so be it. The standard should enforce the rules as they are given, not change them a bit just because you think one of the possibilities that perhaps wasn't considered by the authors looks messy to you. People can always supplement PSR-2 with their own sniffs to tighten up the rules around that area if they want, but if the tool errors for something which is clearly allowed just because the consequences are a bit messy, then the tool is still broken.

As it stands, it looks like I'm going to have to create some custom sniff for this to actually follow PSR-2, which is a real shame for a popular tool that claims to be able to tell you if you have followed standards or not.

@gsherwood
Copy link
Member

The standard should enforce the rules as they are given, not change them a bit just because you think one of the possibilities that perhaps wasn't considered by the authors looks messy to you.

All I'm trying to do is turn a standards document into something that can be objectively checked by an automated tool. It would be nice if PHP-FIG wrote and maintained their own standard to do this given I've had no input into PSR-2 at all, but they haven't. So I use my own time to do it for (hopefully) the good of the PHP community, and spend a good chunk of time defending how the PSR-2 standard is interpreted because it is not written clearly.

If you feel I am being inaccurate or applying my own personal tastes to a coding standard that I don't even use, write your own PSR-2 coding standard and distribute it. PHP_CodeSniffer is designed for exactly that sort of thing to happen and I'd be very happy if I didn't have to field bug reports and questions about where PSR-2 is either ambiguous or just generally lacking in instruction.

@chrisemerson
Copy link

I will be writing my own standard, but I don't think it's helpful to have several out there that claim to follow the same standard but disagree. I'm also not trying to put your tool or the time you spend on it down, but I'm of the opinion that these things should be permissive by default and use the rules to place restrictions, rather than start from an assumption that everything should be perfect/in some particular strict style and using the rules to open up more possibilities.

I also still find it extremely unlikely that identical text means 2 different things in the same document, and that they mean the same thing is a far more reasonable assumption to make than the assumptions you have to make to disallow this kind of formatting.

Anyway, I won't say any more on it - appreciate the replies and the extra info/things to think about, but looks like we'll have to write our own sniffs to allow this.

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

No branches or pull requests

4 participants