Skip to content

Commit 924bb4f

Browse files
phpstan-botclaude
authored and
ondrejmirtes
committed
Use ClassMethodsNode method calls in UnusedMethodParametersRule
Instead of re-parsing the file and searching the class AST, the rule now listens to ClassMethodsNode and finds callable references among the method calls ClassStatementsGatherer collected. Errors are still transformed with the scope inside the method, so they stay identical (including trait context). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 parent a24e1f0 commit 924bb4f

3 files changed

Lines changed: 100 additions & 66 deletions

File tree

‎conf/config.level4.neon‎

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

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

‎src/Rules/Methods/UnusedMethodParametersRule.php‎

Lines changed: 97 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -3,49 +3,93 @@
33
namespace PHPStan\Rules\Methods;
44

55
use PhpParser\Node;
6-
use PhpParser\NodeFinder;
6+
use PHPStan\Analyser\RuleErrorTransformer;
77
use PHPStan\Analyser\Scope;
88
use PHPStan\Internal\SprintfHelper;
9-
use PHPStan\Node\VariableWritesNode;
10-
use PHPStan\Parser\Parser;
11-
use PHPStan\Reflection\ClassReflection;
9+
use PHPStan\Node\ClassMethodsNode;
10+
use PHPStan\Node\Method\MethodVariableWrites;
11+
use PHPStan\Rules\IdentifierRuleError;
1212
use PHPStan\Rules\Rule;
13+
use PHPStan\Rules\RuleErrors\TransformedRuleError;
1314
use PHPStan\Rules\UnusedParametersCheck;
1415
use function count;
1516
use function sprintf;
16-
use function str_ends_with;
1717
use function str_starts_with;
18-
use function strlen;
1918
use function strtolower;
2019

2120
/**
2221
* Reports unused parameters of private methods: a private method has no
2322
* callers outside the class, so its signature is not a contract - unlike a
2423
* public or protected one, whose parameters may be dictated by an interface,
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.
24+
* a parent or an override. A private method the class hands out as a callable
25+
* (first-class callable syntax or a callable array) escapes the class, so it
26+
* is skipped as well.
2827
*
29-
* @implements Rule<VariableWritesNode>
28+
* Errors are reported with the scope inside the method, as if the rule
29+
* listened to the method's VariableWritesNode, so a method declared in a
30+
* trait is reported in the trait's file.
31+
*
32+
* @implements Rule<ClassMethodsNode>
3033
*/
3134
final class UnusedMethodParametersRule implements Rule
3235
{
3336

3437
public function __construct(
3538
private UnusedParametersCheck $check,
36-
private Parser $parser,
39+
private RuleErrorTransformer $ruleErrorTransformer,
3740
)
3841
{
3942
}
4043

4144
public function getNodeType(): string
4245
{
43-
return VariableWritesNode::class;
46+
return ClassMethodsNode::class;
4447
}
4548

4649
public function processNode(Node $node, Scope $scope): array
4750
{
48-
$originalNode = $node->getFunctionLike();
51+
$referencedAsCallable = null;
52+
$errors = [];
53+
foreach ($node->getMethodVariableWrites() as $methodVariableWrites) {
54+
$methodErrors = $this->getMethodErrors($methodVariableWrites);
55+
if (count($methodErrors) === 0) {
56+
continue;
57+
}
58+
59+
$referencedAsCallable ??= $this->getMethodsReferencedAsCallable($node);
60+
if ($referencedAsCallable === true) {
61+
return [];
62+
}
63+
64+
$methodNode = $methodVariableWrites->getNode()->getFunctionLike();
65+
if (!$methodNode instanceof Node\Stmt\ClassMethod) {
66+
continue;
67+
}
68+
if (isset($referencedAsCallable[$methodNode->name->toLowerString()])) {
69+
continue;
70+
}
71+
72+
foreach ($methodErrors as $methodError) {
73+
$errors[] = new TransformedRuleError($this->ruleErrorTransformer->transform(
74+
$methodError,
75+
$methodVariableWrites->getScope(),
76+
[],
77+
$methodVariableWrites->getNode(),
78+
));
79+
}
80+
}
81+
82+
return $errors;
83+
}
84+
85+
/**
86+
* @return list<IdentifierRuleError>
87+
*/
88+
private function getMethodErrors(MethodVariableWrites $methodVariableWrites): array
89+
{
90+
$variableWritesNode = $methodVariableWrites->getNode();
91+
$scope = $methodVariableWrites->getScope();
92+
$originalNode = $variableWritesNode->getFunctionLike();
4993
if (!$originalNode instanceof Node\Stmt\ClassMethod) {
5094
return [];
5195
}
@@ -74,8 +118,8 @@ public function processNode(Node $node, Scope $scope): array
74118
SprintfHelper::escapeFormatString($originalNode->name->toString()),
75119
);
76120

77-
$errors = $this->check->getUnusedParameterErrors(
78-
$node,
121+
return $this->check->getUnusedParameterErrors(
122+
$variableWritesNode,
79123
$method,
80124
$originalNode->params,
81125
sprintf('Method %s has an unused parameter $%%s.', $methodDescription),
@@ -84,65 +128,55 @@ public function processNode(Node $node, Scope $scope): array
84128
sprintf('Method %s has a parameter $%%s that only flows into values that are never used.', $methodDescription),
85129
'method.unusedParameterFlow',
86130
);
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;
101131
}
102132

103-
private function isReferencedAsCallable(ClassReflection $classReflection, string $methodName): bool
133+
/**
134+
* Lowercased names of the methods the class references as callables.
135+
* True when a first-class callable's method name cannot be resolved, so
136+
* any method may be referenced.
137+
*
138+
* @return array<string, true>|true
139+
*/
140+
private function getMethodsReferencedAsCallable(ClassMethodsNode $node): array|bool
104141
{
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;
142+
$methodNames = [];
143+
foreach ($node->getMethodCalls() as $methodCall) {
144+
$callNode = $methodCall->getNode();
145+
$callScope = $methodCall->getScope();
146+
if ($callNode instanceof Node\Expr\Array_) {
147+
$arrayType = $callScope->getType($callNode);
148+
if ($arrayType->isCallable()->no()) {
149+
continue;
124150
}
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;
151+
foreach ($arrayType->getConstantArrays() as $constantArray) {
152+
foreach ($constantArray->findTypeAndMethodNames() as $typeAndMethod) {
153+
if ($typeAndMethod->isUnknown()) {
154+
continue;
155+
}
156+
$methodNames[strtolower($typeAndMethod->getMethod())] = true;
157+
}
130158
}
159+
continue;
160+
}
131161

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-
}
162+
if (!$callNode->isFirstClassCallable()) {
163+
continue;
164+
}
165+
if ($callNode->name instanceof Node\Identifier) {
166+
$methodNames[$callNode->name->toLowerString()] = true;
167+
continue;
168+
}
137169

138-
return false;
139-
});
140-
if ($reference !== null) {
170+
$constantStrings = $callScope->getType($callNode->name)->getConstantStrings();
171+
if (count($constantStrings) === 0) {
141172
return true;
142173
}
174+
foreach ($constantStrings as $constantString) {
175+
$methodNames[strtolower($constantString->getValue())] = true;
176+
}
143177
}
144178

145-
return false;
179+
return $methodNames;
146180
}
147181

148182
}

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
namespace PHPStan\Rules\Methods;
44

5+
use PHPStan\Analyser\RuleErrorTransformer;
56
use PHPStan\Rules\Rule;
67
use PHPStan\Rules\UnusedParametersCheck;
78
use PHPStan\Testing\RuleTestCase;
@@ -17,7 +18,7 @@ protected function getRule(): Rule
1718
{
1819
return new UnusedMethodParametersRule(
1920
self::getContainer()->getByType(UnusedParametersCheck::class),
20-
self::getContainer()->getService('defaultAnalysisParser'),
21+
self::getContainer()->getByType(RuleErrorTransformer::class),
2122
);
2223
}
2324

@@ -75,6 +76,7 @@ public function testValueFlow(): void
7576
public function testBug15405(): void
7677
{
7778
$this->analyse([__DIR__ . '/data/bug-15405.php'], [
79+
['Method Bug15405\OtherForms::stringCallable() has an unused parameter $context.', 54],
7880
['Method Bug15405\NotReferenced::notReferenced() has an unused parameter $context.', 67],
7981
['Method Bug15405\NotReferenced::calledDirectly() has an unused parameter $context.', 72],
8082
]);

0 commit comments

Comments
 (0)