Improve the chained method block fixer

contao · Feb 9, 2024 · b25a74d · b25a74d
1 parent 2f09403
commit b25a74d
Show file tree

Hide file tree

Showing 2 changed files with 177 additions and 15 deletions.
diff --git a/src/Fixer/ChainedMethodBlockFixer.php b/src/Fixer/ChainedMethodBlockFixer.php
@@ -102,32 +102,104 @@ protected function applyFix(\SplFileInfo $file, Tokens $tokens): void
                 continue;
             }
 
-            $chainedCalls = 0;
-            $operators = $tokens->findGivenKind(T_OBJECT_OPERATOR, $start, $end);
-
-            foreach (array_keys($operators) as $pos) {
-                if ($tokens[$pos - 1]->isWhitespace()) {
-                    ++$chainedCalls;
-                }
-            }
+            $chainedCalls = $this->getChainedCalls($tokens, $start, $end);
 
             if ($chainedCalls < 1) {
                 $index = $end;
                 continue;
             }
 
-            $nextMeaningful = $tokens->getNextMeaningfulToken($end);
+            $this->fixLeadingWhitespace($tokens, $start);
 
-            if (null === $nextMeaningful || $tokens[$nextMeaningful]->equals('}')) {
-                $index = $end;
-                continue;
+            $index = $end;
+        }
+    }
+
+    private function getChainedCalls(Tokens $tokens, int $start, int $end): int
+    {
+        $chainedCalls = 0;
+        $operators = $tokens->findGivenKind(T_OBJECT_OPERATOR, $start, $end);
+
+        foreach (array_keys($operators) as $pos) {
+            if ($tokens[$pos - 1]->isWhitespace()) {
+                ++$chainedCalls;
+            }
+        }
+
+        return $chainedCalls;
+    }
+
+    private function fixLeadingWhitespace(Tokens $tokens, int $start): void
+    {
+        $prevStart = $tokens->getPrevTokenOfKind($start, [';', '{']);
+
+        if (null === $prevStart) {
+            return;
+        }
+
+        $prevIndex = $prevStart;
+        $prevVar = $this->getBlockVariable($tokens, $prevStart, $prevIndex);
+
+        if (null === $prevVar) {
+            return;
+        }
+
+        $addNewLine = false;
+        $removeNewLine = false;
+
+        if ($this->isMultiline($tokens, $prevIndex, $start)) {
+            $addNewLine = true;
+        } else {
+            $var = $this->getBlockVariable($tokens, $start);
+            $prevVar = $this->getBlockVariable($tokens, $prevStart);
+
+            if ($var === $prevVar) {
+                $removeNewLine = true;
+            } elseif (str_starts_with($prevVar, "$var->")) {
+                $addNewLine = true;
             }
+        }
+
+        $content = $tokens[$start + 1]->getContent();
 
-            if (substr_count($tokens[$end + 1]->getContent(), "\n") < 2) {
-                $tokens->insertAt($end + 1, new Token([T_WHITESPACE, "\n"]));
+        if ($addNewLine && !$tokens[$start + 2]->isGivenKind(T_WHITESPACE)) {
+            if (substr_count($content, "\n") < 2) {
+                $tokens->offsetSet($start + 1, new Token([T_WHITESPACE, str_replace("\n", "\n\n", $content)]));
             }
+        } elseif ($removeNewLine && substr_count($content, "\n") > 1) {
+            $tokens->offsetSet($start + 1, new Token([T_WHITESPACE, str_replace("\n\n", "\n", $content)]));
+        }
+    }
 
-            $index = $end;
+    private function isMultiline(Tokens $tokens, int $start, int $end): bool
+    {
+        $lineBreaks = 0;
+        $operators = $tokens->findGivenKind(T_WHITESPACE, $start, $end);
+
+        foreach (array_keys($operators) as $pos) {
+            if ($tokens[$pos]->isWhitespace() && str_contains($tokens[$pos]->getContent(), "\n")) {
+                ++$lineBreaks;
+            }
         }
+
+        return $lineBreaks > 1;
+    }
+
+    private function getBlockVariable(Tokens $tokens, int $start, int &$prevIndex = 0): string|null
+    {
+        $index = $tokens->getNextMeaningfulToken($start);
+
+        if (!$tokens[$index]->isGivenKind(T_VARIABLE)) {
+            return null;
+        }
+
+        $prevIndex = $index;
+        $var = $tokens[$index]->getContent();
+
+        if ($tokens[$index + 1]->isObjectOperator()) {
+            $var = $tokens->generatePartialCode($index, $index + 2);
+        }
+
+        return $var;
     }
 }
diff --git a/tests/Fixer/ChainedMethodBlockFixerTest.php b/tests/Fixer/ChainedMethodBlockFixerTest.php
@@ -79,6 +79,50 @@ function () {
                             )
                         ;
                     }
+
+                    public function testBar(): void
+                    {
+                        /*
+                         * Comment
+                         */
+
+                        $this->mock = $this->createMock(Bar::class);
+
+                        $this->mock
+                            ->method("isFoo")
+                            ->willReturn(false)
+                        ;
+
+                        $this->mock
+                            ->method("isBar")
+                            ->willReturn(true)
+                        ;
+                    }
+
+                    public function testBaz(): void
+                    {
+                        $mock = $this->mockClassWithProperties(Baz::class);
+                        $mock->id = 42;
+                        $mock
+                            ->method("isFoo")
+                            ->willReturn(false)
+                        ;
+                        $mock
+                            ->method("isBaz")
+                            ->willReturn(true)
+                        ;
+                    }
+
+                    public function testBat(): void
+                    {
+                        $mock = $this->mockClassWithProperties(Bat::class, [
+                            'id' => 42,
+                        ]);
+                        $mock
+                            ->method("isBat")
+                            ->willReturn(true)
+                        ;
+                    }
                 }
                 EOT,
             <<<'EOT'
@@ -128,6 +172,52 @@ function () {
                             )
                         ;
                     }
+
+                    public function testBar(): void
+                    {
+                        /*
+                         * Comment
+                         */
+
+                        $this->mock = $this->createMock(Bar::class);
+                        $this->mock
+                            ->method("isFoo")
+                            ->willReturn(false)
+                        ;
+
+                        $this->mock
+                            ->method("isBar")
+                            ->willReturn(true)
+                        ;
+                    }
+
+                    public function testBaz(): void
+                    {
+                        $mock = $this->mockClassWithProperties(Baz::class);
+                        $mock->id = 42;
+
+                        $mock
+                            ->method("isFoo")
+                            ->willReturn(false)
+                        ;
+
+                        $mock
+                            ->method("isBaz")
+                            ->willReturn(true)
+                        ;
+                    }
+
+                    public function testBat(): void
+                    {
+                        $mock = $this->mockClassWithProperties(Bat::class, [
+                            'id' => 42,
+                        ]);
+
+                        $mock
+                            ->method("isBat")
+                            ->willReturn(true)
+                        ;
+                    }
                 }
                 EOT,
         ];