Skip to content

Commit 112deba

Browse files
acoultonsamsonasik
andauthored
[Php81] ArrayToFirstClassCallableRector should not skip non-public methods from owning scope (#7760)
* test: Prove ArrayToFirstClassCallableRector skips internal callbacks If a class contains array callbacks with a non-public method, they are not converted into first-class callables. This is due to a fix for a bug that was specific to skipping non-public methods referenced from *outside* the owning class - there was no previous coverage for what should happen when a callable is referenced within a valid scope. * fix: ArrayToFirstClassCallableRector should convert private in own scope If an array callable is explicitly referencing `$this`, then it should always be safe to convert it to a first-class callable even if the method is private / protected. This was the original behaviour of the 8.1 fixers, but it changed in v1.1.1 following a fix for cases where a private / protected method was being referenced from outside the owning scope. * Apply fixture class naming changes from code review Co-authored-by: Abdul Malik Ikhsan <[email protected]> --------- Co-authored-by: Abdul Malik Ikhsan <[email protected]>
1 parent 7d158ab commit 112deba

File tree

2 files changed

+60
-7
lines changed

2 files changed

+60
-7
lines changed
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Rector\Tests\Php81\Rector\Array_\FirstClassCallableRector\Fixture;
4+
5+
final class SomeClassWithOwnPrivate
6+
{
7+
public function run()
8+
{
9+
$name = [$this, 'name'];
10+
}
11+
12+
private function name()
13+
{
14+
}
15+
}
16+
17+
?>
18+
-----
19+
<?php
20+
21+
namespace Rector\Tests\Php81\Rector\Array_\FirstClassCallableRector\Fixture;
22+
23+
final class SomeClassWithOwnPrivate
24+
{
25+
public function run()
26+
{
27+
$name = $this->name(...);
28+
}
29+
30+
private function name()
31+
{
32+
}
33+
}
34+
35+
?>

rules/Php81/Rector/Array_/ArrayToFirstClassCallableRector.php

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ public function name()
7272
}
7373
}
7474
CODE_SAMPLE
75+
,
7576
),
7677
]);
7778
}
@@ -123,7 +124,7 @@ public function refactor(Node $node): StaticCall|MethodCall|null
123124
if ($type instanceof FullyQualifiedObjectType && $this->isNonStaticOtherObject(
124125
$type,
125126
$arrayCallable,
126-
$scope
127+
$scope,
127128
)) {
128129
return null;
129130
}
@@ -133,13 +134,9 @@ public function refactor(Node $node): StaticCall|MethodCall|null
133134

134135
$methodName = $arrayCallable->getMethod();
135136
$methodCall = new MethodCall($callerExpr, $methodName, $args);
136-
$classReflection = $this->reflectionResolver->resolveClassReflectionSourceObject($methodCall);
137137

138-
if ($classReflection instanceof ClassReflection && $classReflection->hasNativeMethod($methodName)) {
139-
$method = $classReflection->getNativeMethod($methodName);
140-
if (! $method->isPublic()) {
141-
return null;
142-
}
138+
if ($this->isReferenceToNonPublicMethodOutsideOwningScope($methodCall, $methodName)) {
139+
return null;
143140
}
144141

145142
return $methodCall;
@@ -174,4 +171,25 @@ private function isNonStaticOtherObject(
174171

175172
return ! $extendedMethodReflection->isPublic();
176173
}
174+
175+
private function isReferenceToNonPublicMethodOutsideOwningScope(MethodCall $methodCall, string $methodName): bool
176+
{
177+
if ($methodCall->var instanceof Variable && $methodCall->var->name === 'this') {
178+
// If the callable is scoped to `$this` then it can be converted even if it is protected / private
179+
return false;
180+
}
181+
182+
// If the callable is scoped to another object / variable then it should only be converted if it is public
183+
// https://github.com/rectorphp/rector/issues/8659
184+
$classReflection = $this->reflectionResolver->resolveClassReflectionSourceObject($methodCall);
185+
186+
if ($classReflection instanceof ClassReflection && $classReflection->hasNativeMethod($methodName)) {
187+
$method = $classReflection->getNativeMethod($methodName);
188+
if (! $method->isPublic()) {
189+
return true;
190+
}
191+
}
192+
193+
return false;
194+
}
177195
}

0 commit comments

Comments
 (0)