From 01e271d89239080265b5867a9c87bf6d281797fb Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Sat, 24 Oct 2026 15:35:03 +0000 Subject: [PATCH 1/3] 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. --- conf/config.level4.neon | 2 + .../Methods/UnusedMethodParametersRule.php | 76 +++++++++- .../UnusedMethodParametersRuleTest.php | 15 +- .../PHPStan/Rules/Methods/data/bug-15405.php | 132 ++++++++++++++++++ 4 files changed, 221 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Rules/Methods/data/bug-15405.php diff --git a/conf/config.level4.neon b/conf/config.level4.neon index f437c1e58ff..cf2cf72e83d 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -64,3 +64,5 @@ services: - class: PHPStan\Rules\Methods\UnusedMethodParametersRule + arguments: + parser: @defaultAnalysisParser diff --git a/src/Rules/Methods/UnusedMethodParametersRule.php b/src/Rules/Methods/UnusedMethodParametersRule.php index 4e7bad0ccec..811f09c10ab 100644 --- a/src/Rules/Methods/UnusedMethodParametersRule.php +++ b/src/Rules/Methods/UnusedMethodParametersRule.php @@ -3,27 +3,38 @@ namespace PHPStan\Rules\Methods; use PhpParser\Node; +use PhpParser\NodeFinder; use PHPStan\Analyser\Scope; use PHPStan\Internal\SprintfHelper; use PHPStan\Node\VariableWritesNode; +use PHPStan\Parser\Parser; +use PHPStan\Reflection\ClassReflection; use PHPStan\Rules\Rule; use PHPStan\Rules\UnusedParametersCheck; use function count; use function sprintf; +use function str_ends_with; use function str_starts_with; +use function strlen; +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 referenced as a callable + * (first-class callable syntax, a callable array or a callable string) escapes + * the class, so it is skipped as well. * * @implements Rule */ final class UnusedMethodParametersRule implements Rule { - public function __construct(private UnusedParametersCheck $check) + public function __construct( + private UnusedParametersCheck $check, + private Parser $parser, + ) { } @@ -63,7 +74,7 @@ public function processNode(Node $node, Scope $scope): array SprintfHelper::escapeFormatString($originalNode->name->toString()), ); - return $this->check->getUnusedParameterErrors( + $errors = $this->check->getUnusedParameterErrors( $node, $method, $originalNode->params, @@ -73,6 +84,65 @@ public function processNode(Node $node, Scope $scope): array sprintf('Method %s has a parameter $%%s that only flows into values that are never used.', $methodDescription), 'method.unusedParameterFlow', ); + if (count($errors) === 0) { + return []; + } + + $methodName = $originalNode->name->toString(); + if ($this->isReferencedAsCallable($scope->getClassReflection(), $methodName)) { + return []; + } + $traitReflection = $scope->getTraitReflection(); + if ($traitReflection !== null && $this->isReferencedAsCallable($traitReflection, $methodName)) { + return []; + } + + return $errors; + } + + private function isReferencedAsCallable(ClassReflection $classReflection, string $methodName): bool + { + $fileName = $classReflection->getFileName(); + if ($fileName === null) { + return false; + } + + $startLine = $classReflection->getNativeReflection()->getStartLine(); + $nodeFinder = new NodeFinder(); + $classNodes = $nodeFinder->find( + $this->parser->parseFile($fileName), + static fn (Node $node): bool => $node instanceof Node\Stmt\ClassLike && $node->getStartLine() === $startLine, + ); + + $lowerMethodName = strtolower($methodName); + foreach ($classNodes as $classNode) { + $reference = $nodeFinder->findFirst($classNode, static function (Node $node) use ($lowerMethodName): bool { + if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\StaticCall) { + return $node->isFirstClassCallable() + && $node->name instanceof Node\Identifier + && $node->name->toLowerString() === $lowerMethodName; + } + + if ($node instanceof Node\Expr\Array_) { + return count($node->items) === 2 + && $node->items[1]->value instanceof Node\Scalar\String_ + && strtolower($node->items[1]->value->value) === $lowerMethodName; + } + + if ($node instanceof Node\Scalar\String_) { + $value = strtolower($node->value); + return strlen($value) > strlen($lowerMethodName) + 2 + && str_ends_with($value, '::' . $lowerMethodName); + } + + return false; + }); + if ($reference !== null) { + return true; + } + } + + return false; } } diff --git a/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php b/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php index 1990463fe46..47718070844 100644 --- a/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php +++ b/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php @@ -5,6 +5,7 @@ use PHPStan\Rules\Rule; use PHPStan\Rules\UnusedParametersCheck; use PHPStan\Testing\RuleTestCase; +use PHPUnit\Framework\Attributes\RequiresPhp; /** * @extends RuleTestCase @@ -14,7 +15,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()->getService('defaultAnalysisParser'), + ); } public function testRule(): void @@ -67,4 +71,13 @@ public function testValueFlow(): void ]); } + #[RequiresPhp('>= 8.1.0')] + public function testBug15405(): void + { + $this->analyse([__DIR__ . '/data/bug-15405.php'], [ + ['Method Bug15405\NotReferenced::notReferenced() has an unused parameter $context.', 67], + ['Method Bug15405\NotReferenced::calledDirectly() has an unused parameter $context.', 72], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/bug-15405.php b/tests/PHPStan/Rules/Methods/data/bug-15405.php new file mode 100644 index 00000000000..833cc0c52fe --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-15405.php @@ -0,0 +1,132 @@ += 8.1 + +declare(strict_types = 1); + +namespace Bug15405; + +use Closure; + +final class Operators +{ + /** @return array */ + public static function all(): array + { + return [ + '+' => self::plus(...), + '-' => Closure::fromCallable([self::class, 'minus']), + ]; + } + + private static function plus(string $context, int $a, int $b): int + { + return $a + $b; + } + + private static function minus(string $context, int $a, int $b): int + { + return $a - $b; + } +} + +final class OtherForms +{ + /** @return list */ + public function all(): array + { + return [ + $this->instanceFirstClass(...), + [$this, 'instanceArray'], + 'self::stringCallable', + [$this, 'MIXEDCASE'], + ]; + } + + private function instanceFirstClass(string $context, int $a): int + { + return $a; + } + + private function instanceArray(string $context, int $a): int + { + return $a; + } + + private static function stringCallable(string $context, int $a): int + { + return $a; + } + + private function mixedCase(string $context, int $a): int + { + return $a; + } +} + +final class NotReferenced +{ + private function notReferenced(string $context, int $a): int + { + return $a; + } + + private function calledDirectly(string $context, int $a): int + { + return $a; + } + + public function run(): int + { + return $this->calledDirectly('x', 1); + } +} + +trait OperatorsTrait +{ + private function traitMethod(string $context, int $a): int + { + return $a; + } +} + +final class UsesTrait +{ + use OperatorsTrait; + + /** @return callable(string, int): int */ + public function get(): callable + { + return $this->traitMethod(...); + } +} + +$f = new class { + /** @return callable(string, int): int */ + public function get(): callable + { + return $this->anonymous(...); + } + + private function anonymous(string $context, int $a): int + { + return $a; + } +}; + +trait SelfReferencingTrait +{ + /** @return callable(string, int): int */ + public function get(): callable + { + return $this->fromTrait(...); + } + + private function fromTrait(string $context, int $a): int + { + return $a; + } +} + +final class UsesSelfReferencingTrait +{ + use SelfReferencingTrait; +} From a24e1f099933a76a21429bede3b4e837cb04dbe3 Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 24 Oct 2026 16:21:35 +0000 Subject: [PATCH 2/3] Gather the VariableWritesNode of each method in ClassStatementsGatherer ClassMethodsNode now carries the VariableWritesNode of each method body together with the scope inside the method, so rules listening to it can combine parameter usage with the method calls gathered in the class. Co-Authored-By: Claude Opus 5.5 --- src/Analyser/StmtHandler/ClassLikeHandler.php | 2 +- src/Node/ClassMethodsNode.php | 14 ++- src/Node/ClassStatementsGatherer.php | 16 ++++ src/Node/Method/MethodVariableWrites.php | 33 +++++++ turbo-ext/src/ClassLikeHandler.cpp | 5 +- turbo-ext/src/ClassStatementsGatherer.cpp | 35 ++++--- .../src/generated/ClassStatementsGatherer.h | 93 ++++++++++--------- turbo-ext/src/support.cpp | 1 + turbo-ext/src/support.h | 2 + turbo-ext/tests/class-statements-gatherer.php | 2 +- 10 files changed, 141 insertions(+), 62 deletions(-) create mode 100644 src/Node/Method/MethodVariableWrites.php diff --git a/src/Analyser/StmtHandler/ClassLikeHandler.php b/src/Analyser/StmtHandler/ClassLikeHandler.php index 88ae6f66543..46075c1e70f 100644 --- a/src/Analyser/StmtHandler/ClassLikeHandler.php +++ b/src/Analyser/StmtHandler/ClassLikeHandler.php @@ -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(); diff --git a/src/Node/ClassMethodsNode.php b/src/Node/ClassMethodsNode.php index 77054590a56..ab35527114e 100644 --- a/src/Node/ClassMethodsNode.php +++ b/src/Node/ClassMethodsNode.php @@ -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; @@ -19,8 +20,9 @@ final class ClassMethodsNode extends NodeAbstract implements VirtualNode /** * @param ClassMethod[] $methods * @param array $methodCalls + * @param list $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()); } @@ -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 + */ + public function getMethodVariableWrites(): array + { + return $this->methodVariableWrites; + } + #[Override] public function getType(): string { diff --git a/src/Node/ClassStatementsGatherer.php b/src/Node/ClassStatementsGatherer.php index 71a56858423..fbc317c0a52 100644 --- a/src/Node/ClassStatementsGatherer.php +++ b/src/Node/ClassStatementsGatherer.php @@ -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; @@ -51,6 +52,9 @@ final class ClassStatementsGatherer /** @var \PHPStan\Node\Method\MethodCall[] */ private array $methodCalls = []; + /** @var list */ + private array $methodVariableWrites = []; + /** @var array */ private array $propertyUsages = []; @@ -101,6 +105,14 @@ public function getMethodCalls(): array return $this->methodCalls; } + /** + * @return list + */ + public function getMethodVariableWrites(): array + { + return $this->methodVariableWrites; + } + /** * @return array */ @@ -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 diff --git a/src/Node/Method/MethodVariableWrites.php b/src/Node/Method/MethodVariableWrites.php new file mode 100644 index 00000000000..b32251ce6f2 --- /dev/null +++ b/src/Node/Method/MethodVariableWrites.php @@ -0,0 +1,33 @@ +node; + } + + public function getScope(): Scope + { + return $this->scope; + } + +} diff --git a/turbo-ext/src/ClassLikeHandler.cpp b/turbo-ext/src/ClassLikeHandler.cpp index 555ac1abee6..fc01e614031 100644 --- a/turbo-ext/src/ClassLikeHandler.cpp +++ b/turbo-ext/src/ClassLikeHandler.cpp @@ -374,8 +374,9 @@ class ClassLikeHandler { zv::Val methods = pt_class_statements_gatherer_get(classStatementsGatherer.raw(), PT_CSG_LIST_METHODS); zv::Val methodCalls = pt_class_statements_gatherer_get(classStatementsGatherer.raw(), PT_CSG_LIST_METHOD_CALLS); - zv::Args nodeArgv{stmt, methods.raw(), methodCalls.raw(), classReflection.raw()}; - zv::Val classMethodsNode = pt_type_new(PT_CLASS_CLASS_METHODS_NODE, 4, nodeArgv); + zv::Val methodVariableWrites = pt_class_statements_gatherer_get(classStatementsGatherer.raw(), PT_CSG_LIST_METHOD_VARIABLE_WRITES); + zv::Args nodeArgv{stmt, methods.raw(), methodCalls.raw(), classReflection.raw(), methodVariableWrites.raw()}; + zv::Val classMethodsNode = pt_type_new(PT_CLASS_CLASS_METHODS_NODE, 5, nodeArgv); if (UNEXPECTED(classMethodsNode.isUndef())) return zv::Val(); if (UNEXPECTED(!pt_node_scope_resolver_call_node_callback(nodeScopeResolver, nodeCallback, classMethodsNode.raw(), classScope.raw(), storage))) return zv::Val(); } diff --git a/turbo-ext/src/ClassStatementsGatherer.cpp b/turbo-ext/src/ClassStatementsGatherer.cpp index 2754ebdc2ef..8e5a67f22d5 100644 --- a/turbo-ext/src/ClassStatementsGatherer.cpp +++ b/turbo-ext/src/ClassStatementsGatherer.cpp @@ -6,19 +6,19 @@ * ClassLikeHandler hands one to the walk of a class body as its node * callback: every node of the class reaches __invoke(), which forwards the * pair to the wrapped callback (the rule and collector dispatch, PHP) and - * then gathers the properties, methods, method calls, property usages, - * constants, constant fetches, return statements and property assigns the - * Class*Node emissions after the walk carry. The gathering asks the scope - * through MutatingScope's and ScopeContext's direct entries and the class - * reflections through ClassReflection's, so the scope and reflection calls - * made from here (~2.7M per self-analysis) stay native; anything a subclass - * may override or a PHP twin answers goes by name. The collected value - * objects are the PHP classes, built through the class map; the PHPStan - * node classes read here are final with plain getters, so their slots are - * read directly. + * then gathers the properties, methods, method calls, method variable + * writes, property usages, constants, constant fetches, return statements + * and property assigns the Class*Node emissions after the walk carry. The + * gathering asks the scope through MutatingScope's and ScopeContext's direct + * entries and the class reflections through ClassReflection's, so the scope + * and reflection calls made from here (~2.7M per self-analysis) stay native; + * anything a subclass may override or a PHP twin answers goes by name. The + * collected value objects are the PHP classes, built through the class map; + * the PHPStan node classes read here are final with plain getters, so their + * slots are read directly. * * State is the twin's properties in declaration order: the untyped - * $nodeCallback, the eight collected arrays, then the promoted + * $nodeCallback, the nine collected arrays, then the promoted * $classReflection. */ @@ -91,6 +91,7 @@ static NodeProp pt_csg_param_hooks = PT_NODE_PROP(PT_CLASS_PARAM, "hooks"); static NodeProp pt_csg_param_var = PT_NODE_PROP(PT_CLASS_PARAM, "var"); static NodeProp pt_csg_arg_value = PT_NODE_PROP(PT_CLASS_ARG, "value"); static NodeProp pt_csg_gathered_class_method_node = PT_NODE_PROP(PT_CLASS_GATHERED_CLASS_METHOD, "node"); +static NodeProp pt_csg_variable_writes_function_like = PT_NODE_PROP(PT_CLASS_VARIABLE_WRITES_NODE, "functionLike"); /* Mirrors PHPStan\Node\ClassStatementsGatherer. State lives in the PHP * object's slots. */ @@ -294,6 +295,9 @@ class ClassStatementsGatherer } if (callableOriginal != NULL) return pushNodeScope(slots::methodCalls, PT_CLASS_GATHERED_METHOD_CALL, callableOriginal, scope); if (isInstanceOf(node, PT_CLASS_METHOD_RETURN_STATEMENTS_NODE)) return addReturnStatementsNode(node, &nodeZv); + if (isInstanceOf(node, PT_CLASS_VARIABLE_WRITES_NODE) && pt_csg_variable_writes_function_like.objectOf(node, PT_CLASS_CLASS_METHOD_STMT) != NULL) { + return pushNodeScope(slots::methodVariableWrites, PT_CLASS_GATHERED_METHOD_VARIABLE_WRITES, &nodeZv, scope); + } if (isInstanceOf(node, PT_CLASS_FUNC_CALL)) { zend_object *callee = pt_csg_func_call_name.objectOf(node, PT_CLASS_NAME); zend_string *calleeName = callee != NULL ? visitors::nameString(callee, pt_csg_name_name) : NULL; @@ -596,7 +600,7 @@ zv::Val pt_class_statements_gatherer_new(zval *classReflection, zval *nodeCallba zv::Val pt_class_statements_gatherer_get(zval *gatherer, pt_class_statements_gatherer_list list) { - static const uint32_t listSlots[] = { slots::properties, slots::methods, slots::methodCalls, slots::propertyUsages, slots::constants, slots::constantFetches, slots::returnStatementNodes, slots::propertyAssigns }; + static const uint32_t listSlots[] = { slots::properties, slots::methods, slots::methodCalls, slots::methodVariableWrites, slots::propertyUsages, slots::constants, slots::constantFetches, slots::returnStatementNodes, slots::propertyAssigns }; return zv::Val::copyOf(zv::Ref(OBJ_PROP_NUM(Z_OBJ_P(gatherer), listSlots[list]))); } @@ -608,12 +612,13 @@ PT_MINIT_REGISTRATION(pt_register_class_statements_gatherer) { reg::Class cls("PHPStan\\Node\\ClassStatementsGatherer"); ptdecl::ClassStatementsGatherer::declareClass(cls); - /* the slots PT_CSG_PROP_*: the nine class-body properties first, the + /* the slots PT_CSG_PROP_*: the ten class-body properties first, the * promoted constructor property after them */ cls.privateNullProperty("nodeCallback"); cls.privateTypedArrayPropertyDefaultEmpty("properties"); cls.privateTypedArrayPropertyDefaultEmpty("methods"); cls.privateTypedArrayPropertyDefaultEmpty("methodCalls"); + cls.privateTypedArrayPropertyDefaultEmpty("methodVariableWrites"); cls.privateTypedArrayPropertyDefaultEmpty("propertyUsages"); cls.privateTypedArrayPropertyDefaultEmpty("constants"); cls.privateTypedArrayPropertyDefaultEmpty("constantFetches"); @@ -644,6 +649,10 @@ PT_MINIT_REGISTRATION(pt_register_class_statements_gatherer) ZEND_PARSE_PARAMETERS_NONE(); RETURN_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), slots::methodCalls)); }); + cls.method(sigs::getMethodVariableWrites, [](INTERNAL_FUNCTION_PARAMETERS) { + ZEND_PARSE_PARAMETERS_NONE(); + RETURN_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), slots::methodVariableWrites)); + }); cls.method(sigs::getPropertyUsages, [](INTERNAL_FUNCTION_PARAMETERS) { ZEND_PARSE_PARAMETERS_NONE(); RETURN_COPY(OBJ_PROP_NUM(Z_OBJ_P(ZEND_THIS), slots::propertyUsages)); diff --git a/turbo-ext/src/generated/ClassStatementsGatherer.h b/turbo-ext/src/generated/ClassStatementsGatherer.h index 866b703bc83..55397d5adb7 100644 --- a/turbo-ext/src/generated/ClassStatementsGatherer.h +++ b/turbo-ext/src/generated/ClassStatementsGatherer.h @@ -14,12 +14,13 @@ inline constexpr uint32_t nodeCallback = 0; inline constexpr uint32_t properties = 1; inline constexpr uint32_t methods = 2; inline constexpr uint32_t methodCalls = 3; -inline constexpr uint32_t propertyUsages = 4; -inline constexpr uint32_t constants = 5; -inline constexpr uint32_t constantFetches = 6; -inline constexpr uint32_t returnStatementNodes = 7; -inline constexpr uint32_t propertyAssigns = 8; -inline constexpr uint32_t classReflection = 9; +inline constexpr uint32_t methodVariableWrites = 4; +inline constexpr uint32_t propertyUsages = 5; +inline constexpr uint32_t constants = 6; +inline constexpr uint32_t constantFetches = 7; +inline constexpr uint32_t returnStatementNodes = 8; +inline constexpr uint32_t propertyAssigns = 9; +inline constexpr uint32_t classReflection = 10; } // namespace slot inline void declareClass(reg::Class &cls) @@ -34,6 +35,7 @@ inline void declareProperties(reg::Class &cls) cls.property("properties", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); cls.property("methods", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); cls.property("methodCalls", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); + cls.property("methodVariableWrites", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); cls.property("propertyUsages", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); cls.property("constants", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); cls.property("constantFetches", ZEND_ACC_PRIVATE, reg::PropertyKind::TypedEmptyArray, MAY_BE_ARRAY); @@ -53,49 +55,51 @@ inline constexpr char strings[] = "getProperties\0" /* 77 */ "getMethods\0" /* 91 */ "getMethodCalls\0" /* 102 */ - "getPropertyUsages\0" /* 117 */ - "getConstants\0" /* 135 */ - "getConstantFetches\0" /* 148 */ - "getReturnStatementsNodes\0" /* 167 */ - "getPropertyAssigns\0" /* 192 */ - "node\0" /* 211 */ - "PhpParser\\Node\0" /* 216 */ - "scope\0" /* 231 */ - "PHPStan\\Analyser\\Scope\0" /* 237 */ - "__invoke\0" /* 260 */ - "gatherNodes\0" /* 269 */ - "PhpParser\\Node\\Expr\\FuncCall\0" /* 281 */ - "tryToApplyPropertyReads\0" /* 310 */ - "PhpParser\\Node\\Expr\\Variable\0" /* 334 */ - "tryToApplyPromotedParameterRead\0" /* 363 */ - "ancestorConstructorCall\0" /* 395 */ - "PhpParser\\Node\\Expr\\StaticCall\0" /* 419 */ - "tryToApplyPropertyWritesFromAncestorConstructor"; /* 450 */ + "getMethodVariableWrites\0" /* 117 */ + "getPropertyUsages\0" /* 141 */ + "getConstants\0" /* 159 */ + "getConstantFetches\0" /* 172 */ + "getReturnStatementsNodes\0" /* 191 */ + "getPropertyAssigns\0" /* 216 */ + "node\0" /* 235 */ + "PhpParser\\Node\0" /* 240 */ + "scope\0" /* 255 */ + "PHPStan\\Analyser\\Scope\0" /* 261 */ + "__invoke\0" /* 284 */ + "gatherNodes\0" /* 293 */ + "PhpParser\\Node\\Expr\\FuncCall\0" /* 305 */ + "tryToApplyPropertyReads\0" /* 334 */ + "PhpParser\\Node\\Expr\\Variable\0" /* 358 */ + "tryToApplyPromotedParameterRead\0" /* 387 */ + "ancestorConstructorCall\0" /* 419 */ + "PhpParser\\Node\\Expr\\StaticCall\0" /* 443 */ + "tryToApplyPropertyWritesFromAncestorConstructor"; /* 474 */ inline constexpr reg::PackedArg args[] = { reg::packed(0, 0, 16), /* __construct $classReflection */ reg::packed(51, MAY_BE_CALLABLE), /* __construct $nodeCallback */ reg::packed(76, MAY_BE_ARRAY), /* getProperties return */ reg::packed(76, MAY_BE_ARRAY), /* getMethods return */ reg::packed(76, MAY_BE_ARRAY), /* getMethodCalls return */ + reg::packed(76, MAY_BE_ARRAY), /* getMethodVariableWrites return */ reg::packed(76, MAY_BE_ARRAY), /* getPropertyUsages return */ reg::packed(76, MAY_BE_ARRAY), /* getConstants return */ reg::packed(76, MAY_BE_ARRAY), /* getConstantFetches return */ reg::packed(76, MAY_BE_ARRAY), /* getReturnStatementsNodes return */ reg::packed(76, MAY_BE_ARRAY), /* getPropertyAssigns return */ - reg::packed(211, 0, 216), /* __invoke $node */ - reg::packed(231, 0, 237), /* __invoke $scope */ + reg::packed(235, 0, 240), /* __invoke $node */ + reg::packed(255, 0, 261), /* __invoke $scope */ reg::packed(76, MAY_BE_VOID), /* __invoke return */ - reg::packed(211, 0, 216), /* gatherNodes $node */ - reg::packed(231, 0, 237), /* gatherNodes $scope */ + reg::packed(235, 0, 240), /* gatherNodes $node */ + reg::packed(255, 0, 261), /* gatherNodes $scope */ reg::packed(76, MAY_BE_VOID), /* gatherNodes return */ - reg::packed(211, 0, 281), /* tryToApplyPropertyReads $node */ - reg::packed(231, 0, 237), /* tryToApplyPropertyReads $scope */ + reg::packed(235, 0, 305), /* tryToApplyPropertyReads $node */ + reg::packed(255, 0, 261), /* tryToApplyPropertyReads $scope */ reg::packed(76, MAY_BE_VOID), /* tryToApplyPropertyReads return */ - reg::packed(211, 0, 334), /* tryToApplyPromotedParameterRead $node */ - reg::packed(231, 0, 237), /* tryToApplyPromotedParameterRead $scope */ + reg::packed(235, 0, 358), /* tryToApplyPromotedParameterRead $node */ + reg::packed(255, 0, 261), /* tryToApplyPromotedParameterRead $scope */ reg::packed(76, MAY_BE_VOID), /* tryToApplyPromotedParameterRead return */ - reg::packed(395, 0, 419), /* tryToApplyPropertyWritesFromAncestorConstructor $ancestorConstructorCall */ - reg::packed(231, 0, 237), /* tryToApplyPropertyWritesFromAncestorConstructor $scope */ + reg::packed(419, 0, 443), /* tryToApplyPropertyWritesFromAncestorConstructor $ancestorConstructorCall */ + reg::packed(255, 0, 261), /* tryToApplyPropertyWritesFromAncestorConstructor $scope */ reg::packed(76, MAY_BE_VOID), /* tryToApplyPropertyWritesFromAncestorConstructor return */ }; using Sig = reg::Sig; @@ -107,16 +111,17 @@ inline constexpr sigtab::Sig __construct = { { 64 /* __construct */, 2, 0, 2, re inline constexpr sigtab::Sig getProperties = { { 77 /* getProperties */, 0, 2, 0, 2, ZEND_ACC_PUBLIC } }; inline constexpr sigtab::Sig getMethods = { { 91 /* getMethods */, 0, 3, 0, 3, ZEND_ACC_PUBLIC } }; inline constexpr sigtab::Sig getMethodCalls = { { 102 /* getMethodCalls */, 0, 4, 0, 4, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig getPropertyUsages = { { 117 /* getPropertyUsages */, 0, 5, 0, 5, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig getConstants = { { 135 /* getConstants */, 0, 6, 0, 6, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig getConstantFetches = { { 148 /* getConstantFetches */, 0, 7, 0, 7, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig getReturnStatementsNodes = { { 167 /* getReturnStatementsNodes */, 0, 8, 0, 8, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig getPropertyAssigns = { { 192 /* getPropertyAssigns */, 0, 9, 0, 9, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig __invoke = { { 260 /* __invoke */, 2, 10, 2, 12, ZEND_ACC_PUBLIC } }; -inline constexpr sigtab::Sig gatherNodes = { { 269 /* gatherNodes */, 2, 13, 2, 15, ZEND_ACC_PRIVATE } }; -inline constexpr sigtab::Sig tryToApplyPropertyReads = { { 310 /* tryToApplyPropertyReads */, 2, 16, 2, 18, ZEND_ACC_PRIVATE } }; -inline constexpr sigtab::Sig tryToApplyPromotedParameterRead = { { 363 /* tryToApplyPromotedParameterRead */, 2, 19, 2, 21, ZEND_ACC_PRIVATE } }; -inline constexpr sigtab::Sig tryToApplyPropertyWritesFromAncestorConstructor = { { 450 /* tryToApplyPropertyWritesFromAncestorConstructor */, 2, 22, 2, 24, ZEND_ACC_PRIVATE } }; +inline constexpr sigtab::Sig getMethodVariableWrites = { { 117 /* getMethodVariableWrites */, 0, 5, 0, 5, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig getPropertyUsages = { { 141 /* getPropertyUsages */, 0, 6, 0, 6, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig getConstants = { { 159 /* getConstants */, 0, 7, 0, 7, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig getConstantFetches = { { 172 /* getConstantFetches */, 0, 8, 0, 8, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig getReturnStatementsNodes = { { 191 /* getReturnStatementsNodes */, 0, 9, 0, 9, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig getPropertyAssigns = { { 216 /* getPropertyAssigns */, 0, 10, 0, 10, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig __invoke = { { 284 /* __invoke */, 2, 11, 2, 13, ZEND_ACC_PUBLIC } }; +inline constexpr sigtab::Sig gatherNodes = { { 293 /* gatherNodes */, 2, 14, 2, 16, ZEND_ACC_PRIVATE } }; +inline constexpr sigtab::Sig tryToApplyPropertyReads = { { 334 /* tryToApplyPropertyReads */, 2, 17, 2, 19, ZEND_ACC_PRIVATE } }; +inline constexpr sigtab::Sig tryToApplyPromotedParameterRead = { { 387 /* tryToApplyPromotedParameterRead */, 2, 20, 2, 22, ZEND_ACC_PRIVATE } }; +inline constexpr sigtab::Sig tryToApplyPropertyWritesFromAncestorConstructor = { { 474 /* tryToApplyPropertyWritesFromAncestorConstructor */, 2, 23, 2, 25, ZEND_ACC_PRIVATE } }; } // namespace sig } // namespace ptdecl::ClassStatementsGatherer diff --git a/turbo-ext/src/support.cpp b/turbo-ext/src/support.cpp index 5f9b626eecc..f71eece8469 100644 --- a/turbo-ext/src/support.cpp +++ b/turbo-ext/src/support.cpp @@ -209,6 +209,7 @@ static const pt_class_template pt_class_templates[PT_CLASS_COUNT] = { /* PT_CLASS_INSTANTIATION_CALLABLE_NODE */ {"instantiationCallableNode", "PHPStan\\Node\\InstantiationCallableNode"}, /* PT_CLASS_SET_OFFSET_VALUE_TYPE_EXPR */ {"setOffsetValueTypeExpr", "PHPStan\\Node\\Expr\\SetOffsetValueTypeExpr"}, /* PT_CLASS_GATHERED_METHOD_CALL */ {"gatheredMethodCall", "PHPStan\\Node\\Method\\MethodCall"}, + /* PT_CLASS_GATHERED_METHOD_VARIABLE_WRITES */ {"gatheredMethodVariableWrites", "PHPStan\\Node\\Method\\MethodVariableWrites"}, /* PT_CLASS_PROPERTY_READ */ {"propertyRead", "PHPStan\\Node\\Property\\PropertyRead"}, /* PT_CLASS_PROPERTY_WRITE */ {"propertyWrite", "PHPStan\\Node\\Property\\PropertyWrite"}, /* PT_CLASS_PROPERTY_ASSIGN */ {"propertyAssign", "PHPStan\\Node\\Property\\PropertyAssign"}, diff --git a/turbo-ext/src/support.h b/turbo-ext/src/support.h index 043560faaa0..9e45c6ad77a 100644 --- a/turbo-ext/src/support.h +++ b/turbo-ext/src/support.h @@ -296,6 +296,7 @@ enum { PT_CLASS_INSTANTIATION_CALLABLE_NODE, PT_CLASS_SET_OFFSET_VALUE_TYPE_EXPR, PT_CLASS_GATHERED_METHOD_CALL, + PT_CLASS_GATHERED_METHOD_VARIABLE_WRITES, PT_CLASS_PROPERTY_READ, PT_CLASS_PROPERTY_WRITE, PT_CLASS_PROPERTY_ASSIGN, @@ -2756,6 +2757,7 @@ enum pt_class_statements_gatherer_list PT_CSG_LIST_PROPERTIES, PT_CSG_LIST_METHODS, PT_CSG_LIST_METHOD_CALLS, + PT_CSG_LIST_METHOD_VARIABLE_WRITES, PT_CSG_LIST_PROPERTY_USAGES, PT_CSG_LIST_CONSTANTS, PT_CSG_LIST_CONSTANT_FETCHES, diff --git a/turbo-ext/tests/class-statements-gatherer.php b/turbo-ext/tests/class-statements-gatherer.php index afc7819663e..ebf80768f60 100644 --- a/turbo-ext/tests/class-statements-gatherer.php +++ b/turbo-ext/tests/class-statements-gatherer.php @@ -115,7 +115,7 @@ function check(bool $cond, string $msg): void return [get_class($value), spl_object_id($value)]; }; -$csgGetters = ['getProperties', 'getMethods', 'getMethodCalls', 'getPropertyUsages', 'getConstants', 'getConstantFetches', 'getReturnStatementsNodes', 'getPropertyAssigns']; +$csgGetters = ['getProperties', 'getMethods', 'getMethodCalls', 'getMethodVariableWrites', 'getPropertyUsages', 'getConstants', 'getConstantFetches', 'getReturnStatementsNodes', 'getPropertyAssigns']; $csgRun = static function (string $gathererClass, \PHPStan\Reflection\ClassReflection $classReflection, array $pairs) use ($csgDescribe, $csgGetters): array { $forwarded = []; $gatherer = new $gathererClass($classReflection, static function (\PhpParser\Node $node, \PHPStan\Analyser\Scope $scope) use (&$forwarded): void { From 924bb4f816282f57a85dd523514248c171eaba6a Mon Sep 17 00:00:00 2001 From: phpstan-bot Date: Sat, 24 Oct 2026 16:21:35 +0000 Subject: [PATCH 3/3] 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 --- conf/config.level4.neon | 2 - .../Methods/UnusedMethodParametersRule.php | 160 +++++++++++------- .../UnusedMethodParametersRuleTest.php | 4 +- 3 files changed, 100 insertions(+), 66 deletions(-) diff --git a/conf/config.level4.neon b/conf/config.level4.neon index cf2cf72e83d..f437c1e58ff 100644 --- a/conf/config.level4.neon +++ b/conf/config.level4.neon @@ -64,5 +64,3 @@ services: - class: PHPStan\Rules\Methods\UnusedMethodParametersRule - arguments: - parser: @defaultAnalysisParser diff --git a/src/Rules/Methods/UnusedMethodParametersRule.php b/src/Rules/Methods/UnusedMethodParametersRule.php index 811f09c10ab..f0d35fa5a26 100644 --- a/src/Rules/Methods/UnusedMethodParametersRule.php +++ b/src/Rules/Methods/UnusedMethodParametersRule.php @@ -3,49 +3,93 @@ namespace PHPStan\Rules\Methods; use PhpParser\Node; -use PhpParser\NodeFinder; +use PHPStan\Analyser\RuleErrorTransformer; use PHPStan\Analyser\Scope; use PHPStan\Internal\SprintfHelper; -use PHPStan\Node\VariableWritesNode; -use PHPStan\Parser\Parser; -use PHPStan\Reflection\ClassReflection; +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_ends_with; use function str_starts_with; -use function strlen; 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 private method referenced as a callable - * (first-class callable syntax, a callable array or a callable string) escapes - * the class, so it is skipped as well. + * 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 + * 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 */ final class UnusedMethodParametersRule implements Rule { public function __construct( private UnusedParametersCheck $check, - private Parser $parser, + 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 + */ + private function getMethodErrors(MethodVariableWrites $methodVariableWrites): array + { + $variableWritesNode = $methodVariableWrites->getNode(); + $scope = $methodVariableWrites->getScope(); + $originalNode = $variableWritesNode->getFunctionLike(); if (!$originalNode instanceof Node\Stmt\ClassMethod) { return []; } @@ -74,8 +118,8 @@ public function processNode(Node $node, Scope $scope): array SprintfHelper::escapeFormatString($originalNode->name->toString()), ); - $errors = $this->check->getUnusedParameterErrors( - $node, + return $this->check->getUnusedParameterErrors( + $variableWritesNode, $method, $originalNode->params, sprintf('Method %s has an unused parameter $%%s.', $methodDescription), @@ -84,65 +128,55 @@ public function processNode(Node $node, Scope $scope): array sprintf('Method %s has a parameter $%%s that only flows into values that are never used.', $methodDescription), 'method.unusedParameterFlow', ); - if (count($errors) === 0) { - return []; - } - - $methodName = $originalNode->name->toString(); - if ($this->isReferencedAsCallable($scope->getClassReflection(), $methodName)) { - return []; - } - $traitReflection = $scope->getTraitReflection(); - if ($traitReflection !== null && $this->isReferencedAsCallable($traitReflection, $methodName)) { - return []; - } - - return $errors; } - private function isReferencedAsCallable(ClassReflection $classReflection, string $methodName): bool + /** + * 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|true + */ + private function getMethodsReferencedAsCallable(ClassMethodsNode $node): array|bool { - $fileName = $classReflection->getFileName(); - if ($fileName === null) { - return false; - } - - $startLine = $classReflection->getNativeReflection()->getStartLine(); - $nodeFinder = new NodeFinder(); - $classNodes = $nodeFinder->find( - $this->parser->parseFile($fileName), - static fn (Node $node): bool => $node instanceof Node\Stmt\ClassLike && $node->getStartLine() === $startLine, - ); - - $lowerMethodName = strtolower($methodName); - foreach ($classNodes as $classNode) { - $reference = $nodeFinder->findFirst($classNode, static function (Node $node) use ($lowerMethodName): bool { - if ($node instanceof Node\Expr\MethodCall || $node instanceof Node\Expr\StaticCall) { - return $node->isFirstClassCallable() - && $node->name instanceof Node\Identifier - && $node->name->toLowerString() === $lowerMethodName; + $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()) { + continue; } - - if ($node instanceof Node\Expr\Array_) { - return count($node->items) === 2 - && $node->items[1]->value instanceof Node\Scalar\String_ - && strtolower($node->items[1]->value->value) === $lowerMethodName; + foreach ($arrayType->getConstantArrays() as $constantArray) { + foreach ($constantArray->findTypeAndMethodNames() as $typeAndMethod) { + if ($typeAndMethod->isUnknown()) { + continue; + } + $methodNames[strtolower($typeAndMethod->getMethod())] = true; + } } + continue; + } - if ($node instanceof Node\Scalar\String_) { - $value = strtolower($node->value); - return strlen($value) > strlen($lowerMethodName) + 2 - && str_ends_with($value, '::' . $lowerMethodName); - } + if (!$callNode->isFirstClassCallable()) { + continue; + } + if ($callNode->name instanceof Node\Identifier) { + $methodNames[$callNode->name->toLowerString()] = true; + continue; + } - return false; - }); - if ($reference !== null) { + $constantStrings = $callScope->getType($callNode->name)->getConstantStrings(); + if (count($constantStrings) === 0) { return true; } + foreach ($constantStrings as $constantString) { + $methodNames[strtolower($constantString->getValue())] = true; + } } - return false; + return $methodNames; } } diff --git a/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php b/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php index 47718070844..cd73d969b17 100644 --- a/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php +++ b/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\Methods; +use PHPStan\Analyser\RuleErrorTransformer; use PHPStan\Rules\Rule; use PHPStan\Rules\UnusedParametersCheck; use PHPStan\Testing\RuleTestCase; @@ -17,7 +18,7 @@ protected function getRule(): Rule { return new UnusedMethodParametersRule( self::getContainer()->getByType(UnusedParametersCheck::class), - self::getContainer()->getService('defaultAnalysisParser'), + self::getContainer()->getByType(RuleErrorTransformer::class), ); } @@ -75,6 +76,7 @@ public function testValueFlow(): void 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], ]);