Generic/InlineControlStructure: fix two bugs and improve code coverage

src/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php

@@ -48,7 +48,6 @@ public function register()
             T_FOREACH,
             T_WHILE,
             T_DO,
-            T_SWITCH,
             T_FOR,
         ];
 
@@ -75,14 +74,14 @@ public function process(File $phpcsFile, $stackPtr)
 
         // Ignore the ELSE in ELSE IF. We'll process the IF part later.
         if ($tokens[$stackPtr]['code'] === T_ELSE) {
-            $next = $phpcsFile->findNext(T_WHITESPACE, ($stackPtr + 1), null, true);
+            $next = $phpcsFile->findNext(Tokens::$emptyTokens, ($stackPtr + 1), null, true);
             if ($tokens[$next]['code'] === T_IF) {
                 return;
             }
         }
 
-        if ($tokens[$stackPtr]['code'] === T_WHILE || $tokens[$stackPtr]['code'] === T_FOR) {
-            // This could be from a DO WHILE, which doesn't have an opening brace or a while/for without body.
+        if (in_array($tokens[$stackPtr]['code'], [T_ELSEIF, T_IF, T_FOR, T_FOREACH, T_WHILE], true) === true) {
+            // This could be from a DO WHILE, which doesn't have an opening brace, or an elseif/if/for/foreach/while without body.
             if (isset($tokens[$stackPtr]['parenthesis_closer']) === true) {
                 $afterParensCloser = $phpcsFile->findNext(Tokens::$emptyTokens, ($tokens[$stackPtr]['parenthesis_closer'] + 1), null, true);
                 if ($afterParensCloser === false) {
@@ -95,21 +94,6 @@ public function process(File $phpcsFile, $stackPtr)
                     return;
                 }
             }
-
-            // In Javascript DO WHILE loops without curly braces are legal. This
-            // is only valid if a single statement is present between the DO and
-            // the WHILE. We can detect this by checking only a single semicolon
-            // is present between them.
-            if ($tokens[$stackPtr]['code'] === T_WHILE && $phpcsFile->tokenizerType === 'JS') {
-                $lastDo        = $phpcsFile->findPrevious(T_DO, ($stackPtr - 1));
-                $lastSemicolon = $phpcsFile->findPrevious(T_SEMICOLON, ($stackPtr - 1));
-                if ($lastDo !== false && $lastSemicolon !== false && $lastDo < $lastSemicolon) {
-                    $precedingSemicolon = $phpcsFile->findPrevious(T_SEMICOLON, ($lastSemicolon - 1));
-                    if ($precedingSemicolon === false || $precedingSemicolon < $lastDo) {
-                        return;
-                    }
-                }
-            }
         }//end if
 
         if (isset($tokens[$stackPtr]['parenthesis_opener'], $tokens[$stackPtr]['parenthesis_closer']) === false
@@ -270,10 +254,6 @@ public function process(File $phpcsFile, $stackPtr)
             }
         }//end for
 
-        if ($end === $phpcsFile->numTokens) {
-            $end = $lastNonEmpty;
-        }
-
         $nextContent = $phpcsFile->findNext(Tokens::$emptyTokens, ($end + 1), null, true);
         if ($nextContent === false || $tokens[$nextContent]['line'] !== $tokens[$end]['line']) {
             // Looks for completely empty statements.
@@ -283,94 +263,66 @@ public function process(File $phpcsFile, $stackPtr)
             $endLine = $end;
         }
 
-        if ($next !== $end) {
-            if ($nextContent === false || $tokens[$nextContent]['line'] !== $tokens[$end]['line']) {
-                // Account for a comment on the end of the line.
-                for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
-                    if (isset($tokens[($endLine + 1)]) === false
-                        || $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
-                    ) {
-                        break;
-                    }
-                }
-
-                if (isset(Tokens::$commentTokens[$tokens[$endLine]['code']]) === false
-                    && ($tokens[$endLine]['code'] !== T_WHITESPACE
-                    || isset(Tokens::$commentTokens[$tokens[($endLine - 1)]['code']]) === false)
-                ) {
-                    $endLine = $end;
-                }
-            }
-
-            if ($endLine !== $end) {
-                $endToken     = $endLine;
-                $addedContent = '';
-            } else {
-                $endToken     = $end;
-                $addedContent = $phpcsFile->eolChar;
-
-                if ($tokens[$end]['code'] !== T_SEMICOLON
-                    && $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
+        if ($nextContent === false || $tokens[$nextContent]['line'] !== $tokens[$end]['line']) {
+            // Account for a comment on the end of the line.
+            for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
+                if (isset($tokens[($endLine + 1)]) === false
+                    || $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
                 ) {
-                    $phpcsFile->fixer->addContent($end, '; ');
+                    break;
                 }
             }
 
-            $next = $phpcsFile->findNext(T_WHITESPACE, ($endToken + 1), null, true);
-            if ($next !== false
-                && ($tokens[$next]['code'] === T_ELSE
-                || $tokens[$next]['code'] === T_ELSEIF)
+            if (isset(Tokens::$commentTokens[$tokens[$endLine]['code']]) === false
+                && ($tokens[$endLine]['code'] !== T_WHITESPACE
+                || isset(Tokens::$commentTokens[$tokens[($endLine - 1)]['code']]) === false)
             ) {
-                $phpcsFile->fixer->addContentBefore($next, '} ');
-            } else {
-                $indent = '';
-                for ($first = $stackPtr; $first > 0; $first--) {
-                    if ($tokens[$first]['column'] === 1) {
-                        break;
-                    }
-                }
+                $endLine = $end;
+            }
+        }
 
-                if ($tokens[$first]['code'] === T_WHITESPACE) {
-                    $indent = $tokens[$first]['content'];
-                } else if ($tokens[$first]['code'] === T_INLINE_HTML
-                    || $tokens[$first]['code'] === T_OPEN_TAG
-                ) {
-                    $addedContent = '';
-                }
+        if ($endLine !== $end) {
+            $endToken     = $endLine;
+            $addedContent = '';
+        } else {
+            $endToken     = $end;
+            $addedContent = $phpcsFile->eolChar;
 
-                $addedContent .= $indent.'}';
-                if ($next !== false && $tokens[$endToken]['code'] === T_COMMENT) {
-                    $addedContent .= $phpcsFile->eolChar;
-                }
+            if ($tokens[$end]['code'] !== T_SEMICOLON
+                && $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
+            ) {
+                $phpcsFile->fixer->addContent($end, '; ');
+            }
+        }
 
-                $phpcsFile->fixer->addContent($endToken, $addedContent);
-            }//end if
+        $next = $phpcsFile->findNext(T_WHITESPACE, ($endToken + 1), null, true);
+        if ($next !== false
+            && ($tokens[$next]['code'] === T_ELSE
+            || $tokens[$next]['code'] === T_ELSEIF)
+        ) {
+            $phpcsFile->fixer->addContentBefore($next, '} ');
         } else {
-            if ($nextContent === false || $tokens[$nextContent]['line'] !== $tokens[$end]['line']) {
-                // Account for a comment on the end of the line.
-                for ($endLine = $end; $endLine < $phpcsFile->numTokens; $endLine++) {
-                    if (isset($tokens[($endLine + 1)]) === false
-                        || $tokens[$endLine]['line'] !== $tokens[($endLine + 1)]['line']
-                    ) {
-                        break;
-                    }
+            $indent = '';
+            for ($first = $stackPtr; $first > 0; $first--) {
+                if ($tokens[$first]['column'] === 1) {
+                    break;
                 }
+            }
 
-                if ($tokens[$endLine]['code'] !== T_COMMENT
-                    && ($tokens[$endLine]['code'] !== T_WHITESPACE
-                    || $tokens[($endLine - 1)]['code'] !== T_COMMENT)
-                ) {
-                    $endLine = $end;
-                }
+            if ($tokens[$first]['code'] === T_WHITESPACE) {
+                $indent = $tokens[$first]['content'];
+            } else if ($tokens[$first]['code'] === T_INLINE_HTML
+                || $tokens[$first]['code'] === T_OPEN_TAG
+            ) {
+                $addedContent = '';
             }
 
-            if ($endLine !== $end) {
-                $phpcsFile->fixer->replaceToken($end, '');
-                $phpcsFile->fixer->addNewlineBefore($endLine);
-                $phpcsFile->fixer->addContent($endLine, '}');
-            } else {
-                $phpcsFile->fixer->replaceToken($end, '}');
+            $addedContent .= $indent.'}';
+            if ($next !== false && $tokens[$endToken]['code'] === T_COMMENT) {
+                $addedContent .= $phpcsFile->eolChar;
             }
+
+            $phpcsFile->fixer->addContent($endToken, $addedContent);
         }//end if
 
         $phpcsFile->fixer->endChangeset();

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc

@@ -272,3 +272,30 @@ function testFinally()
         } finally {
         }
 }
+
+if ($something) {
+    echo 'hello';
+} else /* comment */ if ($somethingElse) echo 'hi';
+
+if ($shouldNotTriggerSniff);
+
+if (false) {
+} elseif ($shouldNotTriggerSniff);
+
+if (false) {
+} else if ($shouldNotTriggerSniff);
+
+if (false) {
+} else ($shouldTriggerSniff);
+
+foreach ($array as $shouldNotTriggerSniff);
+
+do echo $i++; while ($i < 5);
+
+// phpcs:set Generic.ControlStructures.InlineControlStructure error false
+if ($something) echo 'hello';
+// phpcs:set Generic.ControlStructures.InlineControlStructure error true
+
+if ($noSpaceAfterClosingParenthesis)echo 'hello';
+
+do ; while ($i > 5);

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.inc.fixed

@@ -307,3 +307,36 @@ function testFinally()
         }
     }
 }
+
+if ($something) {
+    echo 'hello';
+} else /* comment */ if ($somethingElse) { echo 'hi';
+}
+
+if ($shouldNotTriggerSniff);
+
+if (false) {
+} elseif ($shouldNotTriggerSniff);
+
+if (false) {
+} else if ($shouldNotTriggerSniff);
+
+if (false) {
+} else { ($shouldTriggerSniff);
+}
+
+foreach ($array as $shouldNotTriggerSniff);
+
+do { echo $i++;
+} while ($i < 5);
+
+// phpcs:set Generic.ControlStructures.InlineControlStructure error false
+if ($something) { echo 'hello';
+}
+// phpcs:set Generic.ControlStructures.InlineControlStructure error true
+
+if ($noSpaceAfterClosingParenthesis) { echo 'hello';
+}
+
+do { ;
+} while ($i > 5);

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.js → src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.js

@@ -29,3 +29,7 @@ if ($("#myid").rotationDegrees()=='90')
 
 if ($("#myid").rotationDegrees()=='90')
     $foo = {'transform': 'rotate(90deg)'};
+
+if (something) {
+    alert('hello');
+} else /* comment */ if (somethingElse) alert('hi');

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.js.fixed → src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.1.js.fixed

@@ -37,3 +37,8 @@ if ($("#myid").rotationDegrees()=='90') {
 if ($("#myid").rotationDegrees()=='90') {
     $foo = {'transform': 'rotate(90deg)'};
 }
+
+if (something) {
+    alert('hello');
+} else /* comment */ if (somethingElse) { alert('hi');
+}

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.10.inc

@@ -0,0 +1,9 @@
+<?php
+
+// This should be the only test in this file.
+// There should be only empty tokens after the scope closer of the "if" token.
+
+for ($i = 0; $i < 5; $i++)
+    if ($i === 3) {
+        echo $i;
+    }

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.10.inc.fixed

@@ -0,0 +1,10 @@
+<?php
+
+// This should be the only test in this file.
+// There should be only empty tokens after the scope closer of the "if" token.
+
+for ($i = 0; $i < 5; $i++) {
+    if ($i === 3) {
+        echo $i;
+    }
+}

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.2.js

@@ -0,0 +1,5 @@
+// Intentional parse error (missing closing parenthesis).
+// This should be the only test in this file.
+// Testing that the sniff is *not* triggered.
+
+do i++; while (i < 5

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.3.js

@@ -0,0 +1,5 @@
+// Intentional parse error (missing opening parenthesis).
+// This should be the only test in this file.
+// Testing that the sniff is *not* triggered.
+
+do i++; while

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.8.inc

@@ -0,0 +1,7 @@
+<?php
+
+// Intentional parse error (nothing after for closing parenthesis).
+// This should be the only test in this file.
+// Testing that the sniff is *not* triggered.
+
+for ($i = 0; $i < 5; $i++)

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.9.inc

@@ -0,0 +1,7 @@
+<?php
+
+// Intentional parse error (nothing after while closing parenthesis).
+// This should be the only test in this file.
+// Testing that the sniff is *not* triggered.
+
+while ($i < 5)

src/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php

@@ -78,9 +78,19 @@ public function getErrorList($testFile='')
                 242 => 1,
                 260 => 1,
                 269 => 1,
+                278 => 1,
+                289 => 1,
+                293 => 1,
+                299 => 1,
+                301 => 1,
             ];
 
-        case 'InlineControlStructureUnitTest.js':
+        case 'InlineControlStructureUnitTest.10.inc':
+            return [
+                6 => 1,
+            ];
+
+        case 'InlineControlStructureUnitTest.1.js':
             return [
                 3  => 1,
                 7  => 1,
@@ -90,6 +100,7 @@ public function getErrorList($testFile='')
                 21 => 1,
                 27 => 1,
                 30 => 1,
+                35 => 1,
             ];
 
         default:
@@ -105,11 +116,21 @@ public function getErrorList($testFile='')
      * The key of the array should represent the line number and the value
      * should represent the number of warnings that should occur on that line.
      *
+     * @param string $testFile The name of the file being tested.
+     *
      * @return array<int, int>
      */
-    public function getWarningList()
+    public function getWarningList($testFile='')
     {
-        return [];
+        switch ($testFile) {
+        case 'InlineControlStructureUnitTest.1.inc':
+            return [
+                296 => 1,
+            ];
+
+        default:
+            return [];
+        }
 
     }//end getWarningList()