PSR2/ClassDeclaration: bug fix - prevent fixer conflict

Yet another one in the fixer conflict series. When running the `Squiz` standard over all test case files, a fixer conflict between the `Squiz.Classes.ClassDeclaration`, which extends the `PSR2.Classes.ClassDeclaration` sniff, and the `Squiz.Commenting.InlineComment` sniff was discovered via the tests in the `tests\Core\Tokenizer\UndoNamespacedNameSingleTokenTest.inc` file for a code sample like this: ```php class MyClass /* testExtendedFQN */ extends \Vendor\Level\FQN /* testImplementsRelative */ implements namespace\Name, /* testImplementsFQN */ \Fully\Qualified, /* testImplementsUnqualified */ Unqualified, /* testImplementsPartiallyQualified */ Sub\Level\Name {} ``` The conflict essentially comes down to the `PSR2.Classes.ClassDeclaration` sniff adding a new line between an inline comment (which is already on its own line) and the name of an interface being implemented, which the `Squiz.Commenting.InlineComment` sniff would then remove again. Isolating the code sample even further, the conflict can be reproduced even without the `Squiz.Commenting.InlineComment` sniff. The short of it was, that the `PSR2.Classes.ClassDeclaration` sniff did not take into account that there could be comments between the interface names in a multi-line `implements` and would then get its knickers in a twist. Original errors for the code sample added in the test case file: ``` 302 | ERROR | [x] Only one interface may be specified per line in a multi-line implements declaration | | (PSR2.Classes.ClassDeclaration.InterfaceSameLine) 310 | ERROR | [x] Only one interface may be specified per line in a multi-line implements declaration | | (PSR2.Classes.ClassDeclaration.InterfaceSameLine) 316 | ERROR | [x] Only one interface may be specified per line in a multi-line implements declaration | | (PSR2.Classes.ClassDeclaration.InterfaceSameLine) ``` The fix proposed in this PR changes the logic of the sniff to take comments between interface names into account in the following manner: * If there is a comment before the interface name and the comment is on the same line, move the interface name to a new line via the `InterfaceSameLine` error. * However, if there is a comment before the interface name and the comment is on not on the same line, we know that the interface name is already on its own line, so don't trigger the `InterfaceSameLine` error. Other potential errors (like indentation) will continue to be checked for. With these changes, the code sample in the test case file now yields the following errors: ``` 310 | ERROR | [x] Expected 4 spaces before interface name; 0 found (PSR2.Classes.ClassDeclaration.InterfaceWrongIndent) 316 | ERROR | [x] Only one interface may be specified per line in a multi-line implements declaration | | (PSR2.Classes.ClassDeclaration.InterfaceSameLine) ``` This effectively fixes the fixer conflict. --- <details> <summary>Fixer Conflict details</summary> ``` * fixed 0 violations, starting loop 48 * => Changeset started by PSR2.Classes.ClassDeclaration:362 Q: PSR2.Classes.ClassDeclaration:368 replaced token 1078 (T_WHITESPACE on line 289) "\n\" => "\" Q: PSR2.Classes.ClassDeclaration:371 replaced token 1077 (T_PHPCS_DISABLE on line 288) "// phpcs:disable Stnd.Cat.Sniff -- For reasons.\n" => "// phpcs:disable Stnd.Cat.Sniff -- For reasons.\n\n" A: PSR2.Classes.ClassDeclaration:372 replaced token 1078 (T_WHITESPACE on line 289) "\n\" => "\" A: PSR2.Classes.ClassDeclaration:372 replaced token 1077 (T_PHPCS_DISABLE on line 288) "// phpcs:disable Stnd.Cat.Sniff -- For reasons.\n" => "// phpcs:disable Stnd.Cat.Sniff -- For reasons.\n\n" => Changeset ended: 2 changes applied => Changeset started by PSR2.Classes.ClassDeclaration:362 Q: PSR2.Classes.ClassDeclaration:368 replaced token 1099 (T_WHITESPACE on line 297) "\nAnotherInterface" => "AnotherInterface" Q: PSR2.Classes.ClassDeclaration:371 replaced token 1098 (T_COMMENT on line 296) "// Comment.\n" => "// Comment.\n\n" A: PSR2.Classes.ClassDeclaration:372 replaced token 1099 (T_WHITESPACE on line 297) "\nAnotherInterface" => "AnotherInterface" A: PSR2.Classes.ClassDeclaration:372 replaced token 1098 (T_COMMENT on line 296) "// Comment.\n" => "// Comment.\n\n" => Changeset ended: 2 changes applied => Fixing file: 4/53 violations remaining [made 48 passes]... * fixed 4 violations, starting loop 49 * => Changeset started by PSR2.Classes.ClassDeclaration:362 Q: PSR2.Classes.ClassDeclaration:368 replaced token 1078 (T_WHITESPACE on line 289) "\n\" => "\" Q: PSR2.Classes.ClassDeclaration:371 replaced token 1077 (T_PHPCS_DISABLE on line 288) "// phpcs:disable Stnd.Cat.Sniff -- For reasons.\n" => "// phpcs:disable Stnd.Cat.Sniff -- For reasons.\n\n" **** PSR2.Classes.ClassDeclaration:372 has possible conflict with another sniff on loop 47; caused by the following change **** **** replaced token 1078 (T_WHITESPACE on line 289) "\n\" => "\" **** **** ignoring all changes until next loop **** => Changeset failed to apply => Fixing file: 0/53 violations remaining [made 49 passes]... ``` </details> --- Related to 152 Related to 425
PHPCSStandards · Apr 6, 2024 · 0b4f3e0 · 0b4f3e0
1 parent b131294
commit 0b4f3e0
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 1 deletion.
diff --git a/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php b/src/Standards/PSR2/Sniffs/Classes/ClassDeclarationSniff.php
@@ -349,7 +349,10 @@ public function processOpen(File $phpcsFile, $stackPtr)
                         $phpcsFile->fixer->addNewline($prev);
                         $phpcsFile->fixer->endChangeset();
                     }
