Package home | Report new bug | New search | Development Roadmap Status: Open | Feedback | All | Closed Since Version 3.7.2

Bug #19768 The new Foo(array()) notation reported as invalid for PSR-2.
Submitted: 2013-01-01 19:33 UTC
From: hinikato Assigned: squiz
Status: Closed Package: PHP_CodeSniffer
PHP Version: 5.4.7 OS: Windows 7
Roadmaps: (Not assigned)    
Subscription  
Comments Add Comment Add patch


Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know! Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem : 50 - 30 = ?

 
 [2013-01-01 19:33 UTC] hinikato (Hinikato Dubrai)
Description: ------------ The PSR-2 don't contain any recommendations about of using the notation below, however it detected as invalid. Test script: --------------- // First example. $siteManager = new SiteManager(array( 'exitOnInvalidSite' => $this->exitOnInvalidSite, 'cliMode' => $this->cliMode, )); // Second example return new Site([ 'name' => $siteName, 'dirPath' => $this->getAllSitesDirPath() . '/' . $siteName ]); Expected result: ---------------- This use cases are valid. Actual result: -------------- PHPCS reports the following error: Line | ERROR | Opening parenthesis of a multi-line function call must be the | | last content on the line Line | ERROR | Closing parenthesis of a multi-line function call must be on a | | line by itself -------------------------------------------------------------------------------

Comments

 [2013-01-07 04:41 UTC] squiz (Greg Sherwood)
-Status: Open +Status: Feedback -Assigned To: +Assigned To: squiz
The array syntax is not the cause of the errors message you are getting. The standard requires you to have the braces of the function call on lines by themselves. The following code will not produce any errors: <?php // First example. $siteManager = new SiteManager( array( 'exitOnInvalidSite' => $this->exitOnInvalidSite, 'cliMode' => $this->cliMode, ) ); // Second example return new Site( [ 'name' => $siteName, 'dirPath' => $this->getAllSitesDirPath() . '/' . $siteName ] );
 [2013-01-07 05:41 UTC] hinikato (Hinikato Dubrai)
-Summary: The new Foo(array()) notation reported as invalid. +Summary: The new Foo(array()) notation reported as invalid for PSR-2.
> The standard requires you to have the braces of the function call on lines by themselves. Can you provide the quote from PSR-2/1 that describes that arrays must be indented as you have specified? I have not found any mention in the PSR-2 why we need to choose the following convention: return new Site( array( 'name' => $siteName, 'dirPath' => $this->getAllSitesDirPath() . '/' . $siteName ) ); over the next one: return new Site(array( 'name' => $siteName, 'dirPath' => $this->getAllSitesDirPath() . '/' . $siteName )); or vise versa. There is the related issue on github also: https://github.com/php-fig/fig-standards/issues/61
 [2013-01-07 06:39 UTC] squiz (Greg Sherwood)
This doesn't have anything to do with array indentation. If you use variables, you will get the same error. The error message relates to this part of the standard: "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." Look under section 4.6 of PSR-2
 [2013-01-07 06:50 UTC] hinikato (Hinikato Dubrai)
> "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, however it is not arguments list, it is one argument - array, argument list it is when > 1 arguments were passed to function. This sentence does not specify that array is list and we need indent it somehow.
 [2013-01-07 07:12 UTC] squiz (Greg Sherwood)
I think you'll find the intention of the standard is that the call to new() is across multiple-lines, so it is a multi-line function call and needs to obey the rules. The fact that it only has one argument doesn't really matter. I think the messages are very clear on this point as they refer to multi-line calls (which this obviously is) and not multi-argument calls. If you don't agree, please post of the PHP FIG mailing list as I have had no involvement is creating these standards and can't speak for the authors.
 [2013-01-07 07:49 UTC] hinikato (Hinikato Dubrai)
> I think you'll find the intention of the standard is that the call to new() is across multiple-lines, so it is a multi-line function call and needs to obey the rules. Yes, phpcs need to follow PSR-2 conventions, however it seems like it uses its own conventions regarding described above syntax, because PSR-2 does not mention how the new Foo(array()) must be intended. I have checked PSR-2 again and we have in the section 4.6 the following: 1. 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. and there is other related section - 4.4, we have there: 2. 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. When the argument list is split across multiple lines, the closing parenthesis and opening brace MUST be placed together on their own line with one space between them. Do you see that the word "argument lists" used here? Moreover the word MAY used here too. According RFC 2119 MAY means: MAY This word, or the adjective "OPTIONAL", mean that an item is truly optional. However phpcs generates ERROR. This means that it violates RFC 2119. Can you provide any other quotes?
 [2013-01-08 02:32 UTC] squiz (Greg Sherwood)
-Status: Feedback +Status: Closed
"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." Wouldn't that mean that you have to define your array all on ONE line, as it is a single argument? So do this: $siteManager = new SiteManager(array('exitOnInvalidSite' => $this- >exitOnInvalidSite, 'cliMode' => $this->cliMode)); That wont generate any errors for you. Personally, I think that is hard to read and agree with you that it is better across multiple lines. So make it a multi-line call and adhere the the part of the standard that says to put the first argument on a new line. I really don't see what the big deal here is and I'm not interested in having a really long argument about a standard that I didn't even write. If you want to get clarification, please take this to the PHP FIG google group because we aren't getting anywhere in our conversation. We obviously have very different opinions.
 [2013-08-13 02:43 UTC] benmorel (Benjamin Morel)
FYI: https://github.com/php-fig/fig-standards/issues/61 "PSR-2 is vague about this - some say intentionally vague - but as there is no rule about it, its personal preference. PHPCS is over-zealous, and decides that one way or the other is correct."