squizlabs / PHP_CodeSniffer

PHP_CodeSniffer tokenizes PHP files and detects violations of a defined set of coding standards.
BSD 3-Clause "New" or "Revised" License
10.66k stars 1.48k forks source link

PSR2 standard is not checking that closing brace is on line following the body #908

Closed rtucek closed 8 years ago

rtucek commented 8 years ago

From php-fig

Method names MUST NOT be declared with a space after the method name. The opening brace MUST go on its own line, and the closing brace MUST go on the next line following the body. There MUST NOT be a space after the opening parenthesis, and there MUST NOT be a space before the closing parenthesis.

So based on the definition above a function/method like this:

public function some_function()
{

    // Some code here

    // Some code there

    return $something;

}

should be converted to this (e.g. stripping spaces and linebreaks in the body between the parenthesis and the first/last occurrence of a code block/comment).

public function some_function()
{
    // Some code here

    // Some code there

    return $something;
}
aik099 commented 8 years ago
  1. You haven't specified into what that method is actually converted to.
  2. Are all items from php-fig quote reported as as errors/warnings during phpcs run?
  3. Are all items from php-fig quote fixed by PHP_CodeSniffer during phpcbf run?
rtucek commented 8 years ago

Hi @aik099

  1. I did:

stripping spaces and linebreaks in the body between the parenthesis and the first/last occurrence of a code block/comment

plus providing an example.

2 & 3. I'm just referring to the bold/italic formatted text. There are no warnings/errors returned by phpcs and no fixes done by phpcbf in context of the emphasized sentence above. I believe there is no implementation (this is what I meant by 'not fully honoring').

gsherwood commented 8 years ago

I think you are half right here.

The bold text refers to the parenthesis, not the braces. It is trying to ban code like this:

<?php
public function some_function( $foo, $bar )
{
}

With that code, you get these errors:

 2 | ERROR | [x] Expected 0 spaces between opening bracket and argument "$foo"; 1 found
 2 | ERROR | [x] Expected 0 spaces between argument "$bar" and closing bracket; 1 found

So the rules inside the bold text are already checked, but by a different sniff.

There is also nothing in that rule that says that the body of the method must go on the line directly after the opening brace. In PSR2, you can have as much whitespace at the top of your functions as you'd like. Yes, I think this is crazy. No, I didn't make the rules.

But where I think you are very right is that the rule says that you can't have empty lines at the end of a function because the closing brace must go on the next line after the body. I think that is pretty clear and I should add both a check and an auto-fix for that.

Thanks for reporting it.

gsherwood commented 8 years ago

I finally got around to adding a sniff to check and fix extra blank lines at the end of functions/methods/closures.