-                } else if ($tokens[$prev]['line'] !== ($tokens[$className]['line'] - 1)) {
+                } else if ((isset(Tokens::$commentTokens[$tokens[$prev]['code']]) === false
+                    && $tokens[$prev]['line'] !== ($tokens[$className]['line'] - 1))
+                    || $tokens[$prev]['line'] === $tokens[$className]['line']
+                ) {
                     if ($keywordTokenType === T_EXTENDS) {
                         $error = 'Only one interface may be specified per line in a multi-line extends declaration';
                         $fix   = $phpcsFile->addFixableError($error, $className, 'ExtendsInterfaceSameLine');

diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc
@@ -293,3 +293,26 @@ class BarFoo implements
     namespace\BarFoo
 {
 }
+
+// Safeguard that the sniff ignores comments between interface names in a multiline implements.
+class ClassWithMultiLineImplementsAndIgnoreAnnotation implements
+    SomeInterface,
+    // phpcs:disable Stnd.Cat.Sniff -- For reasons.
+
+    \AnotherInterface
+{
+}
+
+class ClassWithMultiLineImplementsAndComment implements
+    SomeInterface,
+    // Comment.
+
+AnotherInterface
+{
+}
+
+class ClassWithMultiLineImplementsAndCommentOnSameLineAsInterfaceName implements
+    SomeInterface,
+    /* Comment. */ AnotherInterface
+{
+}
diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.inc.fixed
@@ -281,3 +281,27 @@ class BarFoo implements
     namespace\BarFoo
 {
 }
+
+// Safeguard that the sniff ignores comments between interface names in a multiline implements.
+class ClassWithMultiLineImplementsAndIgnoreAnnotation implements
+    SomeInterface,
+    // phpcs:disable Stnd.Cat.Sniff -- For reasons.
+
+    \AnotherInterface
+{
+}
+
+class ClassWithMultiLineImplementsAndComment implements
+    SomeInterface,
+    // Comment.
+
+    AnotherInterface
+{
+}
+
+class ClassWithMultiLineImplementsAndCommentOnSameLineAsInterfaceName implements
+    SomeInterface,
+    /* Comment. */
+    AnotherInterface
+{
+}
diff --git a/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php b/src/Standards/PSR2/Tests/Classes/ClassDeclarationUnitTest.php
@@ -76,6 +76,8 @@ public function getErrorList()
             273 => 1,
             276 => 1,
             282 => 1,
+            310 => 1,
+            316 => 1,
         ];
 
     }//end getErrorList()