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 standard is not checking that closing brace is on line following the body #908

Closed
rtucek opened this issue Mar 4, 2016 · 4 comments

Comments

@rtucek
Copy link

rtucek commented Mar 4, 2016

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
Copy link
Contributor

aik099 commented Mar 4, 2016

  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
Copy link
Author

rtucek commented Mar 4, 2016

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
Copy link
Member

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 gsherwood changed the title Squiz.Functions.FunctionDeclaration is not fully honoring PSR2 standard. PSR2 standard is not checking that closing brace is on line following the body Jun 27, 2016
gsherwood added a commit that referenced this issue Jun 27, 2016
@gsherwood
Copy link
Member

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

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

No branches or pull requests

3 participants