Fix Issue #78 - Error in Redirect::route() Method (#106)

* Bug fix on redirect route to to route transformation * Redirect back to back helper transformation fix 1) Argument missing fix 2) Last child can not be deleted. * rename classes * Code fixes
driftingly · May 17, 2023 · 02c1eb7 · 02c1eb7
1 parent 6043322
commit 02c1eb7
Show file tree

Hide file tree

Showing 10 changed files with 173 additions and 34 deletions.
diff --git a/src/Rector/ClassMethod/AddArgumentDefaultValueRector.php b/src/Rector/ClassMethod/AddArgumentDefaultValueRector.php
@@ -6,6 +6,7 @@
 
 use PhpParser\BuilderHelpers;
 use PhpParser\Node;
+use PhpParser\Node\Expr;
 use PhpParser\Node\Param;
 use PhpParser\Node\Stmt\ClassMethod;
 use Rector\Core\Contract\Rector\ConfigurableRectorInterface;
@@ -93,7 +94,7 @@ public function refactor(Node $node): ClassMethod
             $position = $addedArgument->getPosition();
             $param = $node->params[$position];
 
-            if ($param->default !== null) {
+            if ($param->default instanceof Expr) {
                 continue;
             }
 

diff --git a/src/Rector/Empty_/EmptyToBlankAndFilledFuncRector.php b/src/Rector/Empty_/EmptyToBlankAndFilledFuncRector.php
@@ -1,5 +1,7 @@
 <?php
 
+declare(strict_types=1);
+
 namespace RectorLaravel\Rector\Empty_;
 
 use PhpParser\Node;
@@ -14,22 +16,25 @@
  */
 class EmptyToBlankAndFilledFuncRector extends AbstractRector
 {
-
     public function getRuleDefinition(): RuleDefinition
     {
-        return new RuleDefinition('Replace use of the unsafe empty() function with Laravel\'s safer blank() & filled() functions.', [
-            new CodeSample(
-                <<<'CODE_SAMPLE'
+        return new RuleDefinition(
+            'Replace use of the unsafe empty() function with Laravel\'s safer blank() & filled() functions.',
+            [
+                new CodeSample(
+                    <<<'CODE_SAMPLE'
 empty([]);
 !empty([]);
 CODE_SAMPLE
-                ,
-                <<<'CODE_SAMPLE'
+                    ,
+                    <<<'CODE_SAMPLE'
 blank([]);
 filled([]);
 CODE_SAMPLE
-            ),
-        ]);
+                ),
+
+            ]
+        );
     }
 
     public function getNodeTypes(): array
@@ -45,7 +50,7 @@ public function refactor(Node $node): ?Node
             }
             $method = 'filled';
             $args = [$node->expr->expr];
-        } else if ($node instanceof Empty_) {
+        } elseif ($node instanceof Empty_) {
             $method = 'blank';
             $args = [$node->expr];
         } else {

diff --git a/src/Rector/MethodCall/AssertStatusToAssertMethodRector.php b/src/Rector/MethodCall/AssertStatusToAssertMethodRector.php
@@ -5,7 +5,10 @@
 namespace RectorLaravel\Rector\MethodCall;
 
 use PhpParser\Node;
+use PhpParser\Node\Expr\ClassConstFetch;
 use PhpParser\Node\Expr\MethodCall;
+use PhpParser\Node\Identifier;
+use PhpParser\Node\Scalar\LNumber;
 use PHPStan\Type\ObjectType;
 use Rector\Core\Rector\AbstractRector;
 use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
@@ -189,18 +192,18 @@ private function updateAssertStatusCall(MethodCall $methodCall): ?MethodCall
             return null;
         }
 
