From e5e3b0861a13d10bb78e41919d140f369a65b74d Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 16 Jul 2024 01:03:37 +0200 Subject: [PATCH 1/4] PSR2/ClassDeclaration: add tests covering the `CloseBraceSameLine` error Previously, there were no dedicated tests for the `CloseBraceSameLine` error. --- .../Tests/Classes/ClassDeclarationUnitTest.inc | 15 +++++++++++++++ .../Classes/ClassDeclarationUnitTest.inc.fixed | 15 +++++++++++++++ .../Tests/Classes/ClassDeclarationUnitTest.php | 1 + 3 files changed, 31 insertions(+) diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc index 144eef8fe3..4cc2d4fb87 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc @@ -316,3 +316,18 @@ class ClassWithMultiLineImplementsAndCommentOnSameLineAsInterfaceName implements /* Comment. */ AnotherInterface { } + +// Verify the `CloseBraceSameLine` error code is thrown when expected. +class ClassBraceNotOnLineByItselfError +{ + public $prop; +} $foo = new ClassBraceNotOnLineByItselfError; + +interface ClassBraceNotOnLineByItselfTrailingCommentIsAllowed +{ + public function myMethod(); +} //end interface -- this comment is allowed. + +trait ClassBraceNotOnLineByItselfTrailingAnnotationIsAllowed +{ +} // phpcs:ignore Stnd.Cat.Sniff -- this comment is also allowed. diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed index 84123ca500..1e306e373b 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed @@ -305,3 +305,18 @@ class ClassWithMultiLineImplementsAndCommentOnSameLineAsInterfaceName implements AnotherInterface { } + +// Verify the `CloseBraceSameLine` error code is thrown when expected. +class ClassBraceNotOnLineByItselfError +{ + public $prop; +} $foo = new ClassBraceNotOnLineByItselfError; + +interface ClassBraceNotOnLineByItselfTrailingCommentIsAllowed +{ + public function myMethod(); +} //end interface -- this comment is allowed. + +trait ClassBraceNotOnLineByItselfTrailingAnnotationIsAllowed +{ +} // phpcs:ignore Stnd.Cat.Sniff -- this comment is also allowed. diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php index ef97be4601..df77ebb66e 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php @@ -78,6 +78,7 @@ public function getErrorList() 282 => 1, 310 => 1, 316 => 1, + 324 => 1, ]; }//end getErrorList() From 04da5586979df7d1b71371b81da7fe70a39a074a Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 16 Jul 2024 01:09:06 +0200 Subject: [PATCH 2/4] 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/Sniffs/Classes/ClassDeclarationSniff.php | 1 - .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc | 7 +++++++ .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed | 7 +++++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php index 3325459ae2..42cff38957 100644 --- a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php +++ b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php @@ -525,7 +525,6 @@ public function processClose(File $phpcsFile, $stackPtr) $ignoreTokens[] = T_WHITESPACE; $ignoreTokens[] = T_COMMENT; $ignoreTokens[] = T_SEMICOLON; - $ignoreTokens[] = T_COMMA; $nextContent = $phpcsFile->findNext($ignoreTokens, ($closeBrace + 1), null, true); if ($tokens[$nextContent]['content'] !== $phpcsFile->eolChar && $tokens[$nextContent]['line'] === $tokens[$closeBrace]['line'] diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc index 4cc2d4fb87..c2695983ec 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc @@ -331,3 +331,10 @@ interface ClassBraceNotOnLineByItselfTrailingCommentIsAllowed trait ClassBraceNotOnLineByItselfTrailingAnnotationIsAllowed { } // phpcs:ignore Stnd.Cat.Sniff -- this comment is also allowed. + +// Issue squizlabs/PHP_CodeSniffer#2621 - fix was superseded by fix for #2678. +$foo->bar( + new class implements Bar { + // ... + }, +); diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed index 1e306e373b..7bd99d3a3c 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed @@ -320,3 +320,10 @@ interface ClassBraceNotOnLineByItselfTrailingCommentIsAllowed trait ClassBraceNotOnLineByItselfTrailingAnnotationIsAllowed { } // phpcs:ignore Stnd.Cat.Sniff -- this comment is also allowed. + +// Issue squizlabs/PHP_CodeSniffer#2621 - fix was superseded by fix for #2678. +$foo->bar( + new class implements Bar { + // ... + }, +); From 1d4c9d2a33cdc247b985ed3f2dd22764413648f6 Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 16 Jul 2024 01:15:00 +0200 Subject: [PATCH 3/4] 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. --- src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php index 42cff38957..887c552edc 100644 --- a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php +++ b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php @@ -526,9 +526,7 @@ public function processClose(File $phpcsFile, $stackPtr) $ignoreTokens[] = T_COMMENT; $ignoreTokens[] = T_SEMICOLON; $nextContent = $phpcsFile->findNext($ignoreTokens, ($closeBrace + 1), null, true); - if ($tokens[$nextContent]['content'] !== $phpcsFile->eolChar - && $tokens[$nextContent]['line'] === $tokens[$closeBrace]['line'] - ) { + if ($tokens[$nextContent]['line'] === $tokens[$closeBrace]['line']) { $type = strtolower($tokens[$stackPtr]['content']); $error = 'Closing %s brace must be on a line by itself'; $data = [$type]; From 8303a3fc5d7bd0cb1da5c3c4fc844fae700a384b Mon Sep 17 00:00:00 2001 From: jrfnl Date: Tue, 16 Jul 2024 01:18:58 +0200 Subject: [PATCH 4/4] 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. --- .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc | 6 ++++++ .../PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed | 6 ++++++ .../PSR2/Tests/Classes/ClassDeclarationUnitTest.php | 1 + 3 files changed, 13 insertions(+) diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc index c2695983ec..1df40d5149 100644 --- a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc +++ b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc @@ -338,3 +338,9 @@ $foo->bar( // ... }, ); + +enum BraceNotOnLineByItselfCloseTagError +{ +} ?> + +bar( // ... }, ); + +enum BraceNotOnLineByItselfCloseTagError +{ +} ?> + + 1, 316 => 1, 324 => 1, + 344 => 1, ]; }//end getErrorList()