PSR2/ClassDeclaration: add tests and remove some redundant code #556
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
54 tasks
Previously, there were no dedicated tests for the `CloseBraceSameLine` error.
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.
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.
…e 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.
jrfnl
force-pushed
the
feature/psr2-classdeclaration-add-tests-close-tag
branch
from
ff9ea41 to
8303a3f
Compare
|
Rebased without changes, merging once the build has passed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
PSR2/ClassDeclaration: add tests covering the CloseBraceSameLine error
Previously, there were no dedicated tests for the
CloseBraceSameLineerror.PSR2/ClassDeclaration: remove some redundant code [1]
Issue squizlabs/PHP_CodeSniffer#2621 added the
T_COMMAtoken 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
CloseBraceSameLinecheck.This commit adds the test case from squizlabs/PHP_CodeSniffer#2621 and removed the redundant
T_COMMAtoken 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
CloseBraceSameLinecheck.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->eolCharcheck redundant as that condition can now never be true anymore (as it could only match onT_WHITESPACEtokens 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