fix: LineBreakBetweenMethodArgumentsFixer (#135)

PedroTroller · Apr 15, 2021 · fd2e3d6 · fd2e3d6
1 parent 00fa06b
commit fd2e3d6
Show file tree

Hide file tree

Showing 7 changed files with 119 additions and 45 deletions.
diff --git a/composer.json b/composer.json
@@ -4,7 +4,7 @@
     "license": "MIT",
     "require": {
         "php": ">=7.2",
-        "friendsofphp/php-cs-fixer": "^2.16.2"
+        "friendsofphp/php-cs-fixer": "^2.18"
     },
     "require-dev": {
         "phpspec/phpspec": "^6.1",

diff --git a/spec/PedroTroller/CS/Fixer/RuleSetFactorySpec.php b/spec/PedroTroller/CS/Fixer/RuleSetFactorySpec.php
@@ -77,26 +77,30 @@ function it_adds_a_phpCsFixer_strict_set()
     function it_adds_a_php_version_support()
     {
         $this->php(5.6)->getRules()->shouldReturn([
+            '@PHP54Migration' => true,
             '@PHP56Migration' => true,
             'array_syntax'    => ['syntax' => 'short'],
             'list_syntax'     => ['syntax' => 'long'],
         ]);
 
         $this->php(5.6, true)->getRules()->shouldReturn([
+            '@PHP54Migration'       => true,
             '@PHP56Migration'       => true,
             '@PHP56Migration:risky' => true,
             'array_syntax'          => ['syntax' => 'short'],
             'list_syntax'           => ['syntax' => 'long'],
         ]);
 
         $this->php(7.0)->getRules()->shouldReturn([
+            '@PHP54Migration' => true,
             '@PHP56Migration' => true,
             '@PHP70Migration' => true,
             'array_syntax'    => ['syntax' => 'short'],
             'list_syntax'     => ['syntax' => 'long'],
         ]);
 
         $this->php(7.0, true)->getRules()->shouldReturn([
+            '@PHP54Migration'       => true,
             '@PHP56Migration'       => true,
             '@PHP56Migration:risky' => true,
             '@PHP70Migration'       => true,
@@ -106,6 +110,7 @@ function it_adds_a_php_version_support()
         ]);
 
         $this->php(7.1)->getRules()->shouldReturn([
+            '@PHP54Migration' => true,
             '@PHP56Migration' => true,
             '@PHP70Migration' => true,
             '@PHP71Migration' => true,
@@ -114,6 +119,7 @@ function it_adds_a_php_version_support()
         ]);
 
         $this->php(7.1, true)->getRules()->shouldReturn([
+            '@PHP54Migration'       => true,
             '@PHP56Migration'       => true,
             '@PHP56Migration:risky' => true,
             '@PHP70Migration'       => true,
@@ -125,6 +131,7 @@ function it_adds_a_php_version_support()
         ]);
 
         $this->php(7.2)->getRules()->shouldReturn([
+            '@PHP54Migration' => true,
             '@PHP56Migration' => true,
             '@PHP70Migration' => true,
             '@PHP71Migration' => true,
@@ -133,6 +140,7 @@ function it_adds_a_php_version_support()
         ]);
 
         $this->php(7.2, true)->getRules()->shouldReturn([
+            '@PHP54Migration'       => true,
             '@PHP56Migration'       => true,
             '@PHP56Migration:risky' => true,
             '@PHP70Migration'       => true,
@@ -147,6 +155,7 @@ function it_adds_a_php_version_support()
     function it_can_also_parse_versions_as_string()
     {
         $this->php('5.6.2')->getRules()->shouldReturn([
+            '@PHP54Migration' => true,
             '@PHP56Migration' => true,
             'array_syntax'    => ['syntax' => 'short'],
             'list_syntax'     => ['syntax' => 'long'],

diff --git a/src/PedroTroller/CS/Fixer/AbstractOrderedClassElementsFixer.php b/src/PedroTroller/CS/Fixer/AbstractOrderedClassElementsFixer.php
@@ -95,6 +95,7 @@ private function getElements(Tokens $tokens, $startIndex)
                         $element['methodName'] = $tokens[$tokens->getNextMeaningfulToken($i)]->getContent();
 
                         break;
+
                     case 'property':
                         $element['propertyName'] = $token->getContent();
 

diff --git a/src/PedroTroller/CS/Fixer/CodingStyle/LineBreakBetweenMethodArgumentsFixer.php b/src/PedroTroller/CS/Fixer/CodingStyle/LineBreakBetweenMethodArgumentsFixer.php
@@ -167,43 +167,29 @@ protected function createConfigurationDefinition()
 
     private function splitArgs(Tokens $tokens, $index): void
     {
+        $this->mergeArgs($tokens, $index);
+
         $openBraceIndex  = $tokens->getNextTokenOfKind($index, ['(']);
         $closeBraceIndex = $this->analyze($tokens)->getClosingParenthesis($openBraceIndex);
 
         if (0 === $closeBraceIndex) {
             return;
         }
 
-        $token                   = $tokens[$openBraceIndex];
-        $tokens[$openBraceIndex] = new Token([
-            T_WHITESPACE,
-            trim($token->getContent())."\n".$this->analyze($tokens)->getLineIndentation($index).$this->whitespacesConfig->getIndent(),
-        ]);
-
-        $token                    = $tokens[$closeBraceIndex];
-        $tokens[$closeBraceIndex] = new Token([
-            T_WHITESPACE,
-            rtrim($this->whitespacesConfig->getLineEnding().$this->analyze($tokens)->getLineIndentation($index).$token->getContent()),
-        ]);
-
         if ($tokens[$tokens->getNextMeaningfulToken($closeBraceIndex)]->equals('{')) {
             $tokens->removeTrailingWhitespace($closeBraceIndex);
-            $token                                                     = $tokens[$tokens->getNextMeaningfulToken($closeBraceIndex)];
-            $tokens[$tokens->getNextMeaningfulToken($closeBraceIndex)] = new Token([
-                T_WHITESPACE,
-                ' '.trim($token->getContent()),
-            ]);
+            $tokens->ensureWhitespaceAtIndex($closeBraceIndex, 1, ' ');
         }
 
         if ($tokens[$tokens->getNextMeaningfulToken($closeBraceIndex)]->isGivenKind(self::T_TYPEHINT_SEMI_COLON)) {
             $end = $tokens->getNextTokenOfKind($closeBraceIndex, [';', '{']);
 
-            for ($i = $closeBraceIndex + 1; $i < $end; ++$i) {
-                $content    = preg_replace('/ {2,}/', ' ', str_replace("\n", ' ', $tokens[$i]->getContent()));
-                $tokens[$i] = new Token([$tokens[$i]->getId(), $content]);
-            }
+            $tokens->removeLeadingWhitespace($end);
+            $tokens->ensureWhitespaceAtIndex($end, 0, ' ');
         }
 
+        $linebreaks = [$openBraceIndex, $closeBraceIndex - 1];
+
         for ($i = $openBraceIndex + 1; $i < $closeBraceIndex; ++$i) {
             if ($tokens[$i]->equals('(')) {
                 $i = $this->analyze($tokens)->getClosingParenthesis($i);
@@ -214,29 +200,38 @@ private function splitArgs(Tokens $tokens, $index): void
             }
 
             if ($tokens[$i]->equals(',')) {
-                $tokens->removeTrailingWhitespace($i);
-                $token      = $tokens[$i];
-                $tokens[$i] = new Token([
-                    T_WHITESPACE,
-                    trim($token->getContent())."\n".$this->analyze($tokens)->getLineIndentation($index).$this->whitespacesConfig->getIndent(),
-                ]);
+                $linebreaks[] = $i;
             }
         }
 
-        $tokens->removeTrailingWhitespace($openBraceIndex);
-        $tokens->removeTrailingWhitespace($tokens->getPrevMeaningfulToken($closeBraceIndex));
+        sort($linebreaks);
+
+        foreach (array_reverse($linebreaks, false) as $iteration => $linebreak) {
+            $tokens->removeTrailingWhitespace($linebreak);
+
+            switch ($iteration) {
+                case 0:
+                    $whitespace = "\n".$this->analyze($tokens)->getLineIndentation($index);
+
+                    break;
+
+                default:
+                    $whitespace = "\n".$this->analyze($tokens)->getLineIndentation($index).'    ';
+
+                    break;
+            }
+
+            $tokens->ensureWhitespaceAtIndex($linebreak, 1, $whitespace);
+        }
     }
 
     private function mergeArgs(Tokens $tokens, $index): void
     {
         $openBraceIndex  = $tokens->getNextTokenOfKind($index, ['(']);
         $closeBraceIndex = $this->analyze($tokens)->getClosingParenthesis($openBraceIndex);
 
-        for ($i = $openBraceIndex; $i <= $closeBraceIndex; ++$i) {
-            $content    = preg_replace('/ {2,}/', ' ', str_replace("\n", ' ', $tokens[$i]->getContent()));
-            $tokens[$i] = $tokens[$i]->getId()
-                ? new Token([$tokens[$i]->getId(), $content])
-                : new Token([T_WHITESPACE, $content]);
+        foreach ($tokens->findGivenKind(T_WHITESPACE, $openBraceIndex, $closeBraceIndex) as $spaceIndex => $spaceToken) {
+            $tokens[$spaceIndex] = new Token([T_WHITESPACE, ' ']);
         }
 
         $tokens->removeTrailingWhitespace($openBraceIndex);
@@ -246,7 +241,7 @@ private function mergeArgs(Tokens $tokens, $index): void
 
         if ($tokens[$end]->equals('{')) {
             $tokens->removeLeadingWhitespace($end);
-            $tokens->insertAt($end, new Token([T_WHITESPACE, "\n".$this->analyze($tokens)->getLineIndentation($index)]));
+            $tokens->ensureWhitespaceAtIndex($end, -1, "\n".$this->analyze($tokens)->getLineIndentation($index));
         }
     }
 

diff --git a/src/PedroTroller/CS/Fixer/PhpspecFixer.php b/src/PedroTroller/CS/Fixer/PhpspecFixer.php
@@ -82,14 +82,6 @@ public function its_other_function($file) {
 SPEC;
     }
 
-    /**
-     * {@inheritdoc}
-     */
-    public function getPriority()
-    {
-        return Priority::after(VisibilityRequiredFixer::class);
-    }
-
     public function getDocumentation()
     {
         return implode(
@@ -102,6 +94,14 @@ public function getDocumentation()
         );
     }
 
+    /**
+     * {@inheritdoc}
+     */
+    public function getPriority()
+    {
+        return Priority::after(VisibilityRequiredFixer::class);
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -223,7 +223,7 @@ private function removeReturn(SplFileInfo $file, Tokens $tokens): void
             }
 
             $tokens->clearRange($closeBraceIndex + 1, $openCurlyBracket - 1);
-            $tokens->insertAt($openCurlyBracket, new Token(' '));
+            $tokens->ensureWhitespaceAtIndex($openCurlyBracket, 0, "\n".$this->analyze($tokens)->getLineIndentation($openBraceIndex));
         }
     }
 

diff --git a/src/PedroTroller/CS/Fixer/TokensAnalyzer.php b/src/PedroTroller/CS/Fixer/TokensAnalyzer.php
@@ -124,14 +124,17 @@ public function getNextComma($index)
                     $index = $this->getClosingParenthesis($index);
 
                     break;
+
                 case $this->tokens[$index]->equals('['):
                     $index = $this->getClosingBracket($index);
 
                     break;
+
                 case $this->tokens[$index]->equals('{'):
                     $index = $this->getClosingCurlyBracket($index);
 
                     break;
+
                 case $this->tokens[$index]->equals(';'):
                     return;
             }
@@ -159,10 +162,12 @@ public function getNextSemiColon($index)
                     $index = $this->getClosingParenthesis($index);
 
                     break;
+
                 case $this->tokens[$index]->equals('['):
                     $index = $this->getClosingBracket($index);
 
                     break;
+
                 case $this->tokens[$index]->equals('{'):
                     $index = $this->getClosingCurlyBracket($index);
 
@@ -294,10 +299,12 @@ public function endOfTheStatement($index)
                     $index = $this->getClosingParenthesis($index);
 
                     break;
+
                 case $this->tokens[$index]->equals('['):
                     $index = $this->getClosingBracket($index);
 
                     break;
+
                 case $this->tokens[$index]->equals('{'):
                     $index = $this->getClosingCurlyBracket($index);
 
@@ -528,6 +535,7 @@ public function getElements($startIndex = null)
                         $element['methodName'] = $this->tokens[$this->tokens->getNextMeaningfulToken($i)]->getContent();
 
                         break;
+
                     case 'property':
                         $element['propertyName'] = $token->getContent();
 

diff --git a/tests/UseCase/LineBreakBetweenMethods/Regression/Case5.php b/tests/UseCase/LineBreakBetweenMethods/Regression/Case5.php
@@ -0,0 +1,61 @@
+<?php
+
+declare(strict_types=1);
+
+namespace tests\UseCase\LineBreakBetweenMethods\Regression;
+
+use PedroTroller\CS\Fixer\CodingStyle\LineBreakBetweenMethodArgumentsFixer;
+use tests\UseCase;
+
+/**
+ * https://github.com/PedroTroller/PhpCSFixer-Custom-Fixers/issues/131.
+ */
+final class Case5 implements UseCase
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function getFixer()
+    {
+        $fixer = new LineBreakBetweenMethodArgumentsFixer();
+
+        $fixer->configure([
+            'max-args'   => 4,
+            'max-length' => 100,
+        ]);
+
+        return $fixer;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getRawScript()
+    {
+        return <<<'PHP'
+<?php
+
+return [\dirname(__DIR__) . '/definitions'];
+PHP;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getExpectation()
+    {
+        return <<<'PHP'
+<?php
+
+return [\dirname(__DIR__) . '/definitions'];
+PHP;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function getMinSupportedPhpVersion()
+    {
+        return 0;
+    }
+}
-Original file line number
+Diff line change
@@ Expand Up / @@ -95,6 +95,7 @@ private function getElements(Tokens $tokens, $startIndex) @@
                             $element['methodName'] = $tokens[$tokens->getNextMeaningfulToken($i)]->getContent();
                             break;
                         case 'property':
                             $element['propertyName'] = $token->getContent();
@@ Expand Down @@