Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix JSON GET calls with the route helper #231

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 66 additions & 5 deletions src/Rector/MethodCall/JsonCallToExplicitJsonCallRector.php
Original file line number Diff line number Diff line change
Expand Up @@ -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_;
Expand Down Expand Up @@ -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");
}
}
11 changes: 11 additions & 0 deletions stubs/Illuminate/Foundation/Testing/Concerns/MakesHttpRequests.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Illuminate\Foundation\Testing\Concerns;

if (class_exists('Illuminate\Foundation\Testing\Concerns\MakesHttpRequests')) {
return;
}

class MakesHttpRequests
{
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,77 +2,96 @@

namespace RectorLaravel\Tests\Rector\MethodCall\JsonCallToExplicitJsonCallRector\Fixture;

use Illuminate\Foundation\Testing\Concerns\MakesHttpRequests;

class FixtureWithJsonCalls
{
public function testGet(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http)
public function testGet(MakesHttpRequests $http)
{
$http->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', '/');
}
}

?>
---
-----
<?php

namespace RectorLaravel\Tests\Rector\MethodCall\AssertStatusToAssertMethodRector\Fixture;
namespace RectorLaravel\Tests\Rector\MethodCall\JsonCallToExplicitJsonCallRector\Fixture;

use Illuminate\Foundation\Testing\Concerns\MakesHttpRequests;

class FixtureWithJsonCalls
{
public function testGet(\Illuminate\Foundation\Testing\Concerns\MakesHttpRequests $http)
public function testGet(MakesHttpRequests $http)
{
$http->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('/');
}
}

?>

Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}

?>
Loading