Skip to content

Commit 6b8de93

Browse files
calebdwcalebdw
committed
fix: FunctionLikeToFirstClassCallableRector
1 parent efe86cc commit 6b8de93

4 files changed

Lines changed: 168 additions & 80 deletions

File tree

‎rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_chained_calls.php.inc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
4+
5+
function ($foo)
6+
{
7+
return FooBar::foo()->bar($foo);
8+
};
9+
10+
fn ($foo) => FooBar::foo()->bar($foo);

‎rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/skip_using_this_outside_object.php.inc‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
4+
5+
class SkipUsingThisOutsideObject
6+
{
7+
public static function boot(): void
8+
{
9+
self::macro('foo', fn () => $this->values());
10+
self::macro('foo', fn () => $this->values()->foo());
11+
}
12+
13+
public static function macro(string $name, callable $callback): void
14+
{
15+
}
16+
}

‎rules-tests/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector/Fixture/using_this_in_instance_method.php.inc‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
4+
5+
class UsingThisInInstanceMethod
6+
{
7+
public function test(): callable
8+
{
9+
return fn () => $this->values();
10+
}
11+
12+
public function values(): array
13+
{
14+
return [];
15+
}
16+
}
17+
18+
?>
19+
-----
20+
<?php
21+
22+
namespace Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\Fixture;
23+
24+
class UsingThisInInstanceMethod
25+
{
26+
public function test(): callable
27+
{
28+
return $this->values(...);
29+
}
30+
31+
public function values(): array
32+
{
33+
return [];
34+
}
35+
}
36+
37+
?>

‎rules/CodingStyle/Rector/FunctionLike/FunctionLikeToFirstClassCallableRector.php‎

Lines changed: 105 additions & 80 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,21 @@
77
use PhpParser\Node;
88
use PhpParser\Node\Arg;
99
use PhpParser\Node\Expr\ArrowFunction;
10+
use PhpParser\Node\Expr\CallLike;
1011
use PhpParser\Node\Expr\Closure;
12+
use PhpParser\Node\Expr\FuncCall;
1113
use PhpParser\Node\Expr\MethodCall;
1214
use PhpParser\Node\Expr\StaticCall;
1315
use PhpParser\Node\Identifier;
1416
use PhpParser\Node\Param;
1517
use PhpParser\Node\Stmt\Return_;
1618
use PhpParser\Node\VariadicPlaceholder;
19+
use PhpParser\NodeVisitor;
20+
use PHPStan\Analyser\Scope;
21+
use Rector\PHPStan\ScopeFetcher;
1722
use Rector\Rector\AbstractRector;
1823
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
1924
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
20-
use Webmozart\Assert\Assert;
2125

2226
/**
2327
* @see \Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\FunctionLikeToFirstClassCallableRectorTest
@@ -49,155 +53,176 @@ public function getNodeTypes(): array
4953
/**
5054
* @param ArrowFunction|Closure $node
5155
*/
52-
public function refactor(Node $node): null|StaticCall|MethodCall
56+
public function refactor(Node $node): null|CallLike
5357
{
54-
$extractedMethodCall = $this->extractMethodCallFromFuncLike($node);
58+
$callLike = $this->extractCallLike($node);
5559

56-
if (! $extractedMethodCall instanceof MethodCall && ! $extractedMethodCall instanceof StaticCall) {
60+
if ($callLike === null) {
5761
return null;
5862
}
5963

60-
if ($extractedMethodCall instanceof MethodCall) {
61-
return new MethodCall($extractedMethodCall->var, $extractedMethodCall->name, [new VariadicPlaceholder()]);
64+
if ($this->shouldSkip($node, $callLike, ScopeFetcher::fetch($node))) {
65+
return null;
6266
}
6367

64-
return new StaticCall($extractedMethodCall->class, $extractedMethodCall->name, [new VariadicPlaceholder()]);
65-
}
68+
$callLike->args = [new VariadicPlaceholder()];
6669

67-
private function extractMethodCallFromFuncLike(Closure|ArrowFunction $node): MethodCall|StaticCall|null
68-
{
69-
if ($node instanceof ArrowFunction) {
70-
if (
71-
($node->expr instanceof MethodCall || $node->expr instanceof StaticCall) &&
72-
! $node->expr->isFirstClassCallable() &&
73-
$this->notUsingNamedArgs($node->expr->getArgs()) &&
74-
$this->notUsingByRef($node->getParams()) &&
75-
$this->sameParamsForArgs($node->getParams(), $node->expr->getArgs()) &&
76-
$this->isNonDependantMethod($node->expr, $node->getParams())
77-
) {
78-
return $node->expr;
79-
}
70+
return $callLike;
71+
}
8072

81-
return null;
82-
}
73+
private function shouldSkip(
74+
ArrowFunction|Closure $node,
75+
FuncCall|MethodCall|StaticCall $callLike,
76+
Scope $scope
77+
): bool {
78+
$params = $node->getParams();
79+
$args = $callLike->getArgs();
8380

84-
if (count($node->stmts) != 1 || ! $node->getStmts()[0] instanceof Return_) {
85-
return null;
81+
if (
82+
$callLike->isFirstClassCallable()
83+
|| $this->isChainedCall($callLike)
84+
|| $this->isUsingNamedArgs($args)
85+
|| $this->isUsingByRef($params)
86+
|| $this->isNotUsingSameParamsForArgs($params, $args)
87+
|| $this->isDependantMethod($callLike, $params)
88+
|| $this->isUsingThisInNonObjectContext($callLike, $scope)
89+
) {
90+
return true;
8691
}
8792

88-
$callLike = $node->getStmts()[0]
89-
->expr;
93+
return false;
94+
}
9095

91-
if (! $callLike instanceof MethodCall && ! $callLike instanceof StaticCall) {
92-
return null;
96+
private function extractCallLike(Closure|ArrowFunction $node): FuncCall|MethodCall|StaticCall|null
97+
{
98+
if ($node instanceof Closure) {
99+
if (count($node->stmts) !== 1 || ! $node->stmts[0] instanceof Return_) {
100+
return null;
101+
}
102+
$callLike = $node->stmts[0]->expr;
103+
} else {
104+
$callLike = $node->expr;
93105
}
94106

95107
if (
96-
! $callLike->isFirstClassCallable() &&
97-
$this->notUsingNamedArgs($callLike->getArgs()) &&
98-
$this->notUsingByRef($node->getParams()) &&
99-
$this->sameParamsForArgs($node->getParams(), $callLike->getArgs()) &&
100-
$this->isNonDependantMethod($callLike, $node->getParams())) {
101-
return $callLike;
108+
! $callLike instanceof FuncCall
109+
&& ! $callLike instanceof MethodCall
110+
&& ! $callLike instanceof StaticCall
111+
) {
112+
return null;
102113
}
103114

104-
return null;
115+
return $callLike;
105116
}
106117

107118
/**
108-
* @param Node\Param[] $params
109-
* @param Node\Arg[] $args
119+
* @param Param[] $params
120+
* @param Arg[] $args
110121
*/
111-
private function sameParamsForArgs(array $params, array $args): bool
122+
private function isNotUsingSameParamsForArgs(array $params, array $args): bool
112123
{
113-
Assert::allIsInstanceOf($args, Arg::class);
114-
Assert::allIsInstanceOf($params, Param::class);
115-
116124
if (count($args) > count($params)) {
117-
return false;
125+
return true;
118126
}
119127

120128
if (count($args) === 1 && $args[0]->unpack) {
121-
return $params[0]->variadic;
129+
return ! $params[0]->variadic;
122130
}
123131

124132
foreach ($args as $key => $arg) {
125133
if (! $this->nodeComparator->areNodesEqual($arg->value, $params[$key]->var)) {
126-
return false;
134+
return true;
127135
}
128136
}
129137

130-
return true;
138+
return false;
131139
}
132140

133141
/**
134-
* Makes sure the parameter isn't used to make the call e.g. in the var or class
135-
*
136142
* @param Param[] $params
137143
*/
138-
private function isNonDependantMethod(StaticCall|MethodCall $expr, array $params): bool
144+
private function isDependantMethod(StaticCall|MethodCall|FuncCall $expr, array $params): bool
139145
{
140-
Assert::allIsInstanceOf($params, Param::class);
146+
if ($expr instanceof FuncCall) {
147+
return false;
148+
}
141149

142150
$found = false;
151+
$parentNode = $expr instanceof MethodCall ? $expr->var : $expr->class;
143152

144153
foreach ($params as $param) {
145-
if ($expr instanceof MethodCall) {
146-
$this->traverseNodesWithCallable($expr->var, function (Node $node) use ($param, &$found): null {
147-
if ($this->nodeComparator->areNodesEqual($node, $param->var)) {
148-
$found = true;
149-
}
150-
151-
return null;
152-
});
154+
$this->traverseNodesWithCallable($parentNode, function (Node $node) use ($param, &$found) {
155+
if ($this->nodeComparator->areNodesEqual($node, $param->var)) {
156+
$found = true;
157+
return NodeVisitor::STOP_TRAVERSAL;
158+
}
159+
});
160+
161+
if ($found) {
162+
return true;
153163
}
164+
}
154165

155-
if ($expr instanceof StaticCall) {
156-
$this->traverseNodesWithCallable($expr->class, function (Node $node) use ($param, &$found): null {
157-
if ($this->nodeComparator->areNodesEqual($node, $param->var)) {
158-
$found = true;
159-
}
166+
return false;
167+
}
160168

161-
return null;
162-
});
163-
}
169+
private function isUsingThisInNonObjectContext(FuncCall|MethodCall|StaticCall $callLike, Scope $scope): bool
170+
{
171+
if (! $callLike instanceof MethodCall) {
172+
return false;
173+
}
164174

165-
if ($found) {
166-
return false;
167-
}
175+
if (in_array('this', $scope->getDefinedVariables(), true)) {
176+
return false;
168177
}
169178

170-
return true;
179+
$found = false;
180+
181+
$this->traverseNodesWithCallable($callLike, function (Node $node) use (&$found) {
182+
if ($this->isName($node, 'this')) {
183+
$found = true;
184+
return NodeVisitor::STOP_TRAVERSAL;
185+
}
186+
});
187+
188+
return $found;
171189
}
172190

173191
/**
174192
* @param Param[] $params
175193
*/
176-
private function notUsingByRef(array $params): bool
194+
private function isUsingByRef(array $params): bool
177195
{
178-
Assert::allIsInstanceOf($params, Param::class);
179-
180196
foreach ($params as $param) {
181197
if ($param->byRef) {
182-
return false;
198+
return true;
183199
}
184200
}
185-
186-
return true;
201+
return false;
187202
}
188203

189204
/**
190205
* @param Arg[] $args
191206
*/
192-
private function notUsingNamedArgs(array $args): bool
207+
private function isUsingNamedArgs(array $args): bool
193208
{
194-
Assert::allIsInstanceOf($args, Arg::class);
195-
196209
foreach ($args as $arg) {
197210
if ($arg->name instanceof Identifier) {
198-
return false;
211+
return true;
199212
}
200213
}
214+
return false;
215+
}
216+
217+
private function isChainedCall(FuncCall|MethodCall|StaticCall $callLike): bool
218+
{
219+
if (! $callLike instanceof MethodCall) {
220+
return false;
221+
}
222+
223+
if (! $callLike->var instanceof CallLike) {
224+
return false;
225+
}
201226

202227
return true;
203228
}

0 commit comments

Comments
 (0)