Skip to content

Commit 8360bd1

Browse files
TomasVotrubaTomasVotruba
authored
[code-quality] Add ReturnDirectJsonResponseRector (#910)
1 parent 6458536 commit 8360bd1

File tree

6 files changed

+287
-1
lines changed

6 files changed

+287
-1
lines changed

‎config/sets/symfony/symfony-code-quality.php‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Rector\Symfony\CodeQuality\Rector\ClassMethod\ParamTypeFromRouteRequiredRegexRector;
1616
use Rector\Symfony\CodeQuality\Rector\ClassMethod\RemoveUnusedRequestParamRector;
1717
use Rector\Symfony\CodeQuality\Rector\ClassMethod\ResponseReturnTypeControllerActionRector;
18+
use Rector\Symfony\CodeQuality\Rector\ClassMethod\ReturnDirectJsonResponseRector;
1819
use Rector\Symfony\CodeQuality\Rector\MethodCall\AssertSameResponseCodeWithDebugContentsRector;
1920
use Rector\Symfony\CodeQuality\Rector\MethodCall\LiteralGetToRequestClassConstantRector;
2021
use Rector\Symfony\CodeQuality\Rector\MethodCall\ParameterBagTypedGetMethodCallRector;
@@ -43,6 +44,9 @@
4344
RequestIsMainRector::class,
4445
ParameterBagTypedGetMethodCallRector::class,
4546

47+
// enable once tested
48+
// ReturnDirectJsonResponseRector::class,
49+
4650
// tests
4751
AssertSameResponseCodeWithDebugContentsRector::class,
4852
StringCastDebugResponseRector::class,

‎phpstan.neon‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
# @todo enable
2+
#includes:
3+
# - vendor/symplify/phpstan-rules/config/symplify-rules.neon
4+
# - vendor/symplify/phpstan-rules/config/rector-rules.neon
5+
16
rules:
27
- Symplify\PHPStanRules\Rules\StringFileAbsolutePathExistsRule
38

@@ -65,4 +70,4 @@ parameters:
6570
- '#Parameter 1 should use "PHPStan\\BetterReflection\\Reflection\\Adapter\\ReflectionMethod" type as the only type passed to this method#'
6671

6772
# local use php 8.3
68-
- identifier: typeCoverage.constantTypeCoverage
73+
- identifier: typeCoverage.constantTypeCoverage

‎rules-tests/CodeQuality/Rector/ClassMethod/ReturnDirectJsonResponseRector/Fixture/some_controller_class.php.inc‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
<?php
2+
3+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\ReturnDirectJsonResponseRector\Fixture;
4+
5+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
6+
use Symfony\Component\HttpFoundation\JsonResponse;
7+
8+
final class SomeControllerClass extends AbstractController
9+
{
10+
public function index()
11+
{
12+
$jsonResponse = new JsonResponse();
13+
$jsonResponse->setData([
14+
'status' => 'ok',
15+
]);
16+
17+
return $jsonResponse;
18+
}
19+
}
20+
21+
?>
22+
-----
23+
<?php
24+
25+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\ReturnDirectJsonResponseRector\Fixture;
26+
27+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
28+
use Symfony\Component\HttpFoundation\JsonResponse;
29+
30+
final class SomeControllerClass extends AbstractController
31+
{
32+
public function index()
33+
{
34+
return new \Symfony\Component\HttpFoundation\JsonResponse([
35+
'status' => 'ok',
36+
]);
37+
}
38+
}
39+
40+
?>

‎rules-tests/CodeQuality/Rector/ClassMethod/ReturnDirectJsonResponseRector/ReturnDirectJsonResponseRectorTest.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\ReturnDirectJsonResponseRector;
6+
7+
use Iterator;
8+
use PHPUnit\Framework\Attributes\DataProvider;
9+
use Rector\Testing\PHPUnit\AbstractRectorTestCase;
10+
11+
final class ReturnDirectJsonResponseRectorTest extends AbstractRectorTestCase
12+
{
13+
#[DataProvider('provideData')]
14+
public function test(string $filePath): void
15+
{
16+
$this->doTestFile($filePath);
17+
}
18+
19+
public static function provideData(): Iterator
20+
{
21+
return self::yieldFilesFromDirectory(__DIR__ . '/Fixture');
22+
}
23+
24+
public function provideConfigFilePath(): string
25+
{
26+
return __DIR__ . '/config/configured_rule.php';
27+
}
28+
}

‎rules-tests/CodeQuality/Rector/ClassMethod/ReturnDirectJsonResponseRector/config/configured_rule.php‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
use Rector\Config\RectorConfig;
6+
use Rector\Symfony\CodeQuality\Rector\ClassMethod\ReturnDirectJsonResponseRector;
7+
8+
return static function (RectorConfig $rectorConfig): void {
9+
$rectorConfig->rule(ReturnDirectJsonResponseRector::class);
10+
};

‎rules/CodeQuality/Rector/ClassMethod/ReturnDirectJsonResponseRector.php‎

Lines changed: 199 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,199 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\Symfony\CodeQuality\Rector\ClassMethod;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Arg;
9+
use PhpParser\Node\Expr;
10+
use PhpParser\Node\Expr\Assign;
11+
use PhpParser\Node\Expr\MethodCall;
12+
use PhpParser\Node\Expr\New_;
13+
use PhpParser\Node\Expr\Variable;
14+
use PhpParser\Node\Name\FullyQualified;
15+
use PhpParser\Node\Stmt;
16+
use PhpParser\Node\Stmt\ClassMethod;
17+
use PhpParser\Node\Stmt\Expression;
18+
use PhpParser\Node\Stmt\Return_;
19+
use Rector\Rector\AbstractRector;
20+
use Rector\Symfony\Bridge\NodeAnalyzer\ControllerMethodAnalyzer;
21+
use Rector\Symfony\CodeQuality\Enum\ResponseClass;
22+
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
23+
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
24+
25+
/**
26+
* @see \Rector\Symfony\Tests\CodeQuality\Rector\ClassMethod\ReturnDirectJsonResponseRector\ReturnDirectJsonResponseRectorTest
27+
*/
28+
final class ReturnDirectJsonResponseRector extends AbstractRector
29+
{
30+
public function __construct(
31+
private readonly ControllerMethodAnalyzer $controllerMethodAnalyzer,
32+
) {
33+
}
34+
35+
public function getRuleDefinition(): RuleDefinition
36+
{
37+
return new RuleDefinition(
38+
'Instead of creating JsonResponse, setting items and data, return it directly at once to improve readability',
39+
[
40+
new CodeSample(
41+
<<<'CODE_SAMPLE'
42+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
43+
44+
class SomeController extends AbstractController
45+
{
46+
public function index()
47+
{
48+
$response = new JsonResponse();
49+
$response->setData(['key' => 'value']);
50+
51+
return $response;
52+
}
53+
}
54+
CODE_SAMPLE
55+
,
56+
<<<'CODE_SAMPLE'
57+
use Symfony\Bundle\FrameworkBundle\Controller\AbstractController;
58+
59+
class SomeController extends AbstractController
60+
{
61+
public function index()
62+
{
63+
return new JsonResponse(['key' => 'value']);
64+
}
65+
}
66+
CODE_SAMPLE
67+
),
68+
]
69+
);
70+
}
71+
72+
/**
73+
* @return array<class-string<Node>>
74+
*/
75+
public function getNodeTypes(): array
76+
{
77+
return [ClassMethod::class];
78+
}
79+
80+
/**
81+
* @param ClassMethod $node
82+
*/
83+
public function refactor(Node $node): ?Node
84+
{
85+
if (! $this->controllerMethodAnalyzer->isAction($node)) {
86+
return null;
87+
}
88+
89+
$jsonResponseVariable = null;
90+
$assignStmtPosition = null;
91+
92+
$setDataExpr = null;
93+
$setDataPosition = null;
94+
95+
foreach ((array) $node->stmts as $key => $stmt) {
96+
if ($assignStmtPosition === null) {
97+
$jsonResponseVariable = $this->matchNewJsonResponseAssignVariable($stmt);
98+
if ($jsonResponseVariable instanceof Variable) {
99+
$assignStmtPosition = $key;
100+
continue;
101+
}
102+
}
103+
104+
if ($setDataPosition === null) {
105+
if ($jsonResponseVariable instanceof Variable) {
106+
$setDataExpr = $this->matchSetDataMethodCallExpr($stmt, $jsonResponseVariable);
107+
if ($setDataExpr instanceof Expr) {
108+
$setDataPosition = $key;
109+
continue;
110+
}
111+
}
112+
}
113+
114+
if (! $stmt instanceof Return_) {
115+
continue;
116+
}
117+
118+
// missing important data
119+
if (! $jsonResponseVariable instanceof Variable || ! $setDataExpr instanceof Expr) {
120+
continue;
121+
}
122+
123+
if (! $this->nodeComparator->areNodesEqual($stmt->expr, $jsonResponseVariable)) {
124+
continue;
125+
}
126+
127+
$jsonResponseNew = new New_(new FullyQualified(ResponseClass::JSON), [new Arg($setDataExpr)]);
128+
129+
$stmt->expr = $jsonResponseNew;
130+
131+
// remove previous setData and assignment statements
132+
unset($node->stmts[$assignStmtPosition], $node->stmts[$setDataPosition]);
133+
134+
// reindex statements
135+
$node->stmts = array_values($node->stmts);
136+
137+
return $node;
138+
}
139+
140+
return null;
141+
}
142+
143+
private function matchNewJsonResponseAssignVariable(Stmt $stmt): ?Variable
144+
{
145+
if (! $stmt instanceof Expression) {
146+
return null;
147+
}
148+
149+
if (! $stmt->expr instanceof Assign) {
150+
return null;
151+
}
152+
153+
$assign = $stmt->expr;
154+
if (! $assign->expr instanceof New_) {
155+
return null;
156+
}
157+
158+
if (! $this->isName($assign->expr->class, ResponseClass::JSON)) {
159+
return null;
160+
}
161+
162+
if (! $assign->var instanceof Variable) {
163+
return null;
164+
}
165+
166+
return $assign->var;
167+
}
168+
169+
private function matchSetDataMethodCallExpr(Stmt $stmt, Variable $jsonResponseVariable): ?Expr
170+
{
171+
if (! $stmt instanceof Expression) {
172+
return null;
173+
}
174+
175+
if (! $stmt->expr instanceof MethodCall) {
176+
return null;
177+
}
178+
179+
$methodCall = $stmt->expr;
180+
if (! $this->isName($methodCall->name, 'setData')) {
181+
return null;
182+
}
183+
184+
if (! $this->nodeComparator->areNodesEqual($methodCall->var, $jsonResponseVariable)) {
185+
return null;
186+
}
187+
188+
if ($methodCall->isFirstClassCallable()) {
189+
return null;
190+
}
191+
192+
if ($methodCall->getArgs() === []) {
193+
return null;
194+
}
195+
196+
$soleArg = $methodCall->getArgs()[0];
197+
return $soleArg->value;
198+
}
199+
}

0 commit comments

Comments
 (0)