Skip to content

Commit 50f7984

Browse files
authored
Static closure rule ignores arguments with $this binding (#7721)
* working * fixes * fixes * Fix PHPStan issues * wip * fixes * final class * fixes * Additional param check * feedback changes * fix
1 parent 95eeca2 commit 50f7984

File tree

10 files changed

+254
-2
lines changed

10 files changed

+254
-2
lines changed

composer.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,8 @@
9999
"tests/debug_functions.php",
100100
"rules-tests/Transform/Rector/FuncCall/FuncCallToMethodCallRector/Source/some_view_function.php",
101101
"rules-tests/TypeDeclaration/Rector/ClassMethod/ParamTypeByMethodCallTypeRector/Source/FunctionTyped.php",
102-
"rules-tests/Php70/Rector/ClassMethod/Php4ConstructorRector/Source/ParentClass.php"
102+
"rules-tests/Php70/Rector/ClassMethod/Php4ConstructorRector/Source/ParentClass.php",
103+
"rules-tests/CodingStyle/Rector/Closure/StaticClosureRector/Source/functions.php"
103104
]
104105
},
105106
"scripts": {
Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Fixture;
4+
5+
use Closure;
6+
use Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject;
7+
8+
class FixtureWithNamedThisBinding
9+
{
10+
public function first()
11+
{
12+
return BindObject::call(closure: function () {
13+
return 'bar';
14+
}, closureTwo: function () {
15+
return 'foo';
16+
});
17+
}
18+
}
19+
20+
?>
21+
-----
22+
<?php
23+
24+
namespace Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Fixture;
25+
26+
use Closure;
27+
use Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject;
28+
29+
class FixtureWithNamedThisBinding
30+
{
31+
public function first()
32+
{
33+
return BindObject::call(closure: function () {
34+
return 'bar';
35+
}, closureTwo: static function () {
36+
return 'foo';
37+
});
38+
}
39+
}
40+
41+
?>
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Fixture;
4+
5+
use Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject;
6+
use function Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\bind_on_object;
7+
8+
BindObject::call(closure: function () {
9+
echo 'static call';
10+
});
11+
12+
(new BindObject())->callOnObject(function () {
13+
echo 'method call';
14+
});
15+
16+
bind_on_object(function () {
17+
echo 'closure func call 0';
18+
}, null, function () {
19+
echo 'closure func call 0';
20+
});
21+
22+
?>
Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source;
4+
5+
class BindObject
6+
{
7+
/**
8+
* @param-closure-this \Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject $closure
9+
*/
10+
public static function call(\Closure $closure, \Closure|null $closureTwo = null)
11+
{
12+
13+
}
14+
15+
/**
16+
* @param-closure-this \Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject $closure
17+
*/
18+
public function callOnObject(\Closure $closure)
19+
{
20+
21+
}
22+
}
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?php
2+
3+
namespace Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source;
4+
5+
use Closure;
6+
7+
/**
8+
* @param-closure-this \Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject $closure
9+
* @param-closure-this \Rector\Tests\CodingStyle\Rector\Closure\StaticClosureRector\Source\BindObject $closureFinally
10+
*/
11+
function bind_on_object(Closure|null $closure = null, Closure|null $anotherClosure = null, Closure|null $closureFinally = null)
12+
{
13+
14+
}

rules/CodingStyle/Rector/Closure/StaticClosureRector.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
use PhpParser\Node;
88
use PhpParser\Node\Expr\Closure;
99
use Rector\CodingStyle\Guard\StaticGuard;
10+
use Rector\NodeTypeResolver\Node\AttributeKey;
1011
use Rector\Rector\AbstractRector;
1112
use Symplify\RuleDocGenerator\ValueObject\CodeSample\CodeSample;
1213
use Symplify\RuleDocGenerator\ValueObject\RuleDefinition;
@@ -17,7 +18,7 @@
1718
final class StaticClosureRector extends AbstractRector
1819
{
1920
public function __construct(
20-
private readonly StaticGuard $staticGuard
21+
private readonly StaticGuard $staticGuard,
2122
) {
2223
}
2324

@@ -64,6 +65,10 @@ public function getNodeTypes(): array
6465
*/
6566
public function refactor(Node $node): ?Node
6667
{
68+
if ($node->hasAttribute(AttributeKey::IS_CLOSURE_USES_THIS)) {
69+
return null;
70+
}
71+
6772
if (! $this->staticGuard->isLegal($node)) {
6873
return null;
6974
}

src/DependencyInjection/LazyContainerFactory.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@
120120
use Rector\PhpParser\NodeVisitor\AssignedToNodeVisitor;
121121
use Rector\PhpParser\NodeVisitor\ByRefReturnNodeVisitor;
122122
use Rector\PhpParser\NodeVisitor\ByRefVariableNodeVisitor;
123+
use Rector\PhpParser\NodeVisitor\CallLikeThisBoundClosureArgsNodeVisitor;
123124
use Rector\PhpParser\NodeVisitor\ClassConstFetchNodeVisitor;
124125
use Rector\PhpParser\NodeVisitor\ClosureWithVariadicParametersNodeVisitor;
125126
use Rector\PhpParser\NodeVisitor\ContextNodeVisitor;
@@ -256,6 +257,7 @@ final class LazyContainerFactory
256257
PropertyOrClassConstDefaultNodeVisitor::class,
257258
ParamDefaultNodeVisitor::class,
258259
ClassConstFetchNodeVisitor::class,
260+
CallLikeThisBoundClosureArgsNodeVisitor::class,
259261
];
260262

261263
/**
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\NodeAnalyzer;
6+
7+
use PhpParser\Node\Arg;
8+
use PhpParser\Node\Expr\CallLike;
9+
use PhpParser\Node\Expr\Closure;
10+
use PHPStan\Reflection\ExtendedParameterReflection;
11+
use Rector\NodeTypeResolver\Node\AttributeKey;
12+
use Rector\NodeTypeResolver\PHPStan\ParametersAcceptorSelectorVariantsWrapper;
13+
use Rector\Reflection\ReflectionResolver;
14+
15+
final readonly class CallLikeExpectsThisBoundClosureArgsAnalyzer
16+
{
17+
public function __construct(
18+
private ReflectionResolver $reflectionResolver
19+
) {
20+
}
21+
22+
/**
23+
* @return Arg[]
24+
*/
25+
public function getArgsUsingThisBoundClosure(CallLike $callLike): array
26+
{
27+
if ($callLike->isFirstClassCallable() || $callLike->getArgs() === []) {
28+
return [];
29+
}
30+
31+
$callArgs = $callLike->getArgs();
32+
$hasClosureArg = (bool) array_filter($callArgs, fn (Arg $arg): bool => $arg->value instanceof Closure);
33+
34+
if (! $hasClosureArg) {
35+
return [];
36+
}
37+
38+
$argsUsingThisBoundClosure = [];
39+
40+
$reflection = $this->reflectionResolver->resolveFunctionLikeReflectionFromCall($callLike);
41+
42+
if ($reflection === null) {
43+
return [];
44+
}
45+
46+
$scope = $callLike->getAttribute(AttributeKey::SCOPE);
47+
48+
if ($scope === null) {
49+
return [];
50+
}
51+
52+
$parametersAcceptor = ParametersAcceptorSelectorVariantsWrapper::select($reflection, $callLike, $scope);
53+
$parameters = $parametersAcceptor->getParameters();
54+
55+
foreach ($callArgs as $index => $arg) {
56+
if (! $arg->value instanceof Closure) {
57+
continue;
58+
}
59+
60+
if ($arg->name?->name !== null) {
61+
foreach ($parameters as $parameter) {
62+
if (! $parameter instanceof ExtendedParameterReflection) {
63+
continue;
64+
}
65+
66+
$hasObjectBinding = (bool) $parameter->getClosureThisType();
67+
if ($hasObjectBinding && $arg->name->name === $parameter->getName()) {
68+
$argsUsingThisBoundClosure[] = $arg;
69+
}
70+
}
71+
72+
continue;
73+
}
74+
75+
if (! is_string($arg->name?->name)) {
76+
$parameter = $parameters[$index] ?? null;
77+
78+
if (! $parameter instanceof ExtendedParameterReflection) {
79+
continue;
80+
}
81+
82+
$hasObjectBinding = (bool) $parameter->getClosureThisType();
83+
if ($hasObjectBinding) {
84+
$argsUsingThisBoundClosure[] = $arg;
85+
}
86+
}
87+
}
88+
89+
return $argsUsingThisBoundClosure;
90+
}
91+
}

src/NodeTypeResolver/Node/AttributeKey.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,8 @@ final class AttributeKey
274274

275275
public const PHP_VERSION_CONDITIONED = 'php_version_conditioned';
276276

277+
public const IS_CLOSURE_USES_THIS = 'has_this_closure';
278+
277279
public const HAS_CLOSURE_WITH_VARIADIC_ARGS = 'has_closure_with_variadic_args';
278280

279281
public const IS_IN_TRY_BLOCK = 'is_in_try_block';
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Rector\PhpParser\NodeVisitor;
6+
7+
use PhpParser\Node;
8+
use PhpParser\Node\Expr\Closure;
9+
use PhpParser\Node\Expr\FuncCall;
10+
use PhpParser\Node\Expr\MethodCall;
11+
use PhpParser\Node\Expr\StaticCall;
12+
use PhpParser\NodeVisitorAbstract;
13+
use Rector\Contract\PhpParser\DecoratingNodeVisitorInterface;
14+
use Rector\NodeAnalyzer\CallLikeExpectsThisBoundClosureArgsAnalyzer;
15+
use Rector\NodeTypeResolver\Node\AttributeKey;
16+
17+
final class CallLikeThisBoundClosureArgsNodeVisitor extends NodeVisitorAbstract implements DecoratingNodeVisitorInterface
18+
{
19+
public function __construct(
20+
private readonly CallLikeExpectsThisBoundClosureArgsAnalyzer $callLikeExpectsThisBindedClosureArgsAnalyzer
21+
) {
22+
}
23+
24+
public function enterNode(Node $node): ?Node
25+
{
26+
if (
27+
! $node instanceof MethodCall
28+
&& ! $node instanceof StaticCall
29+
&& ! $node instanceof FuncCall
30+
) {
31+
return null;
32+
}
33+
34+
if ($node->isFirstClassCallable()) {
35+
return null;
36+
}
37+
38+
$args = $this->callLikeExpectsThisBindedClosureArgsAnalyzer->getArgsUsingThisBoundClosure($node);
39+
40+
if ($args === []) {
41+
return null;
42+
}
43+
44+
foreach ($args as $arg) {
45+
if ($arg->value instanceof Closure && ! $arg->hasAttribute(AttributeKey::IS_CLOSURE_USES_THIS)) {
46+
$arg->value->setAttribute(AttributeKey::IS_CLOSURE_USES_THIS, true);
47+
}
48+
}
49+
50+
return $node;
51+
}
52+
}

0 commit comments

Comments
 (0)