Skip to content

Commit 01e271d

Browse files
phpstan-botondrejmirtes
phpstan-bot
authored and
ondrejmirtes
committed
Skip UnusedMethodParametersRule for private methods referenced as callables within their class
- A private method handed out as a callable is called from outside the class with every argument, so its signature is a contract like a public method's. - Before reporting, the rule scans the declaring class-like AST (via the cached `defaultAnalysisParser`) for a first-class callable (`self::m(...)`, `static::m(...)`, `$this->m(...)`), a callable array (`[self::class, 'm']`, `[$this, 'm']`, also inside `Closure::fromCallable()`) or a callable string (`'self::m'`, `'Foo::m'`). Names match case-insensitively. - For trait methods, both the using class and the trait itself are scanned. Anonymous classes are matched by their start line. - The scan runs only when there would be an error to report, so methods with no unused parameters cost nothing extra. - Probed nullsafe first-class callables: PHP does not allow them, so they are not handled. `UnusedFunctionParametersRule` already reports global functions whatever their visibility, so it was left alone.
1 parent 4ef9ab7 commit 01e271d

4 files changed

Lines changed: 221 additions & 4 deletions

File tree

‎conf/config.level4.neon‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,3 +64,5 @@ services:
6464

6565
-
6666
class: PHPStan\Rules\Methods\UnusedMethodParametersRule
67+
arguments:
68+
parser: @defaultAnalysisParser

‎src/Rules/Methods/UnusedMethodParametersRule.php‎

Lines changed: 73 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,27 +3,38 @@
33
namespace PHPStan\Rules\Methods;
44

55
use PhpParser\Node;
6+
use PhpParser\NodeFinder;
67
use PHPStan\Analyser\Scope;
78
use PHPStan\Internal\SprintfHelper;
89
use PHPStan\Node\VariableWritesNode;
10+
use PHPStan\Parser\Parser;
11+
use PHPStan\Reflection\ClassReflection;
912
use PHPStan\Rules\Rule;
1013
use PHPStan\Rules\UnusedParametersCheck;
1114
use function count;
1215
use function sprintf;
16+
use function str_ends_with;
1317
use function str_starts_with;
18+
use function strlen;
19+
use function strtolower;
1420

1521
/**
1622
* Reports unused parameters of private methods: a private method has no
1723
* callers outside the class, so its signature is not a contract - unlike a
1824
* public or protected one, whose parameters may be dictated by an interface,
19-
* a parent or an override.
25+
* a parent or an override. A private method referenced as a callable
26+
* (first-class callable syntax, a callable array or a callable string) escapes
27+
* the class, so it is skipped as well.
2028
*
2129
* @implements Rule<VariableWritesNode>
2230
*/
2331
final class UnusedMethodParametersRule implements Rule
2432
{
2533

26-
public function __construct(private UnusedParametersCheck $check)
34+
public function __construct(
35+
private UnusedParametersCheck $check,
36+
private Parser $parser,
37+
)
2738
{
2839
}
2940

@@ -63,7 +74,7 @@ public function processNode(Node $node, Scope $scope): array
6374
SprintfHelper::escapeFormatString($originalNode->name->toString()),
6475
);
6576

66-
return $this->check->getUnusedParameterErrors(
77+
$errors = $this->check->getUnusedParameterErrors(
6778
$node,
6879
$method,
6980
$originalNode->params,
@@ -73,6 +84,65 @@ public function processNode(Node $node, Scope $scope): array
7384
sprintf('Method %s has a parameter $%%s that only flows into values that are never used.', $methodDescription),
7485
'method.unusedParameterFlow',
7586
);
87+
if (count($errors) === 0) {
88+
return [];
89+
}
90+
91+
$methodName = $originalNode->name->toString();
92+
if ($this->isReferencedAsCallable($scope->getClassReflection(), $methodName)) {
93+
return [];
94+
}
95+
$traitReflection = $scope->getTraitReflection();
96+
if ($traitReflection !== null && $this->isReferencedAsCallable($traitReflection, $methodName)) {
97+
return [];
98+
}
99+
100+
return $errors;
101+
}
102+
103+
private function isReferencedAsCallable(ClassReflection $classReflection, string $methodName): bool
104+
{
105+
$fileName = $classReflection->getFileName();
106+
if ($fileName === null) {
107+
return false;
108+
}
109+
110+
$startLine = $classReflection->getNativeReflection()->getStartLine();
111+
$nodeFinder = new NodeFinder();
112+
$classNodes = $nodeFinder->find(
113+
$this->parser->parseFile($fileName),
114+
static fn (Node $node): bool => $node instanceof Node\Stmt\ClassLike && $node->getStartLine() === $startLine,
115+
);
116+
117+
$lowerMethodName = strtolower($methodName);
118+
foreach ($classNodes as $classNode) {
119+
$reference = $nodeFinder->findFirst($classNode, static function (Node $node) use ($lowerMethodName): bool {
120+
if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\StaticCall) {
121+
return $node->isFirstClassCallable()
122+
&& $node->name instanceof Node\Identifier
123+
&& $node->name->toLowerString() === $lowerMethodName;
124+
}
125+
126+
if ($node instanceof Node\Expr\Array_) {
127+
return count($node->items) === 2
128+
&& $node->items[1]->value instanceof Node\Scalar\String_
129+
&& strtolower($node->items[1]->value->value) === $lowerMethodName;
130+
}
131+
132+
if ($node instanceof Node\Scalar\String_) {
133+
$value = strtolower($node->value);
134+
return strlen($value) > strlen($lowerMethodName) + 2
135+
&& str_ends_with($value, '::' . $lowerMethodName);
136+
}
137+
138+
return false;
139+
});
140+
if ($reference !== null) {
141+
return true;
142+
}
143+
}
144+
145+
return false;
76146
}
77147

78148
}

‎tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use PHPStan\Rules\Rule;
66
use PHPStan\Rules\UnusedParametersCheck;
77
use PHPStan\Testing\RuleTestCase;
8+
use PHPUnit\Framework\Attributes\RequiresPhp;
89

910
/**
1011
* @extends RuleTestCase<UnusedMethodParametersRule>
@@ -14,7 +15,10 @@ class UnusedMethodParametersRuleTest extends RuleTestCase
1415

1516
protected function getRule(): Rule
1617
{
17-
return new UnusedMethodParametersRule(self::getContainer()->getByType(UnusedParametersCheck::class));
18+
return new UnusedMethodParametersRule(
19+
self::getContainer()->getByType(UnusedParametersCheck::class),
20+
self::getContainer()->getService('defaultAnalysisParser'),
21+
);
1822
}
1923

2024
public function testRule(): void
@@ -67,4 +71,13 @@ public function testValueFlow(): void
6771
]);
6872
}
6973

74+
#[RequiresPhp('>= 8.1.0')]
75+
public function testBug15405(): void
76+
{
77+
$this->analyse([__DIR__ . '/data/bug-15405.php'], [
78+
['Method Bug15405\NotReferenced::notReferenced() has an unused parameter $context.', 67],
79+
['Method Bug15405\NotReferenced::calledDirectly() has an unused parameter $context.', 72],
80+
]);
81+
}
82+
7083
}

‎tests/PHPStan/Rules/Methods/data/bug-15405.php‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,132 @@
1+
<?php // lint >= 8.1
2+
3+
declare(strict_types = 1);
4+
5+
namespace Bug15405;
6+
7+
use Closure;
8+
9+
final class Operators
10+
{
11+
/** @return array<string, Closure(string, int, int): int> */
12+
public static function all(): array
13+
{
14+
return [
15+
'+' => self::plus(...),
16+
'-' => Closure::fromCallable([self::class, 'minus']),
17+
];
18+
}
19+
20+
private static function plus(string $context, int $a, int $b): int
21+
{
22+
return $a + $b;
23+
}
24+
25+
private static function minus(string $context, int $a, int $b): int
26+
{
27+
return $a - $b;
28+
}
29+
}
30+
31+
final class OtherForms
32+
{
33+
/** @return list<callable(string, int): int> */
34+
public function all(): array
35+
{
36+
return [
37+
$this->instanceFirstClass(...),
38+
[$this, 'instanceArray'],
39+
'self::stringCallable',
40+
[$this, 'MIXEDCASE'],
41+
];
42+
}
43+
44+
private function instanceFirstClass(string $context, int $a): int
45+
{
46+
return $a;
47+
}
48+
49+
private function instanceArray(string $context, int $a): int
50+
{
51+
return $a;
52+
}
53+
54+
private static function stringCallable(string $context, int $a): int
55+
{
56+
return $a;
57+
}
58+
59+
private function mixedCase(string $context, int $a): int
60+
{
61+
return $a;
62+
}
63+
}
64+
65+
final class NotReferenced
66+
{
67+
private function notReferenced(string $context, int $a): int
68+
{
69+
return $a;
70+
}
71+
72+
private function calledDirectly(string $context, int $a): int
73+
{
74+
return $a;
75+
}
76+
77+
public function run(): int
78+
{
79+
return $this->calledDirectly('x', 1);
80+
}
81+
}
82+
83+
trait OperatorsTrait
84+
{
85+
private function traitMethod(string $context, int $a): int
86+
{
87+
return $a;
88+
}
89+
}
90+
91+
final class UsesTrait
92+
{
93+
use OperatorsTrait;
94+
95+
/** @return callable(string, int): int */
96+
public function get(): callable
97+
{
98+
return $this->traitMethod(...);
99+
}
100+
}
101+
102+
$f = new class {
103+
/** @return callable(string, int): int */
104+
public function get(): callable
105+
{
106+
return $this->anonymous(...);
107+
}
108+
109+
private function anonymous(string $context, int $a): int
110+
{
111+
return $a;
112+
}
113+
};
114+
115+
trait SelfReferencingTrait
116+
{
117+
/** @return callable(string, int): int */
118+
public function get(): callable
119+
{
120+
return $this->fromTrait(...);
121+
}
122+
123+
private function fromTrait(string $context, int $a): int
124+
{
125+
return $a;
126+
}
127+
}
128+
129+
final class UsesSelfReferencingTrait
130+
{
131+
use SelfReferencingTrait;
132+
}

0 commit comments

Comments
 (0)