Squiz/NonExecutableCode: fix false positives when code goes in and out of PHP/HTML (#3770)

Ignore HTML whitespace and PHP re-open tags when determining whether code should be flagged as non executable.

Author: fredden

package.xml

@@ -1940,6 +1940,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
         <file baseinstalldir="PHP/CodeSniffer" name="LowercasePHPFunctionsUnitTest.php" role="test" />
         <file baseinstalldir="PHP/CodeSniffer" name="NonExecutableCodeUnitTest.1.inc" role="test" />
         <file baseinstalldir="PHP/CodeSniffer" name="NonExecutableCodeUnitTest.2.inc" role="test" />
+        <file baseinstalldir="PHP/CodeSniffer" name="NonExecutableCodeUnitTest.3.inc" role="test" />
         <file baseinstalldir="PHP/CodeSniffer" name="NonExecutableCodeUnitTest.php" role="test" />
        </dir>
        <dir name="Scope">

src/Standards/Squiz/Sniffs/PHP/NonExecutableCodeSniff.php

@@ -241,8 +241,8 @@ public function process(File $phpcsFile, $stackPtr)
             $end = ($phpcsFile->numTokens - 1);
         }//end if
 
-        // Find the semicolon that ends this statement, skipping
-        // nested statements like FOR loops and closures.
+        // Find the semicolon or closing PHP tag that ends this statement,
+        // skipping nested statements like FOR loops and closures.
         for ($start = ($stackPtr + 1); $start < $phpcsFile->numTokens; $start++) {
             if ($start === $end) {
                 break;
@@ -262,7 +262,7 @@ public function process(File $phpcsFile, $stackPtr)
                 continue;
             }
 
-            if ($tokens[$start]['code'] === T_SEMICOLON) {
+            if ($tokens[$start]['code'] === T_SEMICOLON || $tokens[$start]['code'] === T_CLOSE_TAG) {
                 break;
             }
         }//end for
@@ -295,6 +295,16 @@ public function process(File $phpcsFile, $stackPtr)
                 continue;
             }
 
+            // Skip HTML whitespace.
+            if ($tokens[$i]['code'] === T_INLINE_HTML && \trim($tokens[$i]['content']) === '') {
+                continue;
+            }
+
+            // Skip PHP re-open tag (eg, after inline HTML).
+            if ($tokens[$i]['code'] === T_OPEN_TAG) {
+                continue;
+            }
+
             $line = $tokens[$i]['line'];
             if ($line > $lastLine) {
                 $type    = substr($tokens[$stackPtr]['type'], 2);

src/Standards/Squiz/Tests/PHP/NonExecutableCodeUnitTest.2.inc

@@ -58,4 +58,16 @@ $a = new class {
     }
 };
 
+// Multiple statements are still one line of unreachable code, so should get
+// only one complaint from this sniff. (Well, technically two here since there
+// are two 'exit()' statements above, so one complaint from each of those. So,
+// two here, but not six.)
+echo 'one'; echo 'two'; echo 'three';
+
+// A single statement split across multiple lines. Here we get complaints for
+// each line, even though they're all part of one statement.
+echo 'one' . 'two'
+    . 'three' . 'four'
+    . 'five' . 'six';
+
 interface MyInterface {

src/Standards/Squiz/Tests/PHP/NonExecutableCodeUnitTest.3.inc

@@ -0,0 +1,64 @@
+<!-- no problem here -->
+<?php if (true): ?>
+    <?php foreach ([] as $item): ?>
+        <?php continue; ?>
+    <?php endforeach; ?>
+<?php endif; ?>
+
+<!-- no problem here -->
+<?php if (true) { ?>
+    <?php foreach ([] as $item) { ?>
+        <?php continue; ?>
+    <?php } ?>
+<?php } ?>
+
+<!-- no problem here -->
+<?php if (true) { ?>
+    <?php foreach ([] as $item) { ?>
+        <!-- note missing semicolon on next line -->
+        <?php continue ?>
+    <?php } ?>
+<?php } ?>
+
+<!-- should detect an error here -->
+<?php if (true): ?>
+    <?php foreach ([] as $item): ?>
+        <?php continue; ?>
+        <div>non-executable</div>
+    <?php endforeach; ?>
+<?php endif; ?>
+
+<!-- should detect an error here -->
+<?php if (true): ?>
+    <?php foreach ([] as $item): ?>
+        <!-- note missing semicolon on next line -->
+        <?php continue ?>
+        <div>non-executable</div>
+    <?php endforeach; ?>
+<?php endif; ?>
+
+<!-- should detect an error here -->
+<?php if (true): ?>
+    <?php foreach ([] as $item): ?>
+        <?php continue; ?>
+
+        <div>non-executable</div>
+
+    <?php endforeach; ?>
+<?php endif; ?>
+
+<!-- should detect an error here -->
+<?php if (true): ?>
+    <?php foreach ([] as $item): ?>
+        <?php continue; ?>
+        <?= 'unreachable - no semicolon' ?>
+    <?php endforeach; ?>
+<?php endif; ?>
+
+<!-- should detect an error here -->
+<?php if (true): ?>
+    <?php foreach ([] as $item): ?>
+        <?php continue; ?>
+        <?= 'unreachable - with semicolon'; ?>
+    <?php endforeach; ?>
+<?php endif; ?>

src/Standards/Squiz/Tests/PHP/NonExecutableCodeUnitTest.php

@@ -92,8 +92,20 @@ public function getWarningList($testFile='')
                 10 => 2,
                 14 => 1,
                 54 => 2,
+                65 => 2,
+                69 => 2,
+                70 => 2,
+                71 => 2,
             ];
             break;
+        case 'NonExecutableCodeUnitTest.3.inc':
+            return [
+                27 => 1,
+                36 => 1,
+                45 => 1,
+                54 => 1,
+                62 => 1,
+            ];
         default:
             return [];
             break;