Improved tokenizing of fallthrough CASE and DEFAULT statements that s…

…hare a closing statement and use curly braces (ref #1558)
squizlabs · Jul 13, 2017 · cc5174a · cc5174a
1 parent 71f7468
commit cc5174a
Show file tree

Hide file tree

Showing 10 changed files with 140 additions and 19 deletions.
diff --git a/package.xml b/package.xml
@@ -36,6 +36,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
   - NOWDOC tokens are now considered conditions, just as HEREDOC tokens are
     -- This makes it easier to find the start and end of a NOWDOC from any token within it
     -- Thanks to Michał Bundyra for the patch
+  - Improved tokenizing of fallthrough CASE and DEFAULT statements that share a closing statement and use curly braces
   - Improved the error message when Squiz.ControlStructures.ControlSignature detects a newline after the closing parenthesis
   - Fixed bug #1465 : Generic.WhiteSpace.ScopeIndent reports incorrect errors when indenting double arrows in short arrays
   - Fixed bug #1478 : Indentation in fallthrough CASE that contains a closure

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc
@@ -1213,6 +1213,21 @@ return [
     ],
 ];
 
+switch ($sContext) {
+    case 'SOMETHING':
+    case 'CONSTANT':
+        do_something();
+        break;
+    case 'GLOBAL':
+    case 'GLOBAL1':
+        do_something();
+        // Fall through
+    default:
+    {
+        do_something();
+    }
+}
+
     function foo()
     {
         $foo = array(

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.1.inc.fixed
@@ -1213,6 +1213,21 @@ return [
     ],
 ];
 
+switch ($sContext) {
+    case 'SOMETHING':
+    case 'CONSTANT':
+        do_something();
+        break;
+    case 'GLOBAL':
+    case 'GLOBAL1':
+        do_something();
+        // Fall through
+    default:
+    {
+        do_something();
+    }
+}
+
 function foo()
 {
     $foo = array(

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc
@@ -1213,6 +1213,21 @@ return [
 	],
 ];
 
+switch ($sContext) {
+	case 'SOMETHING':
+	case 'CONSTANT':
+		do_something();
+		break;
+	case 'GLOBAL':
+	case 'GLOBAL1':
+		do_something();
+		// Fall through
+	default:
+	{
+		do_something();
+	}
+}
+
 	function foo()
 	{
 		$foo = array(

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.2.inc.fixed
@@ -1213,6 +1213,21 @@ return [
 	],
 ];
 
+switch ($sContext) {
+	case 'SOMETHING':
+	case 'CONSTANT':
+		do_something();
+		break;
+	case 'GLOBAL':
+	case 'GLOBAL1':
+		do_something();
+		// Fall through
+	default:
+	{
+		do_something();
+	}
+}
+
 function foo()
 {
 	$foo = array(

diff --git a/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php b/src/Standards/Generic/Tests/WhiteSpace/ScopeIndentUnitTest.php
@@ -141,14 +141,14 @@ public function getErrorList($testFile='ScopeIndentUnitTest.inc')
                 1163 => 1,
                 1197 => 1,
                 1198 => 1,
-                1216 => 1,
-                1221 => 1,
-                1223 => 1,
-                1226 => 1,
-                1230 => 1,
                 1231 => 1,
-                1232 => 1,
-                1233 => 1,
+                1236 => 1,
+                1238 => 1,
+                1241 => 1,
+                1245 => 1,
+                1246 => 1,
+                1247 => 1,
+                1248 => 1,
                );
 
     }//end getErrorList()

diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc
@@ -246,3 +246,19 @@ switch ($foo) {
     case 2:
         return 2;
 }
+
+switch ($sContext)
+{
+    case 'SOMETHING':
+    case 'CONSTANT':
+        do_something();
+        break;
+    case 'GLOBAL':
+    case 'GLOBAL1':
+        do_something();
+        // Fall through
+    default:
+    {
+        do_something();
+    }
+}
diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed
@@ -249,3 +249,19 @@ switch ($foo) {
     case 2:
         return 2;
 }
+
+switch ($sContext)
+{
+    case 'SOMETHING':
+    case 'CONSTANT':
+        do_something();
+        break;
+    case 'GLOBAL':
+    case 'GLOBAL1':
+        do_something();
+        // Fall through
+    default:
+    {
+        do_something();
+    }
+}
diff --git a/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php b/src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.php
@@ -46,6 +46,7 @@ public function getErrorList()
                 194 => 1,
                 224 => 1,
                 236 => 1,
+                260 => 1,
                );
 
     }//end getErrorList()

diff --git a/src/Tokenizers/PHP.php b/src/Tokenizers/PHP.php
@@ -1592,18 +1592,6 @@ protected function processAdditional()
                 $this->tokens[$index]['scope_closer']    = $newCloser;
             }
 
-            unset($this->tokens[$scopeOpener]['scope_condition']);
-            unset($this->tokens[$scopeOpener]['scope_opener']);
-            unset($this->tokens[$scopeOpener]['scope_closer']);
-            unset($this->tokens[$scopeCloser]['scope_condition']);
-            unset($this->tokens[$scopeCloser]['scope_opener']);
-            unset($this->tokens[$scopeCloser]['scope_closer']);
-            unset($this->tokens[$x]['bracket_opener']);
-            unset($this->tokens[$x]['bracket_closer']);
-            unset($this->tokens[$newCloser]['bracket_opener']);
-            unset($this->tokens[$newCloser]['bracket_closer']);
-            $this->tokens[$scopeCloser]['conditions'][] = $i;
-
             if (PHP_CODESNIFFER_VERBOSITY > 1) {
                 $line      = $this->tokens[$i]['line'];
                 $tokenType = $this->tokens[$i]['type'];
@@ -1617,6 +1605,45 @@ protected function processAdditional()
                 echo "\t* token $i ($tokenType) on line $line closer changed from $scopeCloser ($oldType) to $newCloser ($newType)".PHP_EOL;
             }
 
+            if ($this->tokens[$scopeOpener]['scope_condition'] === $i) {
+                unset($this->tokens[$scopeOpener]['scope_condition']);
+                unset($this->tokens[$scopeOpener]['scope_opener']);
+                unset($this->tokens[$scopeOpener]['scope_closer']);
+            }
+
+            if ($this->tokens[$scopeCloser]['scope_condition'] === $i) {
+                unset($this->tokens[$scopeCloser]['scope_condition']);
+                unset($this->tokens[$scopeCloser]['scope_opener']);
+                unset($this->tokens[$scopeCloser]['scope_closer']);
+            } else {
+                // We were using a shared closer. All tokens that were
+                // sharing this closer with us, except for the scope condition
+                // and it's opener, need to now point to the new closer.
+                $condition = $this->tokens[$scopeCloser]['scope_condition'];
+                $start     = ($this->tokens[$condition]['scope_opener'] + 1);
+                for ($y = $start; $y < $scopeCloser; $y++) {
+                    if (isset($this->tokens[$y]['scope_closer']) === true
+                        && $this->tokens[$y]['scope_closer'] === $scopeCloser
+                    ) {
+                        $this->tokens[$y]['scope_closer'] = $newCloser;
+
+                        if (PHP_CODESNIFFER_VERBOSITY > 1) {
+                            $line      = $this->tokens[$y]['line'];
+                            $tokenType = $this->tokens[$y]['type'];
+                            $oldType   = $this->tokens[$scopeCloser]['type'];
+                            $newType   = $this->tokens[$newCloser]['type'];
+                            echo "\t\t* token $y ($tokenType) on line $line closer changed from $scopeCloser ($oldType) to $newCloser ($newType)".PHP_EOL;
+                        }
+                    }
+                }
+            }//end if
+
+            unset($this->tokens[$x]['bracket_opener']);
+            unset($this->tokens[$x]['bracket_closer']);
+            unset($this->tokens[$newCloser]['bracket_opener']);
+            unset($this->tokens[$newCloser]['bracket_closer']);
+            $this->tokens[$scopeCloser]['conditions'][] = $i;
+
             // Now fix up all the tokens that think they are
             // inside the CASE/DEFAULT statement when they are really outside.
             for ($x = $newCloser; $x < $scopeCloser; $x++) {