-        if (count($methodCall->getArgs()) <> 1) {
+        if (count($methodCall->getArgs()) !== 1) {
             return null;
         }
 
         $arg = $methodCall->getArgs()[0];
         $argValue = $arg->value;
 
-        if (! $argValue instanceof Node\Scalar\LNumber && ! $argValue instanceof Node\Expr\ClassConstFetch) {
+        if (! $argValue instanceof LNumber && ! $argValue instanceof ClassConstFetch) {
             return null;
         }
 
-        if ($argValue instanceof Node\Scalar\LNumber) {
+        if ($argValue instanceof LNumber) {
             $replacementMethod = match ($argValue->value) {
                 200 => 'assertOk',
                 204 => 'assertNoContent',
@@ -216,9 +219,9 @@ private function updateAssertStatusCall(MethodCall $methodCall): ?MethodCall
         } else {
             if (! in_array($this->getName($argValue->class), [
                 'Illuminate\Http\Response',
-                'Symfony\Component\HttpFoundation\Response'
+                'Symfony\Component\HttpFoundation\Response',
             ], true)) {
-               return null;
+                return null;
             }
 
             $replacementMethod = match ($this->getName($argValue->name)) {
@@ -239,7 +242,7 @@ private function updateAssertStatusCall(MethodCall $methodCall): ?MethodCall
             return null;
         }
 
-        $methodCall->name = new Node\Identifier($replacementMethod);
+        $methodCall->name = new Identifier($replacementMethod);
         $methodCall->args = [];
 
         return $methodCall;

diff --git a/src/Rector/MethodCall/RedirectBackToBackHelperRector.php b/src/Rector/MethodCall/RedirectBackToBackHelperRector.php
@@ -90,7 +90,7 @@ public function refactor(Node $node): ?Node
         return $this->updateRedirectStaticCall($node);
     }
 
-    private function updateRedirectHelperCall(MethodCall $methodCall): ?MethodCall
+    private function updateRedirectHelperCall(MethodCall $methodCall): MethodCall|FuncCall|null
     {
         if (! $this->isName($methodCall->name, 'back')) {
             return null;
@@ -115,9 +115,15 @@ private function updateRedirectHelperCall(MethodCall $methodCall): ?MethodCall
             return null;
         }
 
-        $this->removeNode($methodCall);
+        $childElement = $methodCall->getAttribute('parent');
 
-        $parentNode->var->name = new Name('back');
+        if ($childElement instanceof MethodCall) {
+            $this->removeNode($methodCall);
+            $parentNode->var->name = new Name('back');
+            $parentNode->var->args = $methodCall->getArgs();
+        } else {
+            return new FuncCall(new Name('back'), $methodCall->getArgs());
+        }
 
         return $parentNode;
     }

diff --git a/src/Rector/MethodCall/RedirectRouteToToRouteHelperRector.php b/src/Rector/MethodCall/RedirectRouteToToRouteHelperRector.php
@@ -90,7 +90,7 @@ public function refactor(Node $node): ?Node
         return $this->updateRedirectStaticCall($node);
     }
 
-    private function updateRedirectHelperCall(MethodCall $methodCall): ?MethodCall
+    private function updateRedirectHelperCall(MethodCall $methodCall): MethodCall|FuncCall|null
     {
         if (! $this->isName($methodCall->name, 'route')) {
             return null;
@@ -115,10 +115,15 @@ private function updateRedirectHelperCall(MethodCall $methodCall): ?MethodCall
             return null;
         }
 
-        $this->removeNode($methodCall);
+        $childElement = $methodCall->getAttribute('parent');
 
-        $parentNode->var->name = new Name('to_route');
-        $parentNode->var->args = $methodCall->getArgs();
+        if ($childElement instanceof MethodCall) {
+            $this->removeNode($methodCall);
+            $parentNode->var->name = new Name('to_route');
+            $parentNode->var->args = $methodCall->getArgs();
+        } else {
+            return new FuncCall(new Name('to_route'), $methodCall->getArgs());
+        }
 
         return $parentNode;
     }

diff --git a/src/Rector/StaticCall/RouteActionCallableRector.php b/src/Rector/StaticCall/RouteActionCallableRector.php
@@ -7,6 +7,8 @@
 use PhpParser\Node;
 use PhpParser\Node\Arg;
 use PhpParser\Node\Expr;
+use PhpParser\Node\Expr\Array_;
+use PhpParser\Node\Expr\ArrayItem;
 use PhpParser\Node\Expr\MethodCall;
 use PhpParser\Node\Expr\StaticCall;
 use PhpParser\Node\Identifier;
@@ -140,16 +142,12 @@ public function refactor(Node $node): ?Node
             if (is_string($argValue['middleware'])) {
                 $argument = new String_($argValue['middleware']);
             } else {
-                $argument = new Node\Expr\Array_(array_map(
-                    static fn ($value) => new Node\Expr\ArrayItem(new String_($value)),
+                $argument = new Array_(array_map(
+                    static fn ($value) => new ArrayItem(new String_($value)),
                     $argValue['middleware']
                 ));
             }
-            $node = new MethodCall(
-                $node,
-                'middleware',
-                [new Arg($argument)]
-            );
+            $node = new MethodCall($node, 'middleware', [new Arg($argument)]);
         }
 
         return $node;
@@ -172,8 +170,6 @@ public function configure(array $configuration): void
     }
 
     /**
-     * @param mixed $action
-     *
      * @return array{string, string}|null
      */
     private function resolveControllerFromAction(mixed $action): ?array

diff --git a/tests/Rector/MethodCall/AssertStatusToAssertMethodRector/config/configured_rule.php b/tests/Rector/MethodCall/AssertStatusToAssertMethodRector/config/configured_rule.php
@@ -4,10 +4,10 @@
 
 use Rector\Config\RectorConfig;
 
-use RectorLaravel\Rector\MethodCall\RedirectBackToBackHelperRector;
+use RectorLaravel\Rector\MethodCall\AssertStatusToAssertMethodRector;
 
 return static function (RectorConfig $rectorConfig): void {
     $rectorConfig->import(__DIR__ . '/../../../../../config/config.php');
 
-    $rectorConfig->rule(\RectorLaravel\Rector\MethodCall\AssertStatusToAssertMethodRector::class);
+    $rectorConfig->rule(AssertStatusToAssertMethodRector::class);
 };
diff --git a/tests/Rector/MethodCall/RedirectBackToBackHelperRector/Fixture/fixture_with_argument.php.inc b/tests/Rector/MethodCall/RedirectBackToBackHelperRector/Fixture/fixture_with_argument.php.inc
@@ -0,0 +1,41 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\MethodCall\RedirectBackToBackHelperRector\Fixture;
+
+use Illuminate\Support\Facades\Redirect;
+
+class FixtureWithArgument
+{
+    public function store()
+    {
+        return redirect()->back(302);
+    }
+
+    public function update()
+    {
+        return Redirect::back(302);
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\MethodCall\RedirectBackToBackHelperRector\Fixture;
+
+use Illuminate\Support\Facades\Redirect;
+
+class FixtureWithArgument
+{
+    public function store()
+    {
+        return back(302);
+    }
+
+    public function update()
+    {
+        return back(302);
+    }
+}
+
+?>
diff --git a/tests/Rector/MethodCall/RedirectBackToBackHelperRector/Fixture/fixture_without_with.php.inc b/tests/Rector/MethodCall/RedirectBackToBackHelperRector/Fixture/fixture_without_with.php.inc
@@ -0,0 +1,41 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\MethodCall\RedirectBackToBackHelperRector\Fixture;
+
+use Illuminate\Support\Facades\Redirect;
+
+class FixtureWithoutWith
+{
+    public function store()
+    {
+        return redirect()->back();
+    }
+
+    public function update()
+    {
+        return Redirect::back();
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\MethodCall\RedirectBackToBackHelperRector\Fixture;
+
+use Illuminate\Support\Facades\Redirect;
+
+class FixtureWithoutWith
+{
+    public function store()
+    {
+        return back();
+    }
+
+    public function update()
+    {
+        return back();
+    }
+}
+
+?>
diff --git a/...Rector/MethodCall/RedirectRouteToToRouteHelperRector/Fixture/fixture_without_with.php.inc b/...Rector/MethodCall/RedirectRouteToToRouteHelperRector/Fixture/fixture_without_with.php.inc
@@ -0,0 +1,41 @@
+<?php
+
+namespace RectorLaravel\Tests\Rector\MethodCall\RedirectRouteToToRouteHelperRector\Fixture;
+
+use Illuminate\Support\Facades\Redirect;
+
+class FixtureWithoutWith
+{
+    public function store()
+    {
+        return redirect()->route('home');
+    }
+
+    public function update()
+    {
+        return Redirect::route('home');
+    }
+}
+
+?>
+-----
+<?php
+
+namespace RectorLaravel\Tests\Rector\MethodCall\RedirectRouteToToRouteHelperRector\Fixture;
+
+use Illuminate\Support\Facades\Redirect;
+
+class FixtureWithoutWith
+{
+    public function store()
+    {
+        return to_route('home');
+    }
+
+    public function update()
+    {
+        return to_route('home');
+    }
+}
+
+?>