Skip to content

Commit 988488c

Browse files
calebdwcalebdw
committed
fix: FunctionLikeToFirstClassCallableRector
1 parent efe86cc commit 988488c

4 files changed

Lines changed: 165 additions & 86 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: 102 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,22 @@
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;
25+
use function array_any;
2126

2227
/**
2328
* @see \Rector\Tests\CodingStyle\Rector\FunctionLike\FunctionLikeToFirstClassCallableRector\FunctionLikeToFirstClassCallableRectorTest
@@ -49,154 +54,165 @@ public function getNodeTypes(): array
4954
/**
5055
* @param ArrowFunction|Closure $node
5156
*/
52-
public function refactor(Node $node): null|StaticCall|MethodCall
57+
public function refactor(Node $node): null|CallLike
5358
{
54-
$extractedMethodCall = $this->extractMethodCallFromFuncLike($node);
59+
$callLike = $this->extractCallLike($node);
5560

56-
if (! $extractedMethodCall instanceof MethodCall && ! $extractedMethodCall instanceof StaticCall) {
61+
if ($callLike === null) {
5762
return null;
5863
}
5964

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

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

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-
}
71+
return $callLike;
72+
}
8073

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

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

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

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

95108
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;
109+
! $callLike instanceof FuncCall
110+
&& ! $callLike instanceof MethodCall
111+
&& ! $callLike instanceof StaticCall
112+
) {
113+
return null;
102114
}
103115

104-
return null;
116+
return $callLike;
105117
}
106118

107119
/**
108-
* @param Node\Param[] $params
109-
* @param Node\Arg[] $args
120+
* @param Param[] $params
121+
* @param Arg[] $args
110122
*/
111-
private function sameParamsForArgs(array $params, array $args): bool
123+
private function isNotUsingSameParamsForArgs(array $params, array $args): bool
112124
{
113-
Assert::allIsInstanceOf($args, Arg::class);
114-
Assert::allIsInstanceOf($params, Param::class);
115-
116125
if (count($args) > count($params)) {
117-
return false;
126+
return true;
118127
}
119128

120129
if (count($args) === 1 && $args[0]->unpack) {
121-
return $params[0]->variadic;
130+
return ! $params[0]->variadic;
122131
}
123132

124133
foreach ($args as $key => $arg) {
125134
if (! $this->nodeComparator->areNodesEqual($arg->value, $params[$key]->var)) {
126-
return false;
135+
return true;
127136
}
128137
}
129138

130-
return true;
139+
return false;
131140
}
132141

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

142151
$found = false;
152+
$parentNode = $expr instanceof MethodCall ? $expr->var : $expr->class;
143153

144154
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-
});
155+
$this->traverseNodesWithCallable($parentNode, function (Node $node) use ($param, &$found) {
156+
if ($this->nodeComparator->areNodesEqual($node, $param->var)) {
157+
$found = true;
158+
return NodeVisitor::STOP_TRAVERSAL;
159+
}
160+
});
161+
162+
if ($found) {
163+
return true;
153164
}
165+
}
166+
167+
return false;
168+
}
154169

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-
}
170+
private function isUsingThisInNonObjectContext(FuncCall|MethodCall|StaticCall $callLike, Scope $scope): bool
171+
{
172+
if (! $callLike instanceof MethodCall) {
173+
return false;
174+
}
160175

161-
return null;
162-
});
163-
}
176+
if (in_array('this', $scope->getDefinedVariables(), true)) {
177+
return false;
178+
}
164179

165-
if ($found) {
166-
return false;
180+
$found = false;
181+
182+
$this->traverseNodesWithCallable($callLike, function (Node $node) use (&$found) {
183+
if ($this->isName($node, 'this')) {
184+
$found = true;
185+
return NodeVisitor::STOP_TRAVERSAL;
167186
}
168-
}
187+
});
169188

170-
return true;
189+
return $found;
171190
}
172191

173192
/**
174193
* @param Param[] $params
175194
*/
176-
private function notUsingByRef(array $params): bool
195+
private function isUsingByRef(array $params): bool
177196
{
178-
Assert::allIsInstanceOf($params, Param::class);
179-
180-
foreach ($params as $param) {
181-
if ($param->byRef) {
182-
return false;
183-
}
184-
}
185-
186-
return true;
197+
return array_any($params, fn (Param $param) => $param->byRef);
187198
}
188199

189200
/**
190201
* @param Arg[] $args
191202
*/
192-
private function notUsingNamedArgs(array $args): bool
203+
private function isUsingNamedArgs(array $args): bool
204+
{
205+
return array_any($args, fn (Arg $arg) => $arg->name instanceof Identifier);
206+
}
207+
208+
private function isChainedCall(FuncCall|MethodCall|StaticCall $callLike): bool
193209
{
194-
Assert::allIsInstanceOf($args, Arg::class);
210+
if (! $callLike instanceof MethodCall) {
211+
return false;
212+
}
195213

196-
foreach ($args as $arg) {
197-
if ($arg->name instanceof Identifier) {
198-
return false;
199-
}
214+
if (! $callLike->var instanceof CallLike) {
215+
return false;
200216
}
201217

202218
return true;

0 commit comments

Comments
 (0)