Skip to content

Commit 169cbec

Browse files
calebdwcalebdw
authored
feat: improve AppEnvironmentComparisonToParameterRector (#368)
This adds several improvements: - support for the `in_array` function - support for != and !== operators - support for the cleaner `isLocal` and `isProduction` methods
1 parent ee0cb06 commit 169cbec

File tree

5 files changed

+169
-43
lines changed

5 files changed

+169
-43
lines changed

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
"symplify/rule-doc-generator-contracts": "^11.2"
1111
},
1212
"require-dev": {
13-
"nikic/php-parser": "^5.3",
13+
"nikic/php-parser": "^5.6",
1414
"phpstan/extension-installer": "^1.3",
1515
"phpstan/phpstan": "^2.0",
1616
"phpstan/phpstan-deprecation-rules": "^2.0",

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use RectorLaravel\Rector\Assign\CallOnAppArrayAccessToStandaloneAssignRector;
77
use RectorLaravel\Rector\ClassMethod\MakeModelAttributesAndScopesProtectedRector;
88
use RectorLaravel\Rector\Coalesce\ApplyDefaultInsteadOfNullCoalesceRector;
9+
use RectorLaravel\Rector\Expr\AppEnvironmentComparisonToParameterRector;
910
use RectorLaravel\Rector\MethodCall\ReverseConditionableMethodCallRector;
1011

1112
return static function (RectorConfig $rectorConfig): void {
@@ -14,4 +15,5 @@
1415
$rectorConfig->rule(ReverseConditionableMethodCallRector::class);
1516
$rectorConfig->rule(ApplyDefaultInsteadOfNullCoalesceRector::class);
1617
$rectorConfig->rule(MakeModelAttributesAndScopesProtectedRector::class);
18+
$rectorConfig->rule(AppEnvironmentComparisonToParameterRector::class);
1719
};

‎src/Rector/Expr/AppEnvironmentComparisonToParameterRector.php‎

Lines changed: 104 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,16 @@
77
use PhpParser\Node;
88
use PhpParser\Node\Arg;
99
use PhpParser\Node\Expr;
10+
use PhpParser\Node\Expr\BinaryOp;
1011
use PhpParser\Node\Expr\BinaryOp\Equal;
1112
use PhpParser\Node\Expr\BinaryOp\Identical;
13+
use PhpParser\Node\Expr\BinaryOp\NotEqual;
14+
use PhpParser\Node\Expr\BinaryOp\NotIdentical;
15+
use PhpParser\Node\Expr\BooleanNot;
16+
use PhpParser\Node\Expr\FuncCall;
1217
use PhpParser\Node\Expr\MethodCall;
1318
use PhpParser\Node\Expr\StaticCall;
19+
use PhpParser\Node\Identifier;
1420
use PhpParser\Node\Scalar\String_;
1521
use PHPStan\Type\ObjectType;
1622
use RectorLaravel\AbstractRector;
@@ -25,82 +31,145 @@ class AppEnvironmentComparisonToParameterRector extends AbstractRector
2531
public function getRuleDefinition(): RuleDefinition
2632
{
2733
return new RuleDefinition(
28-
'Replace `$app->environment() === \'local\'` with `$app->environment(\'local\')`',
34+
'Replace app environment comparison with parameter or method call',
2935
[
3036
new CodeSample(
3137
<<<'CODE_SAMPLE'
32-
$app->environment() === 'production';
38+
$app->environment() === 'local';
39+
$app->environment() !== 'production';
40+
$app->environment() === 'testing';
41+
in_array($app->environment(), ['local', 'testing']);
3342
CODE_SAMPLE
3443
,
3544
<<<'CODE_SAMPLE'
36-
$app->environment('production');
45+
$app->isLocal();
46+
! $app->isProduction();
47+
$app->environment('testing');
48+
$app->environment(['local', 'testing']);
3749
CODE_SAMPLE
3850
),
3951
]
4052
);
4153
}
4254

55+
/** @return list<class-string<Node>> */
4356
public function getNodeTypes(): array
4457
{
45-
return [Expr::class];
58+
return [FuncCall::class, Identical::class, Equal::class, NotIdentical::class, NotEqual::class];
4659
}
4760

48-
public function refactor(Node $node): MethodCall|StaticCall|null
61+
/** @param FuncCall|Identical|Equal|NotIdentical|NotEqual $node */
62+
public function refactor(Node $node): ?Expr
4963
{
50-
if (! $node instanceof Identical && ! $node instanceof Equal) {
64+
if ($node instanceof FuncCall) {
65+
[$methodCall, $otherNode] = $this->handleFuncCall($node);
66+
} else {
67+
[$methodCall, $otherNode] = $this->handleBinaryOp($node);
68+
}
69+
70+
if ($methodCall === null || $otherNode === null) {
5171
return null;
5272
}
5373

54-
/** @var MethodCall|StaticCall|null $methodCall */
74+
$methodName = null;
75+
76+
if ($otherNode instanceof String_) {
77+
$methodName = $this->getMethodName($methodCall, $otherNode->value);
78+
}
79+
80+
if ($methodName === null) {
81+
$methodCall->args[] = new Arg($otherNode);
82+
} else {
83+
$methodCall->name = new Identifier($methodName);
84+
}
85+
86+
if ($node instanceof NotIdentical || $node instanceof NotEqual) {
87+
return new BooleanNot($methodCall);
88+
}
89+
90+
return $methodCall;
91+
}
92+
93+
/** @return array{0: MethodCall|StaticCall|null, 1: Expr|null} */
94+
private function handleFuncCall(FuncCall $funcCall): array
95+
{
96+
if (! $this->isName($funcCall->name, 'in_array')) {
97+
return [null, null];
98+
}
99+
100+
$methodCall = $funcCall->getArg('needle', 0);
101+
$haystack = $funcCall->getArg('haystack', 1);
102+
103+
if (! $haystack instanceof Arg || ! $methodCall instanceof Arg || ! $this->validMethodCall($methodCall->value)) {
104+
return [null, null];
105+
}
106+
107+
return [$methodCall->value, $haystack->value];
108+
}
109+
110+
/** @return array{0: MethodCall|StaticCall|null, 1: Expr|null} */
111+
private function handleBinaryOp(BinaryOp $binaryOp): array
112+
{
55113
$methodCall = array_values(
56114
array_filter(
57-
[$node->left, $node->right],
58-
fn ($node) => ($node instanceof MethodCall || $node instanceof StaticCall) && $this->isName(
59-
$node->name,
60-
'environment'
61-
)
115+
[$binaryOp->left, $binaryOp->right],
116+
fn ($node) => $this->validMethodCall($node),
62117
)
63118
)[0] ?? null;
64119

65-
if ($methodCall === null || ! $this->validMethodCall($methodCall)) {
66-
return null;
67-
}
68-
69-
/** @var Expr $otherNode */
70120
$otherNode = array_values(
71-
array_filter([$node->left, $node->right], static fn ($node) => $node !== $methodCall)
121+
array_filter([$binaryOp->left, $binaryOp->right], static fn ($node) => $node !== $methodCall)
72122
)[0] ?? null;
73123

74-
if (! $otherNode instanceof String_) {
75-
return null;
76-
}
124+
return [$methodCall, $otherNode];
125+
}
77126

78-
// make sure the method call has no arguments
79-
if ($methodCall->getArgs() !== []) {
80-
return null;
127+
/** @phpstan-assert-if-true MethodCall|StaticCall $node */
128+
private function validMethodCall(Node $node): bool
129+
{
130+
if (! $node instanceof MethodCall && ! $node instanceof StaticCall) {
131+
return false;
81132
}
82133

83-
$methodCall->args[] = new Arg($otherNode);
134+
if (! $this->isName($node->name, 'environment')) {
135+
return false;
136+
}
84137

85-
return $methodCall;
86-
}
138+
// make sure the method call has no arguments
139+
if ($node->getArgs() !== []) {
140+
return false;
141+
}
87142

88-
private function validMethodCall(MethodCall|StaticCall $methodCall): bool
89-
{
90143
return match (true) {
91-
$methodCall instanceof MethodCall && $this->isObjectType(
92-
$methodCall->var,
144+
$node instanceof MethodCall && $this->isObjectType(
145+
$node->var,
93146
new ObjectType('Illuminate\Contracts\Foundation\Application')
94147
) => true,
95-
$methodCall instanceof StaticCall && $this->isObjectType(
96-
$methodCall->class,
148+
$node instanceof StaticCall && $this->isObjectType(
149+
$node->class,
97150
new ObjectType('Illuminate\Support\Facades\App')
98151
) => true,
99-
$methodCall instanceof StaticCall && $this->isObjectType(
100-
$methodCall->class,
152+
$node instanceof StaticCall && $this->isObjectType(
153+
$node->class,
101154
new ObjectType('App')
102155
) => true,
103156
default => false,
104157
};
105158
}
159+
160+
private function getMethodName(MethodCall|StaticCall $methodCall, string $environment): ?string
161+
{
162+
if (
163+
$methodCall instanceof MethodCall
164+
&& ! $this->isObjectType($methodCall->var, new ObjectType('Illuminate\Foundation\Application'))
165+
) {
166+
return null;
167+
}
168+
169+
return match ($environment) {
170+
'local' => 'isLocal',
171+
'production' => 'isProduction',
172+
default => null,
173+
};
174+
}
106175
}

‎tests/Rector/Expr/AppEnvironmentComparisonToParameterRector/Fixture/fixture.php.inc‎

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,25 @@
22

33
namespace RectorLaravel\Tests\Rector\Expr\AppEnvironmentComparisonToParameterRector\Fixture;
44

5-
/** @var \Illuminate\Contracts\Foundation\Application $app */
5+
/** @var \Illuminate\Foundation\Application $app */
6+
/** @var \Illuminate\Contracts\Foundation\Application $appContract */
7+
$app->environment() === 'local';
8+
$app->environment() === 'staging';
69
$app->environment() === 'production';
10+
$appContract->environment() === 'local';
11+
$appContract->environment() === 'staging';
12+
$appContract->environment() === 'production';
713
'staging' == $app->environment();
14+
'testing' != $app->environment();
815

9-
if ($app->environment() === 'production') {
16+
if ($app->environment() !== 'production') {
1017
}
1118

19+
\Illuminate\Support\Facades\App::environment() === 'local';
20+
\Illuminate\Support\Facades\App::environment() === 'staging';
1221
\Illuminate\Support\Facades\App::environment() === 'production';
22+
\App::environment() === 'local';
23+
\App::environment() === 'staging';
1324
\App::environment() === 'production';
1425

1526
?>
@@ -18,14 +29,25 @@ if ($app->environment() === 'production') {
1829

1930
namespace RectorLaravel\Tests\Rector\Expr\AppEnvironmentComparisonToParameterRector\Fixture;
2031

21-
/** @var \Illuminate\Contracts\Foundation\Application $app */
22-
$app->environment('production');
32+
/** @var \Illuminate\Foundation\Application $app */
33+
/** @var \Illuminate\Contracts\Foundation\Application $appContract */
34+
$app->isLocal();
2335
$app->environment('staging');
36+
$app->isProduction();
37+
$appContract->environment('local');
38+
$appContract->environment('staging');
39+
$appContract->environment('production');
40+
$app->environment('staging');
41+
!$app->environment('testing');
2442

25-
if ($app->environment('production')) {
43+
if (!$app->isProduction()) {
2644
}
2745

28-
\Illuminate\Support\Facades\App::environment('production');
29-
\App::environment('production');
46+
\Illuminate\Support\Facades\App::isLocal();
47+
\Illuminate\Support\Facades\App::environment('staging');
48+
\Illuminate\Support\Facades\App::isProduction();
49+
\App::isLocal();
50+
\App::environment('staging');
51+
\App::isProduction();
3052

3153
?>

‎tests/Rector/Expr/AppEnvironmentComparisonToParameterRector/Fixture/in_array.php.inc‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
namespace RectorLaravel\Tests\Rector\Expr\AppEnvironmentComparisonToParameterRector\Fixture;
4+
5+
/** @var \Illuminate\Foundation\Application $app */
6+
/** @var \Illuminate\Contracts\Foundation\Application $appContract */
7+
in_array($app->environment(), ['local']);
8+
in_array($appContract->environment(), ['testing'], true);
9+
10+
if (! in_array($app->environment(), ['local', 'testing'], true)) {
11+
}
12+
13+
in_array(\Illuminate\Support\Facades\App::environment(), ['local', 'testing']);
14+
in_array(\App::environment(), ['local', 'testing']);
15+
16+
?>
17+
-----
18+
<?php
19+
20+
namespace RectorLaravel\Tests\Rector\Expr\AppEnvironmentComparisonToParameterRector\Fixture;
21+
22+
/** @var \Illuminate\Foundation\Application $app */
23+
/** @var \Illuminate\Contracts\Foundation\Application $appContract */
24+
$app->environment(['local']);
25+
$appContract->environment(['testing']);
26+
27+
if (! $app->environment(['local', 'testing'])) {
28+
}
29+
30+
\Illuminate\Support\Facades\App::environment(['local', 'testing']);
31+
\App::environment(['local', 'testing']);
32+
33+
?>

0 commit comments

Comments
 (0)