Merge pull request #1316 from PHPCSStandards/feature/tokenizer-php-switch-case-default-can-start-body-with-php-close-tag

PHP close tag should be regarded as scope opener for switch case/default statements

Author: jrfnl

src/Standards/Generic/Tests/CodeAnalysis/AssignmentInConditionUnitTest.1.inc

@@ -77,7 +77,7 @@ switch ( true ) {
 	case $sample = 'something':
 		break;
 
-	case $sample = 'something' && $a = $b:
+	case $sample = 'something' && $a = $b;
 		break;
 }
 
@@ -93,3 +93,14 @@ if ($a = 123) :
 endif;
 
 match ($a[0] = 123) {};
+
+switch ( true ) {
+    case $sample = 'something': {
+        break;
+    }
+    case $sample ?>
+        <?php $a = $b ?><!-- This is okay, this is in the case body, not the condition. -->
+        <?php break;
+    case $sample = 'something' && $a = $b ?>
+        <?php break;
+}

src/Standards/Generic/Tests/CodeAnalysis/AssignmentInConditionUnitTest.php

@@ -50,40 +50,42 @@ public function getWarningList($testFile = '')
         switch ($testFile) {
             case 'AssignmentInConditionUnitTest.1.inc':
                 return [
-                    46 => 1,
-                    47 => 1,
-                    48 => 1,
-                    49 => 1,
-                    50 => 1,
-                    51 => 1,
-                    52 => 1,
-                    53 => 1,
-                    54 => 1,
-                    55 => 1,
-                    56 => 1,
-                    57 => 1,
-                    58 => 1,
-                    59 => 1,
-                    60 => 1,
-                    61 => 2,
-                    63 => 1,
-                    64 => 1,
-                    67 => 1,
-                    68 => 1,
-                    69 => 1,
-                    70 => 1,
-                    71 => 1,
-                    72 => 1,
-                    73 => 1,
-                    75 => 1,
-                    77 => 1,
-                    80 => 2,
-                    84 => 1,
-                    85 => 2,
-                    88 => 1,
-                    90 => 1,
-                    92 => 1,
-                    95 => 1,
+                    46  => 1,
+                    47  => 1,
+                    48  => 1,
+                    49  => 1,
+                    50  => 1,
+                    51  => 1,
+                    52  => 1,
+                    53  => 1,
+                    54  => 1,
+                    55  => 1,
+                    56  => 1,
+                    57  => 1,
+                    58  => 1,
+                    59  => 1,
+                    60  => 1,
+                    61  => 2,
+                    63  => 1,
+                    64  => 1,
+                    67  => 1,
+                    68  => 1,
+                    69  => 1,
+                    70  => 1,
+                    71  => 1,
+                    72  => 1,
+                    73  => 1,
+                    75  => 1,
+                    77  => 1,
+                    80  => 2,
+                    84  => 1,
+                    85  => 2,
+                    88  => 1,
+                    90  => 1,
+                    92  => 1,
+                    95  => 1,
+                    98  => 1,
+                    104 => 2,
                 ];
 
             default:

src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.1.inc

@@ -458,3 +458,32 @@ abstract class AbstractClass {
 interface MyInterface {
     public function sniffShouldIgnoreInterfaceMethods();
 }
+
+function complexityElevenWithSwitchCaseCreativeScopeOpeners()
+{
+    switch ($condition) {
+        case 'colon':
+            break;
+        case 'semicolon';
+            return;
+        case 'colon with curlies': {
+            return;
+        }
+        case 'semicolon with curlies'; {
+            return;
+        }
+        case 'PHP close tag' ?>
+            <?php return ?>
+        <?php case 'PHP close tag with semicolon'; ?>
+            <?php return ?>
+        <?php case 'PHP close tag 2' ?>
+            <?php goto LABEL ?>
+        <?php case 'PHP close tag with semicolon'; ?>
+            <?php exit ?>
+        <?php case 'PHP close tag 3' ?>
+            <?php return ?>
+        <?php default ?>
+            <?php break ?>
+        <?php
+    }
+}

src/Standards/Generic/Tests/Metrics/CyclomaticComplexityUnitTest.php

@@ -66,6 +66,7 @@ public function getWarningList($testFile = '')
                     381 => 1,
                     417 => 1,
                     445 => 1,
+                    462 => 1,
                 ];
             default:
                 return [];

src/Standards/PEAR/Tests/WhiteSpace/ScopeClosingBraceUnitTest.inc

@@ -168,3 +168,33 @@ enum Cards
 <?php if ($someConditional) : ?>
 <div class="container">
 </div> <?php endif;
+
+function switchCaseCreativeScopeOpeners()
+{
+    switch ($correct) {
+        case 'colon':
+            break;
+        case 'semicolon';
+            return;
+        case 'PHP close tag' ?>
+            <?php return ?>
+        <?php case 'PHP close tag with semicolon'; ?>
+            <?php exit ?>
+        <?php case 'PHP close tag 3' ?>
+            <?php return ?>
+        <?php default ?>
+            <?php break ?>
+        <?php
+    }
+
+    switch ($incorrect) {
+        case 'semicolon'; return;
+        case 'PHP close tag' ?>
+    <?php return ?>
+        <?php case 'PHP close tag with semicolon'; ?>
+              <?php exit ?>
+        <?php default ?>
+        <?php break ?>
+        <?php
+      }
+}

src/Standards/PEAR/Tests/WhiteSpace/ScopeClosingBraceUnitTest.inc.fixed

@@ -175,3 +175,34 @@ enum Cards
 <div class="container">
 </div> <?php 
 endif;
+
+function switchCaseCreativeScopeOpeners()
+{
+    switch ($correct) {
+        case 'colon':
+            break;
+        case 'semicolon';
+            return;
+        case 'PHP close tag' ?>
+            <?php return ?>
+        <?php case 'PHP close tag with semicolon'; ?>
+            <?php exit ?>
+        <?php case 'PHP close tag 3' ?>
+            <?php return ?>
+        <?php default ?>
+            <?php break ?>
+        <?php
+    }
+
+    switch ($incorrect) {
+        case 'semicolon'; 
+            return;
+        case 'PHP close tag' ?>
+            <?php return ?>
+        <?php case 'PHP close tag with semicolon'; ?>
+            <?php exit ?>
+        <?php default ?>
+            <?php break ?>
+        <?php
+    }
+}

src/Standards/PEAR/Tests/WhiteSpace/ScopeClosingBraceUnitTest.php

@@ -51,6 +51,11 @@ public function getErrorList()
             160 => 1,
             164 => 1,
             170 => 1,
+            191 => 1,
+            193 => 1,
+            195 => 1,
+            197 => 1,
+            199 => 1,
         ];
     }
 

src/Standards/PSR2/Sniffs/ControlStructures/SwitchDeclarationSniff.php

@@ -186,10 +186,15 @@ public function process(File $phpcsFile, int $stackPtr)
                 }
             } else {
                 $error = strtoupper($type) . ' statements must be defined using a colon';
-                if ($tokens[$opener]['code'] === T_SEMICOLON) {
+                if ($tokens[$opener]['code'] === T_SEMICOLON || $tokens[$opener]['code'] === T_CLOSE_TAG) {
                     $fix = $phpcsFile->addFixableError($error, $nextCase, 'WrongOpener' . $type);
                     if ($fix === true) {
-                        $phpcsFile->fixer->replaceToken($opener, ':');
+                        if ($tokens[$opener]['code'] === T_SEMICOLON) {
+                            $phpcsFile->fixer->replaceToken($opener, ':');
+                        } else {
+                            $prevNonEmpty = $phpcsFile->findPrevious(T_WHITESPACE, ($opener - 1), null, true);
+                            $phpcsFile->fixer->addContent($prevNonEmpty, ':');
+                        }
                     }
                 } else {
                     // Probably a case/default statement with colon + curly braces.

src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc

@@ -622,3 +622,19 @@ switch ($value) {
     default ;
         echo 'Other';
 }
+
+// Verify that case/default statements with a PHP close tag as scope opener are handled correctly by the sniff.
+switch ($foo):
+    case "foo"?>
+        <div>Some html</div>
+        <?php break;
+    case "bar" ; ?>
+        <div>Some html</div>
+        <?php break;
+    default ?>
+        <div>Some html</div>
+        <?php break;
+    default ; ?>
+        <div>Some html</div>
+        <?php break;
+endswitch;

src/Standards/PSR2/Tests/ControlStructures/SwitchDeclarationUnitTest.inc.fixed

@@ -617,3 +617,27 @@ switch ($value) {
     default:
         echo 'Other';
 }
+
+// Verify that case/default statements with a PHP close tag as scope opener are handled correctly by the sniff.
+switch ($foo):
+    case "foo":
+        ?>
+        <div>Some html</div>
+        <?php
+        break;
+    case "bar": 
+        ?>
+        <div>Some html</div>
+        <?php
+        break;
+    default: 
+        ?>
+        <div>Some html</div>
+        <?php
+        break;
+    default: 
+        ?>
+        <div>Some html</div>
+        <?php
+        break;
+endswitch;