PSR2/SwitchDeclaration: bug fix - improve handling of comments

Comments in certain places in the code flow would throw the `findNestedTerminator()` method used for determining the `TerminatingComment` error off. Fixed now. Includes unit tests. Without this fix, the first test would not throw an error (false negative), while the second would (false positive).
squizlabs · May 27, 2021 · 8c9feda · 8c9feda
1 parent c3eab21
commit 8c9feda
Show file tree

Hide file tree

Showing 4 changed files with 56 additions and 3 deletions.
diff --git a/src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php b/src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php
@@ -264,7 +264,7 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
                 $scopeOpener = $tokens[$currentCloser]['scope_opener'];
                 $scopeCloser = $tokens[$currentCloser]['scope_closer'];
 
-                $prevToken = $phpcsFile->findPrevious(T_WHITESPACE, ($scopeOpener - 1), $stackPtr, true);
+                $prevToken = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($scopeOpener - 1), $stackPtr, true);
                 if ($prevToken === false) {
                     return false;
                 }
@@ -292,7 +292,7 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
                         return false;
                     }
 
-                    $currentCloser = $phpcsFile->findPrevious(T_WHITESPACE, ($prevToken - 1), $stackPtr, true);
+                    $currentCloser = $phpcsFile->findPrevious(Tokens::$emptyTokens, ($prevToken - 1), $stackPtr, true);
                     if ($tokens[$prevToken]['code'] === T_ELSE) {
                         $hasElseBlock = true;
                     }
@@ -309,7 +309,7 @@ private function findNestedTerminator($phpcsFile, $stackPtr, $end)
 
                         $opener = $tokens[$nextCase]['scope_opener'];
 
-                        $nextCode = $phpcsFile->findNext(T_WHITESPACE, ($opener + 1), $endOfSwitch, true);
+                        $nextCode = $phpcsFile->findNext(Tokens::$emptyTokens, ($opener + 1), $endOfSwitch, true);
                         if ($tokens[$nextCode]['code'] === T_CASE || $tokens[$nextCode]['code'] === T_DEFAULT) {
                             // This case statement has no content, so skip it.
                             continue;

diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc
@@ -378,3 +378,29 @@ switch ( $test ) {
 
         break;
 }
+
+// Handle comments correctly.
+switch ($foo) {
+    case 1:
+        if ($bar > 0) {
+            doSomething();
+        }
+        // Comment
+        else {
+            return 1;
+        }
+    case 2:
+        return 2;
+}
+
+switch ($foo) {
+    case 1:
+        if ($bar > 0) /*comment*/ {
+            return doSomething();
+        }
+        else {
+            return 1;
+        }
+    case 2:
+        return 2;
+}
diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed
@@ -373,3 +373,29 @@ switch ( $test ) {
 
         break;
 }
+
+// Handle comments correctly.
+switch ($foo) {
+    case 1:
+        if ($bar > 0) {
+            doSomething();
+        }
+        // Comment
+        else {
+            return 1;
+        }
+    case 2:
+        return 2;
+}
+
+switch ($foo) {
+    case 1:
+        if ($bar > 0) /*comment*/ {
+            return doSomething();
+        }
+        else {
+            return 1;
+        }
+    case 2:
+        return 2;
+}
diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php
@@ -53,6 +53,7 @@ public function getErrorList()
             350 => 1,
             356 => 1,
             362 => 1,
+            384 => 1,
         ];
 
     }//end getErrorList()