search

PHPCSStandards / PHP_CodeSniffer

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

PSR2/ClassDeclaration: add tests and remove some redundant code #556

Closed jrfnl closed 1 month ago

jrfnl commented 2 months ago

Description

PSR2/ClassDeclaration: add tests covering the CloseBraceSameLine error

Previously, there were no dedicated tests for the CloseBraceSameLine error.

PSR2/ClassDeclaration: remove some redundant code [1]

Issue squizlabs/PHP_CodeSniffer#2621 added the T_COMMA token to the $ignoreTokens (without adding a test) to prevent a false positive for a anonymous class declaration nested within a function call. That fix was later superseded by another fix for basically the same issue via squizlabs/PHP_CodeSniffer#2678, which excluded anonymous classes completely from the CloseBraceSameLine check.

This commit adds the test case from squizlabs/PHP_CodeSniffer#2621 and removed the redundant T_COMMA token as the test now confirms it is no longer needed.

PSR2/ClassDeclaration: remove some redundant code [2]

In the original version of the sniff, only comments tokens after the close brace were ignored for the CloseBraceSameLine check.

Since then the sniff has received numerous changes improving on that code to prevent false positives.

Once such change was made in response to issue squizlabs/PHP_CodeSniffer#689, the fix adding ignoring for whitespace tokens to the code block.

This makes the $tokens[$nextContent]['content'] !== $phpcsFile->eolChar check redundant as that condition can now never be true anymore (as it could only match on T_WHITESPACE tokens and those are now ignored).

This change is covered by the tests previously added.

PSR2/ClassDeclaration: add test with close brace followed by PHP close tag

Related to #552

This adds a test to the sniff documenting that when a PHP close tag is on the same line as the OO close brace, the sniff will throw an error.

In my opinion, this is the correct and expected behaviour.

Suggested changelog entry

N/A

jrfnl commented 1 month ago

Rebased without changes, merging once the build has passed.