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

Redundant code in "Squiz_Sniffs_Classes_ClassDeclarationSniff" class #285

Closed
aik099 opened this issue Oct 12, 2014 · 2 comments
Closed

Redundant code in "Squiz_Sniffs_Classes_ClassDeclarationSniff" class #285

aik099 opened this issue Oct 12, 2014 · 2 comments

Comments

@aik099
Copy link
Contributor

aik099 commented Oct 12, 2014

The check made in https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Classes/ClassDeclarationSniff.php#L162-L170 is covered at 100% in above lines https://github.com/squizlabs/PHP_CodeSniffer/blob/master/CodeSniffer/Standards/Squiz/Sniffs/Classes/ClassDeclarationSniff.php#L136-L160 in same class.

So if there is something on same line as class/interface close brace then it's being detected already.

Here are the errors reported on last line of fixture file:

Closing brace of a class must be followed by a single blank line (....Classes.ClassDeclaration.NoNewlineAfterCloseBrace)
Closing class brace must be on a line by itself (....Classes.ClassDeclaration.CloseBraceSameLine)
@aik099
Copy link
Contributor Author

aik099 commented Oct 13, 2014

Digging a bit deeper I think 2 mentioned code pieces needs to be united. So basically in code that reports error when new line isn't present after class closing brace we should made an exclusion that allow for comment to be present on same line after class closing brace. If present, then we should look for empty line after that comment (but since it's on same line we can also look for empty line just ignoring comments on same line).

What I trying to achieve here is to reduce duplicate fixing code, that adds new lines otherwise. Because right now class closing brace will be separated from it's comment anyway during fixing process. See how I did that: aik099/CodingStandard@da60ac3

@gsherwood
Copy link
Member

You are right, but I've changed the blank line detection to be better, so I ended up removing one of the checks.

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

2 participants