From 5cafba7117f3c0530a982af5b6627101e6a76482 Mon Sep 17 00:00:00 2001 From: Geni Jaho Date: Wed, 17 Jul 2024 22:48:41 +0200 Subject: [PATCH] Handle JSON GET calls with the route helper (#231) --- .../JsonCallToExplicitJsonCallRector.php | 71 +++++++++++++++++-- .../Testing/Concerns/MakesHttpRequests.php | 11 +++ .../Fixture/fixture_with_json_calls.php.inc | 49 +++++++++---- .../fixture_with_json_calls_to_skip.php.inc | 10 +-- .../skip_invalid_get_json_calls.php.inc | 18 ++++- 5 files changed, 132 insertions(+), 27 deletions(-) create mode 100644 stubs/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php diff --git a/src/Rector/MethodCall/JsonCallToExplicitJsonCallRector.php b/src/Rector/MethodCall/JsonCallToExplicitJsonCallRector.php index 512345d1..80ef2b42 100644 --- a/src/Rector/MethodCall/JsonCallToExplicitJsonCallRector.php +++ b/src/Rector/MethodCall/JsonCallToExplicitJsonCallRector.php @@ -5,6 +5,8 @@ namespace RectorLaravel\Rector\MethodCall; use PhpParser\Node; +use PhpParser\Node\Arg; +use PhpParser\Node\Expr\FuncCall; use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Identifier; use PhpParser\Node\Scalar\String_; @@ -77,19 +79,78 @@ private function updateCall(MethodCall $methodCall): ?MethodCall return null; } - $supportedMethods = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE', 'OPTIONS']; + $lowercaseMethodName = strtolower($firstArg->value); - if (! in_array($firstArg->value, $supportedMethods, true)) { + $supportedMethods = ['get', 'post', 'put', 'patch', 'delete', 'options']; + + if (! in_array($lowercaseMethodName, $supportedMethods, true)) { return null; } - if ($firstArg->value === 'GET' && count($methodCallArgs) > 2) { - return null; + if ($lowercaseMethodName === 'get' && count($methodCallArgs) > 2) { + return $this->refactorGetMethodCall($methodCall); } - $methodCall->name = new Identifier(strtolower($firstArg->value) . 'Json'); + $methodCall->name = $this->getMethodCallName($lowercaseMethodName); $methodCall->args = array_slice($methodCall->args, 1); return $methodCall; } + + /** + * Set the $data argument from the json('GET') call (3rd argument) + * as the route() helper second argument + */ + private function refactorGetMethodCall(MethodCall $methodCall): ?MethodCall + { + if (! $this->isUsingChangeableRouteHelper($methodCall)) { + return null; + } + + $thirdArg = $methodCall->getArgs()[2]; + + // If it's a named argument, and it's not $data, we won't refactor + if ($thirdArg->name !== null && ! $this->isName($thirdArg, 'data')) { + return null; + } + + /** @var FuncCall $routeHelperCall */ + $routeHelperCall = $methodCall->getArgs()[1]->value; + + $routeHelperCall->args = [ + $routeHelperCall->args[0], + new Arg($thirdArg->value), + ]; + + $methodCall->name = $this->getMethodCallName('get'); + $methodCall->args = [new Arg($routeHelperCall)]; + + return $methodCall; + } + + private function isUsingChangeableRouteHelper(MethodCall $methodCall): bool + { + $methodCallArgs = $methodCall->getArgs(); + + // More than 3 arguments means we loose $headers or $options if we refactor + if (count($methodCallArgs) !== 3) { + return false; + } + + $secondArg = $methodCallArgs[1]->value; + + if (! ($secondArg instanceof FuncCall && $this->isName($secondArg, 'route'))) { + return false; + } + + // If there is more than 1 argument in the route() helper + // we have to take into account merging the $data argument, + // but it's too unpredictable to refactor + return count($secondArg->args) === 1; + } + + private function getMethodCallName(string $lowercaseMethodName): Identifier + { + return new Identifier("{$lowercaseMethodName}Json"); + } } diff --git a/stubs/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php b/stubs/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php new file mode 100644 index 00000000..9f0f878a --- /dev/null +++ b/stubs/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php @@ -0,0 +1,11 @@ +json('get', '/'); } - public function testPost(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testGetWithRouteHelper(MakesHttpRequests $http) + { + $http->json('GET', route('home')); + $http->json('GET', route('home'), ['payload']); + $http->json('GET', route('home'), data: ['payload']); + $http->json('GET', route('home', 'some arg'), ['payload']); + } + + public function testPost(MakesHttpRequests $http) { $http->json('post', '/'); } - public function testPut(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testPut(MakesHttpRequests $http) { $http->json('put', '/'); } - public function testPatch(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testPatch(MakesHttpRequests $http) { $http->json('patch', '/'); } - public function testDelete(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testDelete(MakesHttpRequests $http) { $http->json('delete', '/'); } - public function testOptions(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testOptions(MakesHttpRequests $http) { $http->json('options', '/'); } } ?> ---- +----- getJson('/'); } - public function testPost(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testGetWithRouteHelper(MakesHttpRequests $http) + { + $http->getJson(route('home')); + $http->getJson(route('home', ['payload'])); + $http->getJson(route('home', ['payload'])); + $http->json('GET', route('home', 'some arg'), ['payload']); + } + + public function testPost(MakesHttpRequests $http) { $http->postJson('/'); } - public function testPut(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testPut(MakesHttpRequests $http) { $http->putJson('/'); } - public function testPatch(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testPatch(MakesHttpRequests $http) { $http->patchJson('/'); } - public function testDelete(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testDelete(MakesHttpRequests $http) { $http->deleteJson('/'); } - public function testOptions(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testOptions(MakesHttpRequests $http) { $http->optionsJson('/'); } } ?> - diff --git a/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/fixture_with_json_calls_to_skip.php.inc b/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/fixture_with_json_calls_to_skip.php.inc index 1118262a..e6fcb301 100644 --- a/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/fixture_with_json_calls_to_skip.php.inc +++ b/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/fixture_with_json_calls_to_skip.php.inc @@ -2,24 +2,26 @@ namespace RectorLaravel\Tests\Rector\MethodCall\JsonCallToExplicitJsonCallRector\Fixture; +use Illuminate\Foundation\Testing\Concerns\MakesHttpRequests; + class FixtureWithJsonCalls { - public function testHead(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testHead(MakesHttpRequests $http) { $http->json('head', '/'); } - public function testTrace(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testTrace(MakesHttpRequests $http) { $http->json('trace', '/'); } - public function testConnect(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testConnect(MakesHttpRequests $http) { $http->json('connect', '/'); } - public function testNotEnoughArgs(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testNotEnoughArgs(MakesHttpRequests $http) { $http->json('GET'); $http->json(); diff --git a/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/skip_invalid_get_json_calls.php.inc b/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/skip_invalid_get_json_calls.php.inc index 7c1f3774..d1ffc808 100644 --- a/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/skip_invalid_get_json_calls.php.inc +++ b/tests/Rector/MethodCall/JsonCallToExplicitJsonCallRector/Fixture/skip_invalid_get_json_calls.php.inc @@ -2,22 +2,34 @@ namespace RectorLaravel\Tests\Rector\MethodCall\JsonCallToExplicitJsonCallRector\Fixture; +use Illuminate\Foundation\Testing\Concerns\MakesHttpRequests; + class FixtureWithGetJsonCalls { - public function testGetWithAllParams(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testGetWithAllParams(MakesHttpRequests $http) { $http->json('GET', '/', ['data'], ['headers'], 1); } - public function testGetWithoutOptions(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testGetWithoutOptions(MakesHttpRequests $http) { $http->json('GET', '/', ['data'], ['headers']); } - public function testGetWithoutHeaders(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http) + public function testGetWithoutHeaders(MakesHttpRequests $http) { $http->json('GET', '/', ['data']); } + + public function testGetWithNamedHeaders(MakesHttpRequests $http) + { + $http->json('GET', route('home'), headers: ['payload']); + } + + public function testGetWithNamedOptions(MakesHttpRequests $http) + { + $http->json('GET', route('home'), options: 1); + } } ?>