Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Analyser/StmtHandler/ClassLikeHandler.php
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ public function processStmt(
// declaration is visited during an enclosing body's observation pass.
$bodyResult = $nodeScopeResolver->processStmtNodesInternal($stmt, $classLikeStatements, $classScope, $storage, $classStatementsGatherer, StatementContext::createTopLevel());
$nodeScopeResolver->callNodeCallback($nodeCallback, new ClassPropertiesNode($stmt, $this->readWritePropertiesExtensions, $classStatementsGatherer->getProperties(), $classStatementsGatherer->getPropertyUsages(), $classStatementsGatherer->getMethodCalls(), $classStatementsGatherer->getReturnStatementsNodes(), $classStatementsGatherer->getPropertyAssigns(), $classReflection), $classScope, $storage);
$nodeScopeResolver->callNodeCallback($nodeCallback, new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection), $classScope, $storage);
$nodeScopeResolver->callNodeCallback($nodeCallback, new ClassMethodsNode($stmt, $classStatementsGatherer->getMethods(), $classStatementsGatherer->getMethodCalls(), $classReflection, $classStatementsGatherer->getMethodVariableWrites()), $classScope, $storage);
$nodeScopeResolver->callNodeCallback($nodeCallback, new ClassConstantsNode($stmt, $classStatementsGatherer->getConstants(), $classStatementsGatherer->getConstantFetches(), $classReflection), $classScope, $storage);
$classReflection->evictPrivateSymbols();
$this->calledMethodProcessor->clearCalledMethodResults();
Expand Down
14 changes: 13 additions & 1 deletion src/Node/ClassMethodsNode.php
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
use PhpParser\Node\Stmt\ClassLike;
use PhpParser\NodeAbstract;
use PHPStan\Node\Method\MethodCall;
use PHPStan\Node\Method\MethodVariableWrites;
use PHPStan\Reflection\ClassReflection;
use PHPStan\Turbo\ReferencedByTurboExtension;

Expand All @@ -19,8 +20,9 @@ final class ClassMethodsNode extends NodeAbstract implements VirtualNode
/**
* @param ClassMethod[] $methods
* @param array<int, MethodCall> $methodCalls
* @param list<MethodVariableWrites> $methodVariableWrites
*/
public function __construct(private ClassLike $class, private array $methods, private array $methodCalls, private ClassReflection $classReflection)
public function __construct(private ClassLike $class, private array $methods, private array $methodCalls, private ClassReflection $classReflection, private array $methodVariableWrites = [])
{
parent::__construct($class->getAttributes());
}
Expand All @@ -46,6 +48,16 @@ public function getMethodCalls(): array
return $this->methodCalls;
}

/**
* The VariableWritesNode of each method body, with the scope inside the method.
*
* @return list<MethodVariableWrites>
*/
public function getMethodVariableWrites(): array
{
return $this->methodVariableWrites;
}

#[Override]
public function getType(): string
{
Expand Down
16 changes: 16 additions & 0 deletions src/Node/ClassStatementsGatherer.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
use PHPStan\Node\Constant\ClassConstantFetch;
use PHPStan\Node\Expr\SetExistingOffsetValueTypeExpr;
use PHPStan\Node\Expr\SetOffsetValueTypeExpr;
use PHPStan\Node\Method\MethodVariableWrites;
use PHPStan\Node\Printer\ExprPrinter;
use PHPStan\Node\Property\PropertyAssign;
use PHPStan\Node\Property\PropertyRead;
Expand Down Expand Up @@ -51,6 +52,9 @@ final class ClassStatementsGatherer
/** @var \PHPStan\Node\Method\MethodCall[] */
private array $methodCalls = [];

/** @var list<MethodVariableWrites> */
private array $methodVariableWrites = [];

/** @var array<int, PropertyWrite|PropertyRead> */
private array $propertyUsages = [];

Expand Down Expand Up @@ -101,6 +105,14 @@ public function getMethodCalls(): array
return $this->methodCalls;
}

/**
* @return list<MethodVariableWrites>
*/
public function getMethodVariableWrites(): array
{
return $this->methodVariableWrites;
}

/**
* @return array<int, PropertyWrite|PropertyRead>
*/
Expand Down Expand Up @@ -191,6 +203,10 @@ private function gatherNodes(Node $node, Scope $scope): void
$this->returnStatementNodes[strtolower($node->getMethodName())] = $node;
return;
}
if ($node instanceof VariableWritesNode && $node->getFunctionLike() instanceof Node\Stmt\ClassMethod) {
$this->methodVariableWrites[] = new MethodVariableWrites($node, $scope);
return;
}
if (
$node instanceof Expr\FuncCall
&& $node->name instanceof Node\Name
Expand Down
33 changes: 33 additions & 0 deletions src/Node/Method/MethodVariableWrites.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?php declare(strict_types = 1);

namespace PHPStan\Node\Method;

use PHPStan\Analyser\Scope;
use PHPStan\Node\VariableWritesNode;
use PHPStan\Turbo\ReferencedByTurboExtension;

/**
* @api
*/
#[ReferencedByTurboExtension(key: 'gatheredMethodVariableWrites')]
final class MethodVariableWrites
{

public function __construct(
private VariableWritesNode $node,
private Scope $scope,
)
{
}

public function getNode(): VariableWritesNode
{
return $this->node;
}

public function getScope(): Scope
{
return $this->scope;
}

}
118 changes: 111 additions & 7 deletions src/Rules/Methods/UnusedMethodParametersRule.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,38 +3,93 @@
namespace PHPStan\Rules\Methods;

use PhpParser\Node;
use PHPStan\Analyser\RuleErrorTransformer;
use PHPStan\Analyser\Scope;
use PHPStan\Internal\SprintfHelper;
use PHPStan\Node\VariableWritesNode;
use PHPStan\Node\ClassMethodsNode;
use PHPStan\Node\Method\MethodVariableWrites;
use PHPStan\Rules\IdentifierRuleError;
use PHPStan\Rules\Rule;
use PHPStan\Rules\RuleErrors\TransformedRuleError;
use PHPStan\Rules\UnusedParametersCheck;
use function count;
use function sprintf;
use function str_starts_with;
use function strtolower;

/**
* Reports unused parameters of private methods: a private method has no
* callers outside the class, so its signature is not a contract - unlike a
* public or protected one, whose parameters may be dictated by an interface,
* a parent or an override.
* a parent or an override. A private method the class hands out as a callable
* (first-class callable syntax or a callable array) escapes the class, so it
* is skipped as well.
*
* @implements Rule<VariableWritesNode>
* Errors are reported with the scope inside the method, as if the rule
* listened to the method's VariableWritesNode, so a method declared in a
* trait is reported in the trait's file.
*
* @implements Rule<ClassMethodsNode>
*/
final class UnusedMethodParametersRule implements Rule
{

public function __construct(private UnusedParametersCheck $check)
public function __construct(
private UnusedParametersCheck $check,
private RuleErrorTransformer $ruleErrorTransformer,
)
{
}

public function getNodeType(): string
{
return VariableWritesNode::class;
return ClassMethodsNode::class;
}

public function processNode(Node $node, Scope $scope): array
{
$originalNode = $node->getFunctionLike();
$referencedAsCallable = null;
$errors = [];
foreach ($node->getMethodVariableWrites() as $methodVariableWrites) {
$methodErrors = $this->getMethodErrors($methodVariableWrites);
if (count($methodErrors) === 0) {
continue;
}

$referencedAsCallable ??= $this->getMethodsReferencedAsCallable($node);
if ($referencedAsCallable === true) {
return [];
}

$methodNode = $methodVariableWrites->getNode()->getFunctionLike();
if (!$methodNode instanceof Node\Stmt\ClassMethod) {
continue;
}
if (isset($referencedAsCallable[$methodNode->name->toLowerString()])) {
continue;
}

foreach ($methodErrors as $methodError) {
$errors[] = new TransformedRuleError($this->ruleErrorTransformer->transform(
$methodError,
$methodVariableWrites->getScope(),
[],
$methodVariableWrites->getNode(),
));
}
}

return $errors;
}

/**
* @return list<IdentifierRuleError>
*/
private function getMethodErrors(MethodVariableWrites $methodVariableWrites): array
{
$variableWritesNode = $methodVariableWrites->getNode();
$scope = $methodVariableWrites->getScope();
$originalNode = $variableWritesNode->getFunctionLike();
if (!$originalNode instanceof Node\Stmt\ClassMethod) {
return [];
}
Expand Down Expand Up @@ -64,7 +119,7 @@
);

return $this->check->getUnusedParameterErrors(
$node,
$variableWritesNode,
$method,
$originalNode->params,
sprintf('Method %s has an unused parameter $%%s.', $methodDescription),
Expand All @@ -75,4 +130,53 @@
);
}

/**
* Lowercased names of the methods the class references as callables.
* True when a first-class callable's method name cannot be resolved, so
* any method may be referenced.
*
* @return array<string, true>|true
*/
private function getMethodsReferencedAsCallable(ClassMethodsNode $node): array|bool
{
$methodNames = [];
foreach ($node->getMethodCalls() as $methodCall) {
$callNode = $methodCall->getNode();
$callScope = $methodCall->getScope();
if ($callNode instanceof Node\Expr\Array_) {
$arrayType = $callScope->getType($callNode);
if ($arrayType->isCallable()->no()) {

Check warning on line 148 in src/Rules/Methods/UnusedMethodParametersRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.4, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $callScope = $methodCall->getScope(); if ($callNode instanceof Node\Expr\Array_) { $arrayType = $callScope->getType($callNode); - if ($arrayType->isCallable()->no()) { + if (!$arrayType->isCallable()->yes()) { continue; } foreach ($arrayType->getConstantArrays() as $constantArray) {

Check warning on line 148 in src/Rules/Methods/UnusedMethodParametersRule.php

View workflow job for this annotation

GitHub Actions / Mutation Testing (8.3, ubuntu-latest)

Escaped Mutant for Mutator "PHPStan\Infection\TrinaryLogicMutator": @@ @@ $callScope = $methodCall->getScope(); if ($callNode instanceof Node\Expr\Array_) { $arrayType = $callScope->getType($callNode); - if ($arrayType->isCallable()->no()) { + if (!$arrayType->isCallable()->yes()) { continue; } foreach ($arrayType->getConstantArrays() as $constantArray) {
continue;
}
foreach ($arrayType->getConstantArrays() as $constantArray) {
foreach ($constantArray->findTypeAndMethodNames() as $typeAndMethod) {
if ($typeAndMethod->isUnknown()) {
continue;
}
$methodNames[strtolower($typeAndMethod->getMethod())] = true;
}
}
continue;
}

if (!$callNode->isFirstClassCallable()) {
continue;
}
if ($callNode->name instanceof Node\Identifier) {
$methodNames[$callNode->name->toLowerString()] = true;
continue;
}

$constantStrings = $callScope->getType($callNode->name)->getConstantStrings();
if (count($constantStrings) === 0) {
return true;
}
foreach ($constantStrings as $constantString) {
$methodNames[strtolower($constantString->getValue())] = true;
}
}

return $methodNames;
}

}
17 changes: 16 additions & 1 deletion tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

namespace PHPStan\Rules\Methods;

use PHPStan\Analyser\RuleErrorTransformer;
use PHPStan\Rules\Rule;
use PHPStan\Rules\UnusedParametersCheck;
use PHPStan\Testing\RuleTestCase;
use PHPUnit\Framework\Attributes\RequiresPhp;

/**
* @extends RuleTestCase<UnusedMethodParametersRule>
Expand All @@ -14,7 +16,10 @@ class UnusedMethodParametersRuleTest extends RuleTestCase

protected function getRule(): Rule
{
return new UnusedMethodParametersRule(self::getContainer()->getByType(UnusedParametersCheck::class));
return new UnusedMethodParametersRule(
self::getContainer()->getByType(UnusedParametersCheck::class),
self::getContainer()->getByType(RuleErrorTransformer::class),
);
}

public function testRule(): void
Expand Down Expand Up @@ -67,4 +72,14 @@ public function testValueFlow(): void
]);
}

#[RequiresPhp('>= 8.1.0')]
public function testBug15405(): void
{
$this->analyse([__DIR__ . '/data/bug-15405.php'], [
['Method Bug15405\OtherForms::stringCallable() has an unused parameter $context.', 54],
['Method Bug15405\NotReferenced::notReferenced() has an unused parameter $context.', 67],
['Method Bug15405\NotReferenced::calledDirectly() has an unused parameter $context.', 72],
]);
}

}
Loading
Loading