Should an anonymous function be able to be passed like this and still be valid PSR-2?

[Beau Simensen]

I finally updated phpcs last night and have run PSR2 validation against some of my projects. Things looked good at first until I ran into PSR-2 validation errors with the following:

        return $this->em->transactional(function () use ($session, $func) {

            return call_user_func_array($func, $session);

        });

I also discovered that the following somewhat standard Silex style code does not pass PSR-2 validation:

        $app->get('/hello/{name}', function ($name) use ($app) { 

            return 'Hello '.$app->escape($name); 

        }); 

Based on the existing PSR-2 documentation, should these both validate as valid PSR-2?

I was able to clear up the validation warning by making my code look like this. I find this altogether unacceptable so I'm hoping this is a bug in phpcs and not "as intended" PSR-2. :)

        $app->get(

            '/hello/{name}',

            function ($name) use ($app) {

                return 'Hello '.$app->escape($name);

            }

        );

        return $this->entityManager->transactional(

            function () use ($session, $func) {

                return call_user_func_array($func, $session);

            }

        );

If it is a bug, any idea where I should look to try to fix it?

[Greg Sherwood]

Hi Beau,

I am the PHP_CodeSniffer maintainer. I'm lurking around these lists, so you can post here with questions about the PSR standards in PHPCS, or you can post a bug on the project bug tracker if you prefer. The link is at the bottom of this page: https://github.com/squizlabs/PHP_CodeSniffer

On Friday, 1 March 2013 15:12:53 UTC+11, Beau Simensen wrote:

I finally updated phpcs last night and have run PSR2 validation against some of my projects. Things looked good at first until I ran into PSR-2 validation errors with the following:

        return $this->em->transactional(function () use ($session, $func) {

            return call_user_func_array($func, $session);

        });

There errors that you get are:

Opening parenthesis of a multi-line function call must be the last content on the line; AND

Closing parenthesis of a multi-line function call must be on a line by itself

 

I also discovered that the following somewhat standard Silex style code does not pass PSR-2 validation:

        $app->get('/hello/{name}', function ($name) use ($app) { 

            return 'Hello '.$app->escape($name); 

        }); 

The errors that you get are:

Opening parenthesis of a multi-line function call must be the last content on the line; AND

Only one argument is allowed per line in a multi-line function call; AND

Closing parenthesis of a multi-line function call must be on a line by itself

 

Based on the existing PSR-2 documentation, should these both validate as valid PSR-2?

I don't think so, based on this part of the standard: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md#46-method-and-function-calls

In particular, this part: "Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line."

The other error about the closing parenthesis is inferred from the examples given. This is required for other parts of the standard as well, as it sometimes simply asks for you to ensure your code is formatted as per the examples; something I see fairly commonly, but it can be ambiguous. But I don't think this is what you are really concerned about anyway. The other 2 errors affect the formatting of your code much more.

 

I was able to clear up the validation warning by making my code look like this. I find this altogether unacceptable so I'm hoping this is a bug in phpcs and not "as intended" PSR-2. :)

        $app->get(

            '/hello/{name}',

            function ($name) use ($app) {

                return 'Hello '.$app->escape($name);

            }

        );

        return $this->entityManager->transactional(

            function () use ($session, $func) {

                return call_user_func_array($func, $session);

            }

        );

Personally, I find that much easier to read than the first snippets of code, and I find it much easier to comment out and replace a function argument (for debugging) when code is written like this. But that is only my personal preference and am I not a voting member of the PHP-FIG and was not involved in the writing of PSR-2, so I can't comment on behalf of the group.

It doesn't look like any vote information is available in the standard (at the bottom) for this particular part, so I don't think we can infer the intention of the group either.

But hopefully that makes it clearer as to why PHPCS is reporting those errors.

[Beau Simensen]

I am the PHP_CodeSniffer maintainer. I'm lurking around these lists, so you can post here with questions about the PSR standards in PHPCS, or you can post a bug on the project bug tracker if you prefer. The link is at the bottom of this page: https://github.com/squizlabs/PHP_CodeSniffer

Hi Greg!

I must have seen you post before because i recognize your name. I remembered that someone involved with PHPCS had been writing the main fig mailing list in the past and glad to see you are still poking your head around. :)

Thanks for adding the actual errors to the context of my post. I appreciate it. I don't have an easy way to get the exact messages (due to how I'm using phpcs with my editor) so that was nice to see added.

In particular, this part: "Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line."

So in other words since by definition an anonymous function is probably going to be multiple lines it automatically requires that any call made with an anonymous function has to be done the "one argument per line" way? Ugh. I hope this isn't true. :) If it is, so be it, but if there is ambiguity in how this can be interpreted (I've heard that at least phpstorm is doing PSR-2 code formatting and doesn't complain about the more compressed format) that could be problematic.

Can one or more fig members chime in on the spirit of the PSR-2 standard in this regard? As I see it, a healthy chunk of Pimple, Silex, or Cilex based projects (along with anything else heavily inline anonymous function based) are not going to be able to be PSR-2 compliant if this is the correct and only valid interpretation of the spec.

But hopefully that makes it clearer as to why PHPCS is reporting those errors.

Indeed it did, thanks!

Greg, I'm relatively new to phpcs. I'm all about trying to follow PSR2 but this would be a deal breaker for me. I'd like to try and tweak the PSR2 bits such that in the case of passing anonymous functions in the ways I'm trying to use them it give me any problems. Where would I look to start trying to patch this for my local install? Do you think it would be easy to do so or am I in for a world of hurt? :)

[Greg Sherwood]

On Saturday, 2 March 2013 14:49:22 UTC+11, Beau Simensen wrote:

Thanks for adding the actual errors to the context of my post. I appreciate it. I don't have an easy way to get the exact messages (due to how I'm using phpcs with my editor) so that was nice to see added.

No problem. I run PHPCS on the command line all day anyway :)

 

In particular, this part: "Argument lists MAY be split across multiple lines, where each subsequent line is indented once. When doing so, the first item in the list MUST be on the next line, and there MUST be only one argument per line."

So in other words since by definition an anonymous function is probably going to be multiple lines it automatically requires that any call made with an anonymous function has to be done the "one argument per line" way? Ugh. I hope this isn't true. :) If it is, so be it, but if there is ambiguity in how this can be interpreted (I've heard that at least phpstorm is doing PSR-2 code formatting and doesn't complain about the more compressed format) that could be problematic.

That's how I read it, yes.

Sometimes the problem with standards is that they are too ambiguous. Sometimes, they appear clear but just don't cover all possible scenarios, so they can also appear ambiguous because a specific language feature or scenario is not mentioned. You're not the first person to ask me about anonymous functions in this context, so the standard must not be clear enough in this case (one way or the other). But I don't think PSRs are changed, so...

Can one or more fig members chime in on the spirit of the PSR-2 standard in this regard? 

Exactly what I normally suggest. I think it's the only way to know for sure.

 

Greg, I'm relatively new to phpcs. I'm all about trying to follow PSR2 but this would be a deal breaker for me. I'd like to try and tweak the PSR2 bits such that in the case of passing anonymous functions in the ways I'm trying to use them it give me any problems. Where would I look to start trying to patch this for my local install? Do you think it would be easy to do so or am I in for a world of hurt? :)

Customising PSR-2 is pretty easy. It involves you creating a ruleset.xml file and using that instead of PSR2.

So if you wanted everything in PSR-2 except for this multi-line function stuff, you'd create a file that has this content:

<?xml version="1.0"?>

<ruleset name="MyStandard">

 <description>A custom PSR2 standard.</description>

 <rule ref="PSR2">

  <exclude name="PEAR.Functions.FunctionCallSignature"/>

 </rule>

</ruleset>

If that file is saved at /home/users/mystandard.xml then the standard you tell PHPCS to use is the file path, instead of "PSR2". If you were doing this on the command line, you'd run phpcs --standard=/home/users/mystandard.xml /path/to/code but you'll need to see if your editor allows you to customise the standard.

The way I got that special code was from the command line, by running phpcs --standard=PSR2 -s , which shows the source code of all errors. You get something that looks like this:

Opening parenthesis of a multi-line function call must be the last content on the line (PEAR.Functions.FunctionCallSignature.ContentAfterOpenBracket)

Closing parenthesis of a multi-line function call must be on a line by itself (PEAR.Functions.FunctionCallSignature.CloseBracketLine)

Only one argument is allowed per line in a multi-line function call (PEAR.Functions.FunctionCallSignature.MultipleArguments)

You can go ahead and just mute the specific codes you want, but I decide to mute the whole FunctionCallSignature sniff in this case.

There is a lot more you can do with your own ruleset.xml file as well. Committing it to your repository will also let other developers use it too. Some more docs about it here: https://github.com/squizlabs/PHP_CodeSniffer/wiki/Annotated-ruleset.xml

Incidentally, if anyone else following the tabs vs spaces thing is interested, you can customise your PSR2 standard like this:

<?xml version="1.0"?>

<ruleset name="MyStandard">

 <description>PSR2 without the requirement to use spaces.</description>

 <rule ref="PSR2">

  <exclude name="Generic.WhiteSpace.DisallowTabIndent"/>

 </rule>

</ruleset>

[Beau Simensen]

Thanks for the tips, Greg. I found that the following was sufficient to stop seeing stuff pop up for my specific use cases:

<?xml version="1.0"?>

<ruleset name="PSR2Lite">

 <description>A custom PSR2 standard.</description>

 <rule ref="PSR2">

  <exclude name="PEAR.Functions.FunctionCallSignature.MultipleArguments"/>

 </rule>

</ruleset>

It isn't clear to me what impact this is going to have on the standards checking, though. If I wanted to see exactly what disabling "PEAR.Functions.FunctionCallSignature.MultipleArguments" does, where would I look? Is it only in the code somewhere? Or is it documented on a website as well? I'm curious to know what other types of sins I may end up committing that I won't be aware of by disabling this. :)

[Greg Sherwood]

On Sunday, 3 March 2013 09:00:37 UTC+11, Beau Simensen wrote:

Thanks for the tips, Greg. I found that the following was sufficient to stop seeing stuff pop up for my specific use cases:

<?xml version="1.0"?>

<ruleset name="PSR2Lite">

 <description>A custom PSR2 standard.</description>

 <rule ref="PSR2">

  <exclude name="PEAR.Functions.FunctionCallSignature.MultipleArguments"/>

 </rule>

</ruleset>

That actually does the same thing as my example, but it is a little confusing as to why.

The exclude argument only works on whole sniffs. So if you paste in a specific message's code, it will still just use the first 3 parts and exclude PEAR.Functions.FunctionCallSignature from the standard.

If you only want to mute that specific error message, you would do this:

<rule ref="PEAR.Functions.FunctionCallSignature.MultipleArguments">

  <severity>0</severity>

</rule>

Or you could turn it into a warning:

<rule ref="PEAR.Functions.FunctionCallSignature.MultipleArguments">

  <type>warning</type>

</rule>

Or even change the message as well, to remind you why it is there:

<rule ref="PEAR.Functions.FunctionCallSignature.MultipleArguments">

  <message>Reminder that PSR-2 bans multiple arguments on the same line, but I don't agree</message>

  <type>warning</type>

</rule>

 

It isn't clear to me what impact this is going to have on the standards checking, though. If I wanted to see exactly what disabling "PEAR.Functions.FunctionCallSignature.MultipleArguments" does, where would I look? Is it only in the code somewhere? Or is it documented on a website as well? I'm curious to know what other types of sins I may end up committing that I won't be aware of by disabling this. :)

There are a lot of codes and they are not documented somewhere specific. The best way to find the code you are looking for is the run phpcs with the -s command line argument and see the codes in the output.

Each code is unique though, and only ever silences a single error message when excluded. The best way to find out what it does, and what the message says, is to check the code.

For example:  PEAR.Functions.FunctionCallSignature.MultipleArguments

Go to the directory: CodeSniffer/Standards/PEAR/Functions

Looks for the file: FunctionCallSignatureSniff.php

Inside the file, search for: MultipleArguments

In this case: https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/PEAR/Sniffs/Functions/FunctionCallSignatureSniff.php#L290

If a sniff extends another sniff, you may need to follow the class structure into another standard, but this is fairly rare.

[Beau Simensen]

Thanks for the information here. I'll try to play with it a bit more. :)

Meanwhile, I don't think any actual PHP-FIG members have paid any attention to this. :-/ I'd love for one or two people to step in and help figure out what the spirit of the PSR intended for this use case.

Anyone?

[Pádraic Brady]

Hi Beau,

The relevant sections of PSR-2 are sections 4.6 and 6. Section 4.6
does not address closures as arguments. Section 6 dictates the format
of a closure. My interpretation is therefore that a closure may be a
single argument and can therefore start on the same line as the
bracket opening the argument list. It is not a "multiple argument" by
itself. The last example of Section 6 shows that the format is
required in an argument list (so presumably also when a single
argument).

On this basis, CodeSniffer is throwing an error incorrectly and it
should be reported as a bug.

Any other members out there with a differing interpretation?

Paddy

--
Pádraic Brady

http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team
Zend Framework PHP-FIG Representative

> --
> You received this message because you are subscribed to the Google Groups
> "PHP Framework Interoperability Group - Coding Style" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to php-fig-cs+...@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[Pádraic Brady]

Beau,

From the phpcs error naming, they seem to be using "multiple lines"
detection which would definitely fail your example. Does the same
issue happen if you include a concatenated string as the single
argument to fit code width over multiple lines? Probably does which, I
assume, is also wrong. Might be something to consider in the future
for PSR-2 whenever we get around to having a workflow for revisions to
clear up such potential misunderstandings.

Paddy

--
Pádraic Brady

http://blog.astrumfutura.com
http://www.survivethedeepend.com
Zend Framework Community Review Team
Zend Framework PHP-FIG Representative

[Greg Sherwood]

First, congratulations on becoming a PHP-FIG voting member. That vote didn't take long ;)

On Wednesday, 6 March 2013 07:49:02 UTC+11, Pádraic Brady wrote:

From the phpcs error naming, they seem to be using "multiple lines"
detection which would definitely fail your example.

This is correct. I use the multiple-line definition because that is what the standard reads.

See my first response to this thread, which reads (in part):

> Based on this part of the standard: https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md#46-method-and-function-calls

> In particular, this part: "Argument lists MAY be split across multiple lines, where each

> subsequent line is indented once. When doing so, the first item in the list MUST be on

> the next line, and there MUST be only one argument per line."

Yes, it says "argument lists". But a list can have a single argument in it, so the spirit of the standard seems to be much more focused on the fact that it is defined over multiple lines.

Does the same
issue happen if you include a concatenated string as the single
argument to fit code width over multiple lines? Probably does which, I
assume, is also wrong.

Yes, it would do exactly the same thing because the function call is considered a multi-line function call instead of a single-line call. The number of arguments is not taken into account.

Note that this is pretty much the same rule that the PEAR standard uses (I assume it is not a coincidence) and so they actually share the same check here: http://pear.php.net/manual/en/standards.funcalls.php

 

Might be something to consider in the future
for PSR-2 whenever we get around to having a workflow for revisions to
clear up such potential misunderstandings.

If you check out the posts I made on the main PHP-FIG list while writing these standards into PHP_CodeSniffer, you'll see there are many places in the standard that are open to interpretation. I was told to interpret and code the rules as I saw them, but I tried (and still try) to get some agreement about what the majority of developers trying to adhere to the standard want to see, assuming that is a valid interpretation of the PSRs. I'd love to see some clarity put around some of these sections, especially as the language matures further and additional features are added.

[Pádraic Brady]

On 5 March 2013 21:29, Greg Sherwood <gshe...@gmail.com> wrote:
> First, congratulations on becoming a PHP-FIG voting member. That vote didn't
> take long ;)

Thanks :P

> See my first response to this thread, which reads (in part):
>
>> Based on this part of the standard:
>> https://github.com/php-fig/fig-standards/blob/master/accepted/PSR-2-coding-style-guide.md#46-method-and-function-calls
>> In particular, this part: "Argument lists MAY be split across multiple
>> lines, where each
>> subsequent line is indented once. When doing so, the first item in the
>> list MUST be on
>> the next line, and there MUST be only one argument per line."
>
> Yes, it says "argument lists". But a list can have a single argument in it,
> so the spirit of the standard seems to be much more focused on the fact that
> it is defined over multiple lines.

I've noted on the main mailing list that this needs comment from other
members. Personally, standards either state something or they don't -
if there's doubt, the FIG members need to come to a concensus and
amend the standard. In this case, the emphasis is entirely on
"argument lists" which all sections and examples make clear include >1
argument. The only literal interpretation is that the standard does
not preclude breaking a single argument legally over multiple lines
without necessitating a newline after the opening function arg
bracket. It's just silent on the whole issue. We can point to other
standards but I couldn't find a specific example out there (maybe
others will have better luck) addressing anon. functions as a single
argument.

For me at least, the OP's original example is completely legal under
PSR-2. That may be against the intended spirit of the PSR but I'm not
psychic and I don't expect those adopting the PSR to be either. The
only current reason to adopt the multiline interpretation is to avoid
CodeSniffer's or similar errors.

Paddy

[Beau Simensen]

On Tuesday, March 5, 2013 4:03:10 PM UTC-6, Pádraic Brady wrote:

On 5 March 2013 21:29, Greg Sherwood <gshe...@gmail.com> wrote: 
> First, congratulations on becoming a PHP-FIG voting member. That vote didn't 
> take long ;) 

Thanks :P 

Indeed, Paddy, congrats. :)

I've noted on the main mailing list that this needs comment from other
members.

Thanks for that. :) I feel like the voting members have bigger fish to fry at this point so I'm not terribly surprised that this issue is not getting more exposure.

 

For me at least, the OP's original example is completely legal under
PSR-2. That may be against the intended spirit of the PSR but I'm not
psychic and I don't expect those adopting the PSR to be either. The
only current reason to adopt the multiline interpretation is to avoid
CodeSniffer's or similar errors.

I think that if enough voting members are of the mindset that the existing phpcs interpretation is not in line with the spirit of PSR-2 it won't be a problem to get it changed. I just don't know how many voting members care about tuning in at this point. :)

Greg has been super helpful so far and I really appreciate it. With his help I've been able to get pretty close to changing this behavior to something I'm more comfortable working with. I'm not above amending PSR-2 for my own purposes if it turns out that the existing phpcs interpretation is the correct one, but I'm happy to help things along if it is incorrect and should be updated.

I'd like to thank you both for helping me out. :)

[Beau Simensen]

I added this issue to the further style guide considerations wiki:

https://github.com/php-fig/fig-standards/wiki/_preview

I would welcome more feedback from other members/interested parties. At this point I'm taking Greg's advice and have intentionally bypassed the checks for this aspect of the PSR-2 spec.

[Phil Sturgeon]

I think you meant this URL:

https://github.com/php-fig/fig-standards/wiki/Further-Style-Guide-Considerations

[Jason Judge]

On Thursday, 9 May 2013 18:29:34 UTC+1, Phil Sturgeon wrote:

I think you meant this URL:

https://github.com/php-fig/fig-standards/wiki/Further-Style-Guide-Considerations

So the clarification on the "single blank line on the end of the file" has now transformed into "no blank lines whatsoever allowed on the end of the file". I'm not sure I like that, or that it is even necessary to include in the style guide. I suspect opinions on that point will vary depending on what editor you use. Some editors will allow you to place your cursor below the last line of a file (after the last newline) and will let you start typing, others (e.g. vi) will not - you need to create a new line below the last line before you start typing.

However, reading it again, does ending in a "single newline" exclude a single newline after any number of previous newlines? I could put ten blank lines on the end of my file, and the last character will be a newline character, so it would still strictly end in a "single newline". It depend on what the point was trying to deal with. Was it trying to limit the number of blank lines on the end of a file, or was it trying to ensure the last line of a file is properly terminated?

Now I've confused myself.

[Richard Turner]

I believe the idea was simply to ensure that the file is "properly" terminated. Who really care how many blank lines there are at the end of the file as long as the last character is a LF?

--
RJT

[Jason Judge]

On Friday, 10 May 2013 11:12:50 UTC+1, Richard Turner wrote:

I believe the idea was simply to ensure that the file is "properly" terminated. Who really care how many blank lines there are at the end of the file as long as the last character is a LF?

Then perhaps it needs to say that `every line MUST be terminated with a newline (LF) character`. The last line of the file is no exception, so it does not need to be mentioned at all.

I personally don't care how many blank lines are on the end of a PHP script (I tend to always put one blank line on the end, because it helps when editing on some editors). Some people that will care, are those putting their files through style validators that may have interpreted the requirement in a different way. I mean, it mentions the last line in the file, so it must be special, right?

Thinking about it, this is more about the definition of how a text file is structured, than about the coding style. A validator could look at that structure first, and if it consists of lines, each properly terminated, of the right lengths, with the correct characterset and fully valid byte sequences (e.g. UTF-8) and optional lead-in characters etc. then that is one box ticked.

On the whole, it is all good stuff, but where there is some ambiguity, the intention probably needs to be spelled out.

-- Jason

[Richard Turner]

On Friday, 10 May 2013 11:26:19 UTC+1, Jason Judge wrote:

On Friday, 10 May 2013 11:12:50 UTC+1, Richard Turner wrote:

I believe the idea was simply to ensure that the file is "properly" terminated. Who really care how many blank lines there are at the end of the file as long as the last character is a LF?

Then perhaps it needs to say that `every line MUST be terminated with a newline (LF) character`. The last line of the file is no exception, so it does not need to be mentioned at all.

Quite right.
 

<snip>

On the whole, it is all good stuff, but where there is some ambiguity, the intention probably needs to be spelled out.

 
Yep, PSRs 1 & 2 really needed road-testing before they were finalised, so that there was opportunity for projects like PHP CodeSniffer to implement the rules and then feed-back any issues and questions to be clarified before further edits were prohibited. The "debate" about tabs vs. spaces that followed their ratification drained most people's will to talk about these two PSRs any further (hence the creation of this splinter Google Group) and subsequently the attitude seems to have been "live with them, warts and all, or don't use them". That's a shame because as far as I've seen a lot of questions about them have been about whether PHPCS is doing the right thing, and whether that thing was really intended. I think Greg has done a good job of interpreting the rules, but it's a shame that there are cases where the wording can technically be interpreted differently from what was intended.

I'd like to see clarifications where there's ambiguity; hopefully the proposal to allow PSRs to be maintained via Errata and Addendum sections will go through soon and some of the issues with these PSRs can be addressed.

--
RJT

[Phil Sturgeon]

Beau: your original formatting has been confirmed by the FIG to be fully acceptable in PSR-2.

See this post for more information:

https://groups.google.com/forum/#!topic/php-fig-cs/S-mcJEStVWY