Forbidden new/empty lines?

497 views
Skip to first unread message

Giacomo Gatelli

unread,
Jun 16, 2013, 4:33:52 AM6/16/13
to php-f...@googlegroups.com
Hey guys,

I'm using phpcs with the PSR-2 ruleset, but I'm getting several new/empty lines errors.
I'd like to understand if it's me misreading the standard or phpcs being over-zealous :)

1. Empty lines and switch/case

We often use empty lines to improve the readability of switch/cases, like:

switch ($var) {
    case 1:
        return $something;

    case 2:
        return $somethingElse;
}

but phpcs doesn't like it:

Blank lines are not allowed between case statements; found 1

What rules is this based on? The switch/case section doesn't talk about empty lines at all

A switch structure looks like the following. Note the placement of parentheses, spaces, and braces. The case statement MUST be indented once from switch, and the break keyword (or other terminating keyword) MUST be indented at the same level as the case body. There MUST be a comment such as // no break when fall-through is intentional in a non-empty case body.

2. Empty lines and method calls

We also use empty lines to improve readability of multi-line method calls (particularly when setting up mocks, because some method calls have a logical grouping), like:

$this
    ->a()
    ->b()

    ->c()
    ->d()

but again phpcs complains:

Empty lines are not allowed in multi-line function calls

In this case the rule says

When making a method or function call, there MUST NOT be a space between the method or function name and the opening parenthesis, there MUST NOT be a space after the opening parenthesis, and there MUST NOT be a space before the closing parenthesis. In the argument list, there MUST NOT be a space before each comma, and there MUST be one space after each comma.
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.

but it doesn't anything about empty lines (and I don't think empty lines count as trailing white space, right?)

3. New lines and semicolons

When concatenating several calls we usually put the semicolon on a new line, which is useful to re-order/add calls (similar to trailing semicolons in arrays), like this:

$this
    ->a()
    ->b()
    ->c()
    ;

However, this generates the following error:

Space after closing parenthesis of function call prohibited

and I'm unsure what rule is this coming from.


Thanks
Giacomo

Pádraic Brady

unread,
Jun 16, 2013, 5:13:58 AM6/16/13
to Giacomo Gatelli, php-f...@googlegroups.com
Hi Giacomo,

None of these examples are forbidden by PSR-2. The history behind PHP
CS is that it created a PSR-2 ruleset that also includes judgement
calls to cover any perceived gaps in the standard. In other words, PHP
CS is not just PSR-2 compliant. It adds additional requirements not
mentioned, alluded to, or otherwise insinuated by PSR-1 and PSR-2.
This has been brought up before in other emails to the list. You might
have a look at Sensiolabs CS Fixer? I believe it will leave your
current practices alone and it will still fix many (not necessarily
all!) PSR issues. You can do some testing with the --dry-run option or
a spare copy.

Paddy
> --
> 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

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

Greg Sherwood

unread,
Jun 16, 2013, 10:46:27 PM6/16/13
to php-f...@googlegroups.com, Giacomo Gatelli

On Sunday, 16 June 2013 19:13:58 UTC+10, Pádraic Brady wrote:
The history behind PHP
CS is that it created a PSR-2 ruleset that also includes judgement
calls to cover any perceived gaps in the standard. In other words, PHP
CS is not just PSR-2 compliant. It adds additional requirements not
mentioned, alluded to, or otherwise insinuated by PSR-1 and PSR-2.

No. Actually, this is a complete load of rubbish.

I've tried to make the standards in PHPCS exactly reflect the wording of the standards. In this case, it looks to me like part of another standard has been incorrectly imported into PSR1 or 2 and needs to be fixed. 

Giacomo, you can report a bug if you'd like, but I'll have a look into it regardless.

Greg

Greg Sherwood

unread,
Jun 17, 2013, 12:00:57 AM6/17/13
to php-f...@googlegroups.com
On Sunday, 16 June 2013 18:33:52 UTC+10, Giacomo Gatelli wrote:
1. Empty lines and switch/case

What rules is this based on? The switch/case section doesn't talk about empty lines at all

A switch structure looks like the following. Note the placement of parentheses, spaces, and braces. The case statement MUST be indented once from switch, and the break keyword (or other terminating keyword) MUST be indented at the same level as the case body. There MUST be a comment such as // no break when fall-through is intentional in a non-empty case body.

Looks like I copied the sniff from another standard and didn't remove the spacing part. I have removed it now: https://github.com/squizlabs/PHP_CodeSniffer/commit/2e1bd08c7e7cd5cf6e89a4f601686ddb1048ee14


2. Empty lines and method calls

We also use empty lines to improve readability of multi-line method calls (particularly when setting up mocks, because some method calls have a logical grouping), like:

$this
    ->a()
    ->b()

    ->c()
    ->d()

but again phpcs complains:


I don't get any errors when I check this code. Can you please provide me with a bit of code that causes the problem and/or the PHPCS version (phpcs --version).
 

3. New lines and semicolons

When concatenating several calls we usually put the semicolon on a new line, which is useful to re-order/add calls (similar to trailing semicolons in arrays), like this:

$this
    ->a()
    ->b()
    ->c()
    ;

However, this generates the following error:

Space after closing parenthesis of function call prohibited

and I'm unsure what rule is this coming from.

This is coming from the PEAR sniff that I used to implement PSR2. But while the PEAR standard clearly states that there must be no space between the closing parenthesis and the semicolon, you are right in that PSR2 does not mention this. So I've muted this error from PSR2: https://github.com/squizlabs/PHP_CodeSniffer/commit/238a160b54b979bbf3e9f6941d1a1dc64ae9b48b

 

Giacomo Gatelli

unread,
Jun 18, 2013, 7:59:54 AM6/18/13
to php-f...@googlegroups.com
Hi Greg,

thanks for the changes.


On Monday, 17 June 2013 14:00:57 UTC+10, Greg Sherwood wrote:


2. Empty lines and method calls

We also use empty lines to improve readability of multi-line method calls (particularly when setting up mocks, because some method calls have a logical grouping), like:

$this
    ->a()
    ->b()

    ->c()
    ->d()

but again phpcs complains:

Ops, my bad, I misread the error. The code I was actually looking at was:

Something::doSomething(

    $this
        ->a()
        ->b()

        ->c()
);

Which is complaining because multi-line argument list function calls can't have white lines.
However, in this case the white line belongs to the chained calls rather than to the multi-line argument list.

I don't think the standard clarifies what happens when there are nested calls, so I guess this is up for interpretation

Thanks again for your help

Giacomo

Greg Sherwood

unread,
Jun 19, 2013, 2:01:23 AM6/19/13
to Giacomo Gatelli, php-f...@googlegroups.com
On Tue, Jun 18, 2013 at 9:59 PM, Giacomo Gatelli <art...@gmail.com> wrote:

Which is complaining because multi-line argument list function calls can't have white lines. 
However, in this case the white line belongs to the chained calls rather than to the multi-line argument list.

I don't think the standard clarifies what happens when there are nested calls, so I guess this is up for interpretation


Yeah, I think I'll leave this one for now, but clarification would be good. I've added this to the wiki page: https://github.com/php-fig/fig-standards/wiki/Further-Style-Guide-Considerations
Reply all
Reply to author
Forward
0 new messages