Fixed bug squizlabs#1120 : InlineControlStructureSniff does not handl…

…e auto-fixing for control structures that make function calls Also improved the formatting of the end brace when auto fixing InlineControlStructure errors (request squizlabs#1121) as it made the original bug fix a little easier.
kudeepak · Aug 17, 2016 · a5d3f14 · a5d3f14
1 parent 71945d9
commit a5d3f14
Show file tree

Hide file tree

Showing 6 changed files with 153 additions and 34 deletions.
diff --git a/CodeSniffer/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php b/CodeSniffer/Standards/Generic/Sniffs/ControlStructures/InlineControlStructureSniff.php
@@ -202,14 +202,28 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
                     }
                 }//end if
 
-                break;
+                if ($tokens[$end]['code'] !== T_END_HEREDOC
+                    && $tokens[$end]['code'] !== T_END_NOWDOC
+                ) {
+                    break;
+                }
             }//end if
 
+            if (isset($tokens[$end]['parenthesis_closer']) === true) {
+                $end          = $tokens[$end]['parenthesis_closer'];
+                $lastNonEmpty = $end;
+                continue;
+            }
+
             if ($tokens[$end]['code'] !== T_WHITESPACE) {
                 $lastNonEmpty = $end;
             }
         }//end for
 
+        if ($end === $phpcsFile->numTokens) {
+            $end = $lastNonEmpty;
+        }
+
         $next = $phpcsFile->findNext(T_WHITESPACE, ($closer + 1), ($end + 1), true);
 
         // Account for a comment on the end of the line.
@@ -227,16 +241,50 @@ public function process(PHP_CodeSniffer_File $phpcsFile, $stackPtr)
 
         if ($next !== $end) {
             if ($endLine !== $end) {
-                $phpcsFile->fixer->addContent($endLine, '}');
+                $endToken     = $endLine;
+                $addedContent = '';
             } else {
+                $endToken     = $end;
+                $addedContent = $phpcsFile->eolChar;
+
                 if ($tokens[$end]['code'] !== T_SEMICOLON
                     && $tokens[$end]['code'] !== T_CLOSE_CURLY_BRACKET
                 ) {
-                    $phpcsFile->fixer->addContent($end, ';');
+                    $phpcsFile->fixer->addContent($end, '; ');
                 }
-
-                $phpcsFile->fixer->addContent($end, ' }');
             }
+
+            $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 {
+                $indent = '';
+                for ($first = $stackPtr; $first > 0; $first--) {
+                    if ($first === 1
+                        || $tokens[($first - 1)]['line'] !== $tokens[$first]['line']
+                    ) {
+                        break;
+                    }
+                }
+
+                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 = '';
+                }
+
+                $addedContent .= $indent.'}';
+                if ($next !== false && $tokens[$endToken]['code'] === T_COMMENT) {
+                    $addedContent .= $phpcsFile->eolChar;
+                }
+
+                $phpcsFile->fixer->addContent($endToken, $addedContent);
+            }//end if
         } else {
             if ($endLine !== $end) {
                 $phpcsFile->fixer->replaceToken($end, '');

diff --git a/CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc b/CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc
@@ -163,3 +163,21 @@ foreach ($sql as $s)
                 if ($this->neverAbort) $ret = false;
                 else return false;
         }
+
+if ($bar)
+    if ($foo) echo 'hi'; // lol
+
+if ($level == 'district')
+    \DB::update(<<<EOD
+some
+text
+here
+EOD
+    );
+
+if ($level == 'district')
+    $var = <<<EOD
+some
+text
+here
+EOD;
diff --git a/...niffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc.fixed b/...niffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.inc.fixed
@@ -1,27 +1,34 @@
 <?php
 
-if ($something) { echo 'hello'; }
+if ($something) { echo 'hello';
+}
 
 if ($something) {
     echo 'hello';
-} else { echo 'hi'; }
+} else { echo 'hi';
+}
 
 if ($something) {
     echo 'hello';
-} else if ($else) { echo 'hi'; }
+} else if ($else) { echo 'hi';
+}
 
-foreach ($something as $thing) { echo 'hello'; }
+foreach ($something as $thing) { echo 'hello';
+}
 
-for ($i; $i > 0; $i--) { echo 'hello'; }
+for ($i; $i > 0; $i--) { echo 'hello';
+}
 
-while ($something) { echo 'hello'; }
+while ($something) { echo 'hello';
+}
 
 do {
     $i--;
 } while ($something);
 
 if(true) {
-  $someObject->{$name}; }
+  $someObject->{$name};
+}
 
 if (true) :
     $foo = true;
@@ -45,11 +52,14 @@ while (!$this->readLine($tokens, $tag)) {
 }
 foreach ($cookies as $cookie) {
     if ($cookie->match($uri, $matchSessionCookies, $now)) {
-        $ret[] = $cookie; } }
+        $ret[] = $cookie;
+    }
+}
 
 foreach ($stringParade as $hit) {
     $hitParade[] = $hit + 0; //cast to integer
 }
+
 if ($foo) :
     echo 'true';
 elseif ($something) :
@@ -61,7 +71,8 @@ endif;
 function test()
 {
     if ($a) {
-        $a.=' '.($b ? 'b' : ($c ? ($d ? 'd' : 'c') : '')); }
+        $a.=' '.($b ? 'b' : ($c ? ($d ? 'd' : 'c') : ''));
+    }
 }
 
 if ($a) {
@@ -72,7 +83,8 @@ if ($a) {
         } elseif ($i==0) {
             $j=$k;
         }
-    } }
+    }
+}
 
 ?>
 <div style="text-align: right;">
@@ -123,44 +135,73 @@ foreach($stuff as $num) {
         echo "even";
     } else {
         echo "odd";
-    } }
+    }
+}
 
 $i = 0;
 foreach($stuff as $num) {
     do {
         echo $i;
         $i++;
-    } while ($i < 5); }
+    } while ($i < 5);
+}
 
 foreach($stuff as $num) {
     if (true) {
         echo "true1\n";
-    } }
+    }
+}
     if (true) {
         echo "true2\n";
     }
 
-if ($foo) { echo 'foo'; }
-elseif ($bar) { echo 'bar'; }
-else { echo 'baz'; }
+if ($foo) { echo 'foo';
+} elseif ($bar) { echo 'bar';
+} else { echo 'baz';
+}
 
 switch ($type) {
     case 1:
         if ($foo) {
             return true;
         } elseif ($baz) {
-            return true; }
-        else {
+            return true;
+        } else {
             echo 'else';
         }
     break;
 }
 
 foreach ($sql as $s) {
-        if (!$this->execute) { echo "<pre>",$s.";\n</pre>"; }
-        else {
+        if (!$this->execute) { echo "<pre>",$s.";\n</pre>";
+        } else {
             $ok = $this->connDest->Execute($s);
             if (!$ok) {
-                if ($this->neverAbort) { $ret = false; }
-                else { return false; } }
-        } }
+                if ($this->neverAbort) { $ret = false;
+                } else { return false;
+                }
+            }
+        }
+}
+
+if ($bar) {
+    if ($foo) { echo 'hi'; // lol
+    }
+}
+
+if ($level == 'district') {
+    \DB::update(<<<EOD
+some
+text
+here
+EOD
+    );
+}
+
+if ($level == 'district') {
+    $var = <<<EOD
+some
+text
+here
+EOD;
+}
diff --git a/...Sniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.js.fixed b/...Sniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.js.fixed
@@ -1,24 +1,30 @@
 
 
-if (something) { print 'hello'; }
+if (something) { print 'hello';
+}
 
 if (something) {
     print 'hello';
-} else { print 'hi'; }
+} else { print 'hi';
+}
 
 if (something) {
     print 'hello';
-} else if (something) { print 'hi'; }
+} else if (something) { print 'hi';
+}
 
-for (i; i > 0; i--) { print 'hello'; }
+for (i; i > 0; i--) { print 'hello';
+}
 
-while (something) { print 'hello'; }
+while (something) { print 'hello';
+}
 
 do {
     i--;
 } while (something);
 
-do { i++; } while (i < 5);
+do { i++;
+} while (i < 5);
 
 SomeClass.prototype.switch = function() {
     // do something

diff --git a/CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php b/CodeSniffer/Standards/Generic/Tests/ControlStructures/InlineControlStructureUnitTest.php
@@ -74,6 +74,10 @@ public function getErrorList($testFile='InlineControlStructureUnitTest.inc')
                     162 => 1,
                     163 => 1,
                     164 => 1,
+                    167 => 1,
+                    168 => 1,
+                    170 => 1,
+                    178 => 1,
                    );
             break;
         case 'InlineControlStructureUnitTest.js':

diff --git a/package.xml b/package.xml
@@ -69,6 +69,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
     -- Thanks to Walt Sorensen for the patch
   - PHPCBF is now able to fix Generic.PHP.DisallowShortOpenTag
     -- Thanks to Juliette Reinders Folmer for the patch
+  - Improved the formatting of the end brace when auto fixing InlineControlStructure errors (request #1121)
   - Generic.Functions.OpeningFunctionBraceKernighanRitchie.BraceOnNewLine fix no longer leaves blank line after brace (request #1085)
   - Generic UpperCaseConstantNameSniff now allows lowercase namespaces in constant definitions
     -- Thanks to Daniel Schniepp for the patch
@@ -92,6 +93,7 @@ http://pear.php.net/dtd/package-2.0.xsd">
   - Fixed bug #1102 : Squiz.Formatting.OperatorBracket.MissingBrackets faulty bracketing fix
   - Fixed bug #1109 : Wrong scope indent reported in anonymous class
   - Fixed bug #1112 : File docblock not recognized when require_once follows it
+  - Fixed bug #1120 : InlineControlStructureSniff does not handle auto-fixing for control structures that make function calls
  </notes>
  <contents>
   <dir name="/">
-Original file line number
+Diff line change
@@ Expand Up @@
 => 1,
 => 1,
 => 1,
+=> 1,
+=> 1,
+=> 1,
+=> 1,
                        );
                 break;
             case 'InlineControlStructureUnitTest.js':
@@ Expand Down @@