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.Namespaces.NamespaceDeclaration has changed behaviour for namespace scopes #2117

Closed
fenetikm opened this issue Aug 12, 2018 · 3 comments

Comments

@fenetikm
Copy link

fenetikm commented Aug 12, 2018

Before the change made in #2095 / 8e2b570 the following would be picked up:

namespace FirstNamespace{
  /**
   * FirstClass for the test.
   */
  class FirstClass{

  }
}

as there needing to be a blank line between the namespace line and the start of the block comment, thus throwing PSR2.Namespaces.NamespaceDeclaration.BlankLineAfter.

@gsherwood
Copy link
Member

Bracketed namespaces are not something I was actively checking with the sniff, so I think it was reporting an error for the same reason as the bug you linked - that it was assuming namespaces were a single statement on a single line.

I'm not actually sure what PSR-2 wants for bracketed namespaces. I assume that the "namespace declaration" means the entire bracketed statement so it should be enforcing a blank line after the closing brace, but I also strongly suspect this syntax was never considered.

The text of the standard reads as if it assumes a single namespace per file, so the use of bracketed statements would not be required and wouldn't have been considered. The fact that there are no rules about spacing around curly braces for bracketed statements is also a bit of a give-away that they were not considered.

In these cases, the best thing to do with PSR-2 is not apply any standards for these code constructs because the standard itself does not reference them. It results in potentially messier code overall, but I've been slapped down a number of times for inferring rules that are not there, so I'm now wary of trying to improve PSR-2 as PHP syntax evolves.

Also note that PSR-12 (still in draft) does not contain any rules or references to bracketed namespaces declarations either. Even more so than PSR-2, the text assumes the namespace declaration is a single line at the top of the file followed by use statements.

So my suggestion, assuming you want to try and conform to PSR-2 and PSR-12 eventually, is to try and limit a file to a single namespace so that it follows the example code displayed in those standards. Including multiple namespace and use blocks in a file is probably going to end up conflicting with something in PSR-12, or force me to make up rules for bracketed statements, which I don't want to do.

But that's a personal preference obviously, so keep doing what you're doing if you prefer that. I just don't have any included sniffs to help enforce rules for bracketed namespace declarations.

If you think I've missed something in the standards, please let me know. I'd prefer to enforce more rules rather than less, but only if I can back them up with some text or examples in the written standard.

@fenetikm
Copy link
Author

Thanks for the useful feedback and detail. I too would have thought a blank line after the complete block would have made sense.

This came about from a PR to the Drupal coder module which has some tests that check for namespaces that are bracketed... and in the many years that I have worked on Drupal I have never seen such a thing in the wild. They may have been a left over from very old Drupal versions but I am just guessing.

I think I will just update the tests in this other PR to be more in line with the tests in php code sniffer.

Thanks again for your great work on PHP_CodeSniffer.

@gsherwood
Copy link
Member

No problem. Thanks for the feedback.

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

No branches or pull requests

2 participants