From a323b661567de2948e151e92d37a3e1df83ee143 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 11 Sep 2026 11:31:01 +0200 Subject: [PATCH 1/4] Track unused values and array offset assignments A read is no longer a use by itself. A write is used iff its value reaches a sink - a call argument, a condition, a return, echo, throw, a property write - directly or through the writes it is computed into, so `$a = 5; $a = $a + 1;` and Psalm's `$b = $b + 1` loop are reported at every write while `$a = 5; $a = $a + 1; sink($a);` stays quiet. ExpressionContext carries the value-flow target; pure combinators keep it, sinks drop it; VariableHandler emits reads tagged with the target and the liveness resolver turns them into dependency edges resolved as a fixpoint. A write that is read, but only into values that never reach a sink, is reported as " only flows into values that are never used." under the existing identifier plus "Flow" (assign.unusedFlow, foreach.unusedValueFlow, catch.unusedVariableFlow, ...); "is never read" keeps its meaning. Array writes are tracked per offset: items of a literal assigned to a variable are child writes (array.unusedOffset / array.unusedOffsetFlow), `$a['k'] = ...` kills only offset 'k', `$a['k']['j'] = ...` extends it, dynamic offsets are unknown-offset writes, the receiver of an offset access is read as a container. `unset()` discards the reaching writes without reading them unless releasing the value has side effects. Assignments to $this and superglobals are never write sites, and a write to a variable captured by reference is a sink for the values flowing into it while its own liveness follows the capture read. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018CZ79ZBiMt4KWgU3uy4Y4j --- .../ExprHandler/ArrayDimFetchHandler.php | 21 +- src/Analyser/ExprHandler/ArrayHandler.php | 36 +- src/Analyser/ExprHandler/AssignHandler.php | 19 +- src/Analyser/ExprHandler/AssignOpHandler.php | 7 +- src/Analyser/ExprHandler/BinaryOpHandler.php | 4 +- .../ExprHandler/BitwiseNotHandler.php | 2 +- .../ExprHandler/BooleanAndHandler.php | 2 +- .../ExprHandler/BooleanNotHandler.php | 2 +- src/Analyser/ExprHandler/BooleanOrHandler.php | 2 +- src/Analyser/ExprHandler/CastHandler.php | 2 +- .../ExprHandler/CastStringHandler.php | 2 +- src/Analyser/ExprHandler/CoalesceHandler.php | 2 +- .../ExprHandler/InterpolatedStringHandler.php | 2 +- src/Analyser/ExprHandler/MatchHandler.php | 6 +- src/Analyser/ExprHandler/PipeHandler.php | 2 +- src/Analyser/ExprHandler/PostDecHandler.php | 6 +- src/Analyser/ExprHandler/PostIncHandler.php | 6 +- src/Analyser/ExprHandler/PreDecHandler.php | 6 +- src/Analyser/ExprHandler/PreIncHandler.php | 6 +- .../ExprHandler/UnaryMinusHandler.php | 2 +- src/Analyser/ExprHandler/UnaryPlusHandler.php | 2 +- src/Analyser/ExprHandler/VariableHandler.php | 6 +- src/Analyser/ExpressionContext.php | 103 +++- src/Analyser/NodeScopeResolver.php | 2 +- src/Analyser/StmtHandler/UnsetHandler.php | 30 +- src/Analyser/VariableAccessFlow.php | 8 +- src/Analyser/VariableFlow.php | 20 +- src/Analyser/VariableFlowBuilder.php | 51 +- src/Analyser/VariableInputFlow.php | 16 + src/Analyser/VariableLivenessResolver.php | 223 ++++++- src/Analyser/VariableWriteOffset.php | 39 ++ src/Node/Variable/VariableWrite.php | 35 +- src/Node/VariableWritesNode.php | 14 +- src/Rules/DeadCode/UnusedVariableRule.php | 105 ++-- .../Analyser/AnalyserIntegrationTest.php | 21 +- .../Levels/data/arrayDimFetches-4.json | 7 + .../Rules/DeadCode/UnusedVariableRuleTest.php | 456 ++++++++------ .../data/unused-variable-destructor.php | 41 ++ .../unused-variable-flow-messages-catch.php | 29 + .../data/unused-variable-flow-messages.php | 124 ++++ .../data/unused-variable-offset-overwrite.php | 19 + .../DeadCode/data/unused-variable-offsets.php | 573 ++++++++++++++++++ .../DeadCode/data/unused-variable-php8.php | 16 + .../data/unused-variable-value-flow.php | 509 ++++++++++++++++ .../Rules/DeadCode/data/unused-variable.php | 18 +- 45 files changed, 2278 insertions(+), 326 deletions(-) create mode 100644 src/Analyser/VariableInputFlow.php create mode 100644 src/Analyser/VariableWriteOffset.php create mode 100644 tests/PHPStan/Levels/data/arrayDimFetches-4.json create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-destructor.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-flow-messages-catch.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-flow-messages.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-offset-overwrite.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-offsets.php create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php diff --git a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php index b26e3d37819..f5e26d83d89 100644 --- a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php +++ b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php @@ -25,6 +25,7 @@ use PHPStan\Analyser\TypeSpecifierContext; use PHPStan\Analyser\VariableFlow; use PHPStan\Analyser\VariableFlowBuilder; +use PHPStan\Analyser\VariableWriteOffset; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\Expr\TypeExpr; use PHPStan\Reflection\ParametersAcceptorSelector; @@ -34,6 +35,7 @@ use PHPStan\Type\Type; use PHPStan\Type\TypeCombinator; use function array_merge; +use function is_string; /** * @implements ExprHandler @@ -60,13 +62,13 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex { $beforeScope = $scope; if ($expr->dim === null) { - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()->enterArrayDimFetchRoot()); return $this->composeResult($nodeScopeResolver, $stmt, $expr, null, $varResult, $storage, $context, $beforeScope); } - $dimResult = $nodeScopeResolver->processExprNode($stmt, $expr->dim, $scope, $storage, $nodeCallback, $context->enterDeep()); - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $dimResult->getScope(), $storage, $nodeCallback, $context->enterDeep()); + $dimResult = $nodeScopeResolver->processExprNode($stmt, $expr->dim, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $dimResult->getScope(), $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()->enterArrayDimFetchRoot()); return $this->composeResult($nodeScopeResolver, $stmt, $expr, $dimResult, $varResult, $storage, $context, $beforeScope); } @@ -86,7 +88,7 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, $scope, beforeScope: $beforeScope, expr: $expr, - variableFlow: $varResult->getVariableFlow(), + variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), self::offsetRead($expr, null, $context)), hasYield: $varResult->hasYield(), isAlwaysTerminating: $varResult->isAlwaysTerminating(), throwPoints: $varResult->getThrowPoints(), @@ -121,7 +123,7 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, $scope, beforeScope: $beforeScope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $dimResult->getVariableFlow(), VariableFlowBuilder::throws($expr, $throwPoints)), + variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $dimResult->getVariableFlow(), self::offsetRead($expr, $dimResult, $context), VariableFlowBuilder::throws($expr, $throwPoints)), hasYield: $dimResult->hasYield() || $varResult->hasYield(), isAlwaysTerminating: $dimResult->isAlwaysTerminating() || $varResult->isAlwaysTerminating(), throwPoints: $throwPoints, @@ -157,4 +159,13 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, ); } + private static function offsetRead(ArrayDimFetch $expr, ?ExpressionResult $dimResult, ExpressionContext $context): ?VariableFlow + { + if ($context->isUnsetTarget() || !$expr->var instanceof Expr\Variable || !is_string($expr->var->name)) { + return null; + } + $target = $context->getValueFlowTarget(); + return VariableFlow::read($expr->var->name, $target !== null ? $target->getId() : null, offset: $dimResult !== null ? VariableWriteOffset::fromType($dimResult->getType()) : null); + } + } diff --git a/src/Analyser/ExprHandler/ArrayHandler.php b/src/Analyser/ExprHandler/ArrayHandler.php index f02c51c187d..f49935fe6e0 100644 --- a/src/Analyser/ExprHandler/ArrayHandler.php +++ b/src/Analyser/ExprHandler/ArrayHandler.php @@ -18,9 +18,11 @@ use PHPStan\Analyser\SpecifiedTypes; use PHPStan\Analyser\VariableFlow; use PHPStan\Analyser\VariableFlowBuilder; +use PHPStan\Analyser\VariableWriteOffset; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\Node\LiteralArrayItem; use PHPStan\Node\LiteralArrayNode; +use PHPStan\Node\Variable\VariableWrite; use PHPStan\Reflection\InitializerExprTypeResolver; use PHPStan\ShouldNotHappenException; use PHPStan\Type\CallableType; @@ -29,6 +31,8 @@ use function array_key_exists; use function array_merge; use function count; +use function is_int; +use function max; use function spl_object_id; /** @@ -60,11 +64,17 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $throwPoints = []; $impurePoints = []; $isAlwaysTerminating = false; + $literalWrite = $context->isValueFlowDirect() ? $context->getValueFlowTarget() : null; + if ($literalWrite !== null && $literalWrite->isOffsetWrite()) { + $literalWrite = null; + } + $nextIndex = 0; foreach ($expr->items as $arrayItem) { $itemNodes[] = new LiteralArrayItem($scope, $arrayItem); $itemCallbackScope = $scope; + $keyResult = null; if ($arrayItem->key !== null) { - $keyResult = $nodeScopeResolver->processExprNode($stmt, $arrayItem->key, $scope, $storage, $nodeCallback, $context->enterDeep()); + $keyResult = $nodeScopeResolver->processExprNode($stmt, $arrayItem->key, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); $itemResults[spl_object_id($arrayItem->key)] = $keyResult; $variableFlows[] = $keyResult->getVariableFlow(); $hasYield = $hasYield || $keyResult->hasYield(); @@ -74,7 +84,29 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $scope = $keyResult->getScope(); } - $valueResult = $nodeScopeResolver->processExprNode($stmt, $arrayItem->value, $scope, $storage, $nodeCallback, $context->enterDeep()); + $valueContext = $context->enterDeepKeepingValueFlow(); + if ($literalWrite !== null) { + if ($arrayItem->unpack) { + $offset = null; + $nextIndex = null; + } elseif ($keyResult === null) { + $offset = $nextIndex; + if ($nextIndex !== null) { + $nextIndex++; + } + } else { + $offset = VariableWriteOffset::fromType($keyResult->getType()); + if ($offset === null) { + $nextIndex = null; + } elseif (is_int($offset) && $nextIndex !== null) { + $nextIndex = max($nextIndex, $offset + 1); + } + } + $itemWrite = new VariableWrite($literalWrite->getVariableName(), $arrayItem, spl_object_id($arrayItem), VariableWrite::KIND_ARRAY_LITERAL_ITEM, true, $offset, $literalWrite->getId()); + $variableFlows[] = VariableFlow::write($itemWrite); + $valueContext = $context->enterDeep()->enterValueFlow($itemWrite, false); + } + $valueResult = $nodeScopeResolver->processExprNode($stmt, $arrayItem->value, $scope, $storage, $nodeCallback, $valueContext); $itemResults[spl_object_id($arrayItem->value)] = $valueResult; $variableFlows[] = $valueResult->getVariableFlow(); if ($arrayItem->byRef) { diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 60694494526..83d87f9ff4a 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -178,7 +178,9 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex ); } - $assignedExprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $valueScope, $storage, $nodeCallback, $valueContext->enterDeep()); + $valueFlowWrite = $expr instanceof Assign ? VariableFlowBuilder::writeSite($expr->var, VariableWrite::KIND_ASSIGN, $valueScope, $storage) : null; + $valueContext = $valueFlowWrite !== null ? $valueContext->enterDeep()->enterValueFlow($valueFlowWrite, true) : $valueContext->enterDeep(); + $assignedExprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $valueScope, $storage, $nodeCallback, $valueContext); $valueImpurePoints = array_merge($valueImpurePoints, $assignedExprResult->getImpurePoints()); $valueScope = $assignedExprResult->getScope(); @@ -263,6 +265,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $variableFlow = VariableFlow::sequence( VariableFlowBuilder::targetRead($expr->var, $storage, false), $assignedExprResult->getVariableFlow(), + $valueFlowWrite !== null && $context->isValueConsumed() ? VariableFlow::inputs($valueFlowWrite->getId(), $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null) : null, VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_ASSIGN, $scope, $storage, $redundantType), ); if ($expr instanceof Assign && $expr->expr instanceof Expr\Array_ && self::hasArrayReference($expr->expr)) { @@ -601,7 +604,7 @@ private function doPrepareTarget( if (!is_string($var->name)) { // `$$name OP= ...` evaluates the name before reading the old // value: walk it once here, the write flow consumes the result - $variableNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $variableNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $hasYield = $variableNameResult->hasYield(); $throwPoints = $variableNameResult->getThrowPoints(); $impurePoints = $variableNameResult->getImpurePoints(); @@ -838,7 +841,7 @@ private function doPrepareTarget( if ($var instanceof PropertyFetch) { $scopeBeforeVar = $scope; - $objectResult = $nodeScopeResolver->processExprNode($stmt, $var->var, $scope, $storage, $nodeCallback, $context); + $objectResult = $nodeScopeResolver->processExprNode($stmt, $var->var, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $hasYield = $objectResult->hasYield(); $throwPoints = $objectResult->getThrowPoints(); $impurePoints = $objectResult->getImpurePoints(); @@ -850,7 +853,7 @@ private function doPrepareTarget( if ($var->name instanceof Node\Identifier) { $propertyName = $var->name->name; } else { - $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $hasYield = $hasYield || $propertyNameResult->hasYield(); $throwPoints = array_merge($throwPoints, $propertyNameResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $propertyNameResult->getImpurePoints()); @@ -906,7 +909,7 @@ private function doPrepareTarget( if ($var->class instanceof Node\Name) { $propertyHolderType = $scope->resolveTypeByName($var->class); } else { - $classResult = $nodeScopeResolver->processExprNode($stmt, $var->class, $scope, $storage, $nodeCallback, $context); + $classResult = $nodeScopeResolver->processExprNode($stmt, $var->class, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $propertyHolderType = $classResult->getType(); } @@ -915,7 +918,7 @@ private function doPrepareTarget( if ($var->name instanceof Node\Identifier) { $propertyName = $var->name->name; } else { - $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $propertyNameResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $hasYield = $propertyNameResult->hasYield(); $throwPoints = $propertyNameResult->getThrowPoints(); $impurePoints = $propertyNameResult->getImpurePoints(); @@ -1027,7 +1030,7 @@ private function doPrepareTarget( ); } - $varResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context); + $varResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $hasYield = $varResult->hasYield(); $throwPoints = array_merge($throwPoints, $varResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $varResult->getImpurePoints()); @@ -1257,7 +1260,7 @@ public function applyWrite( $nameExprResult = $target->getVariableNameResult(); if ($nameExprResult === null) { // Read-modify-write targets already evaluated the dynamic name in prepareTarget(). - $nameExprResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context); + $nameExprResult = $nodeScopeResolver->processExprNode($stmt, $var->name, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); $hasYield = $hasYield || $nameExprResult->hasYield(); $throwPoints = array_merge($throwPoints, $nameExprResult->getThrowPoints()); $impurePoints = array_merge($impurePoints, $nameExprResult->getImpurePoints()); diff --git a/src/Analyser/ExprHandler/AssignOpHandler.php b/src/Analyser/ExprHandler/AssignOpHandler.php index e587f919568..7d3e732c673 100644 --- a/src/Analyser/ExprHandler/AssignOpHandler.php +++ b/src/Analyser/ExprHandler/AssignOpHandler.php @@ -100,7 +100,9 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } } - $valueResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $valueScope, $storage, $nodeCallback, $valueContext->enterDeep()); + $valueFlowWrite = VariableFlowBuilder::writeSite($expr->var, VariableWrite::KIND_READ_MODIFY_WRITE, $valueScope, $storage); + $valueContext = $valueFlowWrite !== null ? $valueContext->enterDeep()->enterValueFlow($valueFlowWrite, false) : $valueContext->enterDeep(); + $valueResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $valueScope, $storage, $nodeCallback, $valueContext); $rhsResult = $valueResult; if ($expr instanceof Expr\AssignOp\Coalesce) { $rightResult = $valueResult; @@ -285,10 +287,11 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $writeFlow = VariableFlow::sequence( $rhsResult->getVariableFlow(), + $valueFlowWrite !== null && $context->isValueConsumed() ? VariableFlow::inputs($valueFlowWrite->getId(), $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null) : null, VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_READ_MODIFY_WRITE, $scope, $storage), ); $variableFlow = VariableFlow::sequence( - VariableFlowBuilder::targetRead($expr->var, $storage, true), + VariableFlowBuilder::targetRead($expr->var, $storage, true, !($expr instanceof Expr\AssignOp\Coalesce) && $valueFlowWrite !== null ? $valueFlowWrite->getId() : null), $expr instanceof Expr\AssignOp\Coalesce ? VariableFlow::choice($writeFlow, null) : $writeFlow, ); diff --git a/src/Analyser/ExprHandler/BinaryOpHandler.php b/src/Analyser/ExprHandler/BinaryOpHandler.php index 1884ce0e3a1..3312bc30828 100644 --- a/src/Analyser/ExprHandler/BinaryOpHandler.php +++ b/src/Analyser/ExprHandler/BinaryOpHandler.php @@ -91,8 +91,8 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; - $leftResult = $nodeScopeResolver->processExprNode($stmt, $expr->left, $scope, $storage, $nodeCallback, $context->enterDeep()); - $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $leftResult->getScope(), $storage, $nodeCallback, $context->enterDeep()); + $leftResult = $nodeScopeResolver->processExprNode($stmt, $expr->left, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); + $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $leftResult->getScope(), $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); $throwPoints = array_merge($leftResult->getThrowPoints(), $rightResult->getThrowPoints()); $impurePoints = array_merge($leftResult->getImpurePoints(), $rightResult->getImpurePoints()); if ( diff --git a/src/Analyser/ExprHandler/BitwiseNotHandler.php b/src/Analyser/ExprHandler/BitwiseNotHandler.php index e8bae880b9f..50870082312 100644 --- a/src/Analyser/ExprHandler/BitwiseNotHandler.php +++ b/src/Analyser/ExprHandler/BitwiseNotHandler.php @@ -41,7 +41,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); return $this->expressionResultFactory->create( $exprResult->getScope(), diff --git a/src/Analyser/ExprHandler/BooleanAndHandler.php b/src/Analyser/ExprHandler/BooleanAndHandler.php index cf828e121e5..73d476d940e 100644 --- a/src/Analyser/ExprHandler/BooleanAndHandler.php +++ b/src/Analyser/ExprHandler/BooleanAndHandler.php @@ -48,7 +48,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex { $leftResult = $nodeScopeResolver->processExprNode($stmt, $expr->left, $scope, $storage, $nodeCallback, $context->enterDeep()); $leftTruthyScope = $leftResult->getTruthyScope(); - $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $leftTruthyScope, $storage, $nodeCallback, $context); + $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $leftTruthyScope, $storage, $nodeCallback, $context->withoutValueFlow()); $rightExprType = $rightResult->getType(); if ($rightExprType instanceof NeverType && $rightExprType->isExplicit()) { $leftMergedWithRightScope = $leftResult->getFalseyScope()->addTemplateArgumentConstraints($rightResult->getScope()->getTemplateArgumentConstraints()); diff --git a/src/Analyser/ExprHandler/BooleanNotHandler.php b/src/Analyser/ExprHandler/BooleanNotHandler.php index 6709df2595c..730f39d9316 100644 --- a/src/Analyser/ExprHandler/BooleanNotHandler.php +++ b/src/Analyser/ExprHandler/BooleanNotHandler.php @@ -42,7 +42,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); $scope = $exprResult->getScope(); return $this->expressionResultFactory->create( diff --git a/src/Analyser/ExprHandler/BooleanOrHandler.php b/src/Analyser/ExprHandler/BooleanOrHandler.php index 185d32d54b7..c648430b0e7 100644 --- a/src/Analyser/ExprHandler/BooleanOrHandler.php +++ b/src/Analyser/ExprHandler/BooleanOrHandler.php @@ -66,7 +66,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex { $leftResult = $nodeScopeResolver->processExprNode($stmt, $expr->left, $scope, $storage, $nodeCallback, $context->enterDeep()); $leftFalseyScope = $leftResult->getFalseyScope(); - $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $leftFalseyScope, $storage, $nodeCallback, $context); + $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $leftFalseyScope, $storage, $nodeCallback, $context->withoutValueFlow()); $rightExprType = $rightResult->getType(); if ($rightExprType instanceof NeverType && $rightExprType->isExplicit()) { $leftMergedWithRightScope = $leftResult->getTruthyScope()->addTemplateArgumentConstraints($rightResult->getScope()->getTemplateArgumentConstraints()); diff --git a/src/Analyser/ExprHandler/CastHandler.php b/src/Analyser/ExprHandler/CastHandler.php index af5242b29ba..fe4a9604197 100644 --- a/src/Analyser/ExprHandler/CastHandler.php +++ b/src/Analyser/ExprHandler/CastHandler.php @@ -52,7 +52,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); $scope = $exprResult->getScope(); $subjectArgResult = $this->identicalNarrowingHelper->captureFirstArgResult($expr->expr, $storage); diff --git a/src/Analyser/ExprHandler/CastStringHandler.php b/src/Analyser/ExprHandler/CastStringHandler.php index 9521ca13486..392b10b984f 100644 --- a/src/Analyser/ExprHandler/CastStringHandler.php +++ b/src/Analyser/ExprHandler/CastStringHandler.php @@ -48,7 +48,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { $beforeScope = $scope; - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); $impurePoints = $exprResult->getImpurePoints(); $throwPoints = $exprResult->getThrowPoints(); diff --git a/src/Analyser/ExprHandler/CoalesceHandler.php b/src/Analyser/ExprHandler/CoalesceHandler.php index c3680a3f4f3..798ffeb4a42 100644 --- a/src/Analyser/ExprHandler/CoalesceHandler.php +++ b/src/Analyser/ExprHandler/CoalesceHandler.php @@ -59,7 +59,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex // the falsey narrowing of this very node - asking the scope about it // mid-processing would take the on-demand path and recurse $rightScope = $scope->applySpecifiedTypes($this->coalesceCompositionHelper->getFalseySpecifiedTypes($scope, $scope, $expr->left, $condResult, $expr, TypeSpecifierContext::createFalsey())); - $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $rightScope, $storage, $nodeCallback, $context->enterDeep()); + $rightResult = $nodeScopeResolver->processExprNode($stmt, $expr->right, $rightScope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); // the left-is-set narrowing, composed from the already-processed chain // results - the inside-out equivalent of narrowing by isset($expr->left) // without synthesizing an Isset_ node and re-walking the chain on demand diff --git a/src/Analyser/ExprHandler/InterpolatedStringHandler.php b/src/Analyser/ExprHandler/InterpolatedStringHandler.php index 3e937b56591..6a155f322da 100644 --- a/src/Analyser/ExprHandler/InterpolatedStringHandler.php +++ b/src/Analyser/ExprHandler/InterpolatedStringHandler.php @@ -59,7 +59,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if (!$part instanceof Expr) { continue; } - $partResult = $nodeScopeResolver->processExprNode($stmt, $part, $scope, $storage, $nodeCallback, $context->enterDeep()); + $partResult = $nodeScopeResolver->processExprNode($stmt, $part, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); $variableFlows[] = $partResult->getVariableFlow(); $partResults[spl_object_id($part)] = $partResult; $hasYield = $hasYield || $partResult->hasYield(); diff --git a/src/Analyser/ExprHandler/MatchHandler.php b/src/Analyser/ExprHandler/MatchHandler.php index 0239e02e610..eb6cbc6cd86 100644 --- a/src/Analyser/ExprHandler/MatchHandler.php +++ b/src/Analyser/ExprHandler/MatchHandler.php @@ -287,7 +287,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $matchArmBodyScope, $storage, $nodeCallback, - ExpressionContext::createTopLevel($context->shouldResolveTemplateArguments()), + $context->enterMatchArm(), ); $armFlows[$i] = $armResult->getVariableFlow(); $armScope = $armResult->getScope(); @@ -328,7 +328,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $defaultArmBodyScope = $matchScope; $matchArmBody = new MatchExpressionArmBody($matchScope, $arm->body); $armNodes[$i] = new MatchExpressionArm($matchArmBody, [], $arm->getStartLine()); - $armResult = $nodeScopeResolver->processExprNode($stmt, $arm->body, $matchScope, $storage, $nodeCallback, ExpressionContext::createTopLevel($context->shouldResolveTemplateArguments())); + $armResult = $nodeScopeResolver->processExprNode($stmt, $arm->body, $matchScope, $storage, $nodeCallback, $context->enterMatchArm()); $armFlows[$i] = $armResult->getVariableFlow(); $matchScope = $armResult->getScope(); $scope = $scope->addTemplateArgumentConstraints($matchScope->getTemplateArgumentConstraints()); @@ -437,7 +437,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $bodyScope, $storage, $nodeCallback, - ExpressionContext::createTopLevel($context->shouldResolveTemplateArguments()), + $context->enterMatchArm(), ); $armFlows[$i] = $armResult->getVariableFlow(); $armScope = $armResult->getScope(); diff --git a/src/Analyser/ExprHandler/PipeHandler.php b/src/Analyser/ExprHandler/PipeHandler.php index a096541a3df..ed813672588 100644 --- a/src/Analyser/ExprHandler/PipeHandler.php +++ b/src/Analyser/ExprHandler/PipeHandler.php @@ -97,7 +97,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex )); } - $callResult = $nodeScopeResolver->processExprNode($stmt, $callExpr, $scope, $storage, $nodeCallback, $context); + $callResult = $nodeScopeResolver->processExprNode($stmt, $callExpr, $scope, $storage, $nodeCallback, $context->withoutValueFlow()); return $this->expressionResultFactory->create( $callResult->getScope(), diff --git a/src/Analyser/ExprHandler/PostDecHandler.php b/src/Analyser/ExprHandler/PostDecHandler.php index e323b360688..6c97d45b54c 100644 --- a/src/Analyser/ExprHandler/PostDecHandler.php +++ b/src/Analyser/ExprHandler/PostDecHandler.php @@ -45,7 +45,9 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $valueFlowWrite = VariableFlowBuilder::writeSite($expr->var, VariableWrite::KIND_POST_DEC, $scope, $storage); + $valueContext = $valueFlowWrite !== null ? $context->enterDeep()->enterValueFlow($valueFlowWrite, false) : $context->enterDeep(); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $valueContext); // the virtual assign writes the decremented value - hand it the synthetic's // result so applyWrite composes off it instead of pricing the @@ -82,7 +84,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $assignedScope, beforeScope: $scope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_POST_DEC, $assignedScope, $storage)), + variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $valueFlowWrite !== null && $context->isValueConsumed() ? VariableFlow::inputs($valueFlowWrite->getId(), $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null) : null, VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_POST_DEC, $assignedScope, $storage)), hasYield: $varResult->hasYield(), isAlwaysTerminating: $varResult->isAlwaysTerminating(), throwPoints: $varResult->getThrowPoints(), diff --git a/src/Analyser/ExprHandler/PostIncHandler.php b/src/Analyser/ExprHandler/PostIncHandler.php index 257f52dc87f..83368839776 100644 --- a/src/Analyser/ExprHandler/PostIncHandler.php +++ b/src/Analyser/ExprHandler/PostIncHandler.php @@ -45,7 +45,9 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $valueFlowWrite = VariableFlowBuilder::writeSite($expr->var, VariableWrite::KIND_POST_INC, $scope, $storage); + $valueContext = $valueFlowWrite !== null ? $context->enterDeep()->enterValueFlow($valueFlowWrite, false) : $context->enterDeep(); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $valueContext); // the virtual assign writes the incremented value - hand it the synthetic's // result so applyWrite composes off it instead of pricing the @@ -82,7 +84,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $assignedScope, beforeScope: $scope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_POST_INC, $assignedScope, $storage)), + variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $valueFlowWrite !== null && $context->isValueConsumed() ? VariableFlow::inputs($valueFlowWrite->getId(), $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null) : null, VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_POST_INC, $assignedScope, $storage)), hasYield: $varResult->hasYield(), isAlwaysTerminating: $varResult->isAlwaysTerminating(), throwPoints: $varResult->getThrowPoints(), diff --git a/src/Analyser/ExprHandler/PreDecHandler.php b/src/Analyser/ExprHandler/PreDecHandler.php index 10d39d39c61..c646ea565cc 100644 --- a/src/Analyser/ExprHandler/PreDecHandler.php +++ b/src/Analyser/ExprHandler/PreDecHandler.php @@ -43,7 +43,9 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $valueFlowWrite = VariableFlowBuilder::writeSite($expr->var, VariableWrite::KIND_PRE_DEC, $scope, $storage); + $valueContext = $valueFlowWrite !== null ? $context->enterDeep()->enterValueFlow($valueFlowWrite, false) : $context->enterDeep(); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $valueContext); $typeCallback = $this->incDecTypeHelper->getTypeCallback($expr->var, $varResult, false); $specifyTypesCallback = fn (TypeSpecifierContext $context, bool $nativeTypesPromoted): SpecifiedTypes => $this->defaultNarrowingHelper->specifyDefaultTypes($expr, $context); @@ -84,7 +86,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $assignedScope, beforeScope: $scope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_PRE_DEC, $assignedScope, $storage)), + variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $valueFlowWrite !== null && $context->isValueConsumed() ? VariableFlow::inputs($valueFlowWrite->getId(), $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null) : null, VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_PRE_DEC, $assignedScope, $storage)), hasYield: $varResult->hasYield(), isAlwaysTerminating: $varResult->isAlwaysTerminating(), throwPoints: $varResult->getThrowPoints(), diff --git a/src/Analyser/ExprHandler/PreIncHandler.php b/src/Analyser/ExprHandler/PreIncHandler.php index beba3f0b134..ba7b208166d 100644 --- a/src/Analyser/ExprHandler/PreIncHandler.php +++ b/src/Analyser/ExprHandler/PreIncHandler.php @@ -43,7 +43,9 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $context->enterDeep()); + $valueFlowWrite = VariableFlowBuilder::writeSite($expr->var, VariableWrite::KIND_PRE_INC, $scope, $storage); + $valueContext = $valueFlowWrite !== null ? $context->enterDeep()->enterValueFlow($valueFlowWrite, false) : $context->enterDeep(); + $varResult = $nodeScopeResolver->processExprNode($stmt, $expr->var, $scope, $storage, $nodeCallback, $valueContext); $typeCallback = $this->incDecTypeHelper->getTypeCallback($expr->var, $varResult, true); $specifyTypesCallback = fn (TypeSpecifierContext $context, bool $nativeTypesPromoted): SpecifiedTypes => $this->defaultNarrowingHelper->specifyDefaultTypes($expr, $context); @@ -84,7 +86,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $assignedScope, beforeScope: $scope, expr: $expr, - variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_PRE_INC, $assignedScope, $storage)), + variableFlow: VariableFlow::sequence($varResult->getVariableFlow(), $valueFlowWrite !== null && $context->isValueConsumed() ? VariableFlow::inputs($valueFlowWrite->getId(), $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null) : null, VariableFlowBuilder::targetWrite($expr->var, VariableWrite::KIND_PRE_INC, $assignedScope, $storage)), hasYield: $varResult->hasYield(), isAlwaysTerminating: $varResult->isAlwaysTerminating(), throwPoints: $varResult->getThrowPoints(), diff --git a/src/Analyser/ExprHandler/UnaryMinusHandler.php b/src/Analyser/ExprHandler/UnaryMinusHandler.php index 29c0218a4f2..743e215c29b 100644 --- a/src/Analyser/ExprHandler/UnaryMinusHandler.php +++ b/src/Analyser/ExprHandler/UnaryMinusHandler.php @@ -40,7 +40,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); return $this->expressionResultFactory->create( $exprResult->getScope(), diff --git a/src/Analyser/ExprHandler/UnaryPlusHandler.php b/src/Analyser/ExprHandler/UnaryPlusHandler.php index 1cbca23fd92..254d6303cb8 100644 --- a/src/Analyser/ExprHandler/UnaryPlusHandler.php +++ b/src/Analyser/ExprHandler/UnaryPlusHandler.php @@ -41,7 +41,7 @@ public function supports(Expr $expr): bool public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Expr $expr, MutatingScope $scope, ExpressionResultStorage $storage, callable $nodeCallback, ExpressionContext $context): ExpressionResult { - $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeep()); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $expr->expr, $scope, $storage, $nodeCallback, $context->enterDeepKeepingValueFlow()); return $this->expressionResultFactory->create( $exprResult->getScope(), diff --git a/src/Analyser/ExprHandler/VariableHandler.php b/src/Analyser/ExprHandler/VariableHandler.php index 8622c66c1b0..4d41b7db60d 100644 --- a/src/Analyser/ExprHandler/VariableHandler.php +++ b/src/Analyser/ExprHandler/VariableHandler.php @@ -132,7 +132,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex $nameResult = $nodeScopeResolver->processExprNode($stmt, $expr->name, $scope, $storage, $nodeCallback, $context->enterDeep()); } - return $this->composeResult($nodeScopeResolver, $expr, $nameResult, $storage, $beforeScope); + return $this->composeResult($nodeScopeResolver, $expr, $nameResult, $storage, $beforeScope, $context); } /** @@ -141,7 +141,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex * walking a dynamic name; AssignHandler::prepareTarget() calls it to price a * read-modify-write target without re-walking it. */ - public function composeResult(NodeScopeResolver $nodeScopeResolver, Variable $expr, ?ExpressionResult $nameResult, ExpressionResultStorage $storage, MutatingScope $beforeScope): ExpressionResult + public function composeResult(NodeScopeResolver $nodeScopeResolver, Variable $expr, ?ExpressionResult $nameResult, ExpressionResultStorage $storage, MutatingScope $beforeScope, ?ExpressionContext $context = null): ExpressionResult { $scope = $beforeScope; $hasYield = false; @@ -150,7 +150,7 @@ public function composeResult(NodeScopeResolver $nodeScopeResolver, Variable $ex $isAlwaysTerminating = false; $variableFlow = null; if (is_string($expr->name)) { - $variableFlow = VariableFlow::read($expr->name); + $variableFlow = ($context !== null && $context->isUnsetTarget() ? VariableFlow::mention($expr->name) : VariableFlow::read($expr->name, $context !== null && $context->getValueFlowTarget() !== null ? $context->getValueFlowTarget()->getId() : null, $context !== null && $context->isArrayDimFetchRoot())); if (in_array($expr->name, Scope::SUPERGLOBAL_VARIABLES, true)) { $impurePoints[] = new ImpurePoint($scope, $expr, 'superglobal', 'access to superglobal variable', true); } diff --git a/src/Analyser/ExpressionContext.php b/src/Analyser/ExpressionContext.php index 6955241a9a5..da509bccfab 100644 --- a/src/Analyser/ExpressionContext.php +++ b/src/Analyser/ExpressionContext.php @@ -3,6 +3,7 @@ namespace PHPStan\Analyser; use PhpParser\Node\Expr; +use PHPStan\Node\Variable\VariableWrite; use PHPStan\Reflection\ExtendedParametersAcceptor; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Type\Generic\TemplateTypeHelper; @@ -19,6 +20,11 @@ private function __construct( private ?Type $inAssignRightSideType = null, private ?Type $inAssignRightSideNativeType = null, private bool $resolveTemplateArguments = true, + private ?VariableWrite $valueFlowTarget = null, + private bool $valueFlowDirect = false, + private bool $arrayDimFetchRoot = false, + private bool $unsetTarget = false, + private ?bool $valueConsumed = null, ) { } @@ -33,15 +39,59 @@ public static function createDeep(bool $resolveTemplateArguments = true): self return new self(isDeep: true, inAssignRightSideVariableName: null, inAssignRightSideExpr: null, resolveTemplateArguments: $resolveTemplateArguments); } + /** + * The context of a sub-expression whose value does not flow into the + * enclosing expression's value (a call argument, a condition, a receiver): + * a value-flow target and the read flavours are dropped. + */ public function enterDeep(): self { - if ($this->isDeep) { + if ($this->isDeep && $this->valueFlowTarget === null && !$this->arrayDimFetchRoot && !$this->unsetTarget) { return $this; } return new self(true, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, $this->resolveTemplateArguments); } + /** + * The context of an operand of a pure combinator (arithmetic, concat, a + * cast, a literal array item...): its value flows into the enclosing + * expression's value, so the value-flow target is kept. + */ + public function enterDeepKeepingValueFlow(): self + { + if ($this->valueFlowTarget === null) { + return $this->enterDeep(); + } + + return new self(true, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, $this->resolveTemplateArguments, valueFlowTarget: $this->valueFlowTarget, valueFlowDirect: false); + } + + /** + * The context of a sub-expression at the same depth whose value does not + * flow into the enclosing expression's value (the right operand of && / ||, + * a piped call, a closure use, the receiver of an assignment target). + */ + public function withoutValueFlow(): self + { + if ($this->valueFlowTarget === null && !$this->arrayDimFetchRoot && !$this->unsetTarget) { + return $this; + } + + return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, $this->resolveTemplateArguments); + } + + /** Match arms allow void expressions, but still feed the enclosing value. */ + public function enterMatchArm(): self + { + return new self(false, null, null, resolveTemplateArguments: $this->resolveTemplateArguments, valueFlowTarget: $this->valueFlowTarget, valueConsumed: $this->isValueConsumed()); + } + + public function isValueConsumed(): bool + { + return $this->valueFlowTarget !== null || ($this->valueConsumed ?? $this->isDeep); + } + public function isDeep(): bool { return $this->isDeep; @@ -58,7 +108,7 @@ public function withoutTemplateArgumentResolution(): self return $this; } - return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, false); + return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, false, $this->valueFlowTarget, $this->valueFlowDirect, $this->arrayDimFetchRoot, $this->unsetTarget, $this->valueConsumed); } public function enterThrow(): self @@ -116,4 +166,53 @@ public function getInAssignRightSideNativeType(): ?Type return $this->inAssignRightSideNativeType; } + /** + * The expression computes the value of $target: variable reads inside it + * are not sinks, the target's value depends on them. $direct marks the + * assigned expression itself (a literal array there gets per-offset + * writes), an operand of a combinator is not direct. + */ + public function enterValueFlow(VariableWrite $target, bool $direct): self + { + return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, $this->resolveTemplateArguments, valueFlowTarget: $target, valueFlowDirect: $direct); + } + + public function getValueFlowTarget(): ?VariableWrite + { + return $this->valueFlowTarget; + } + + public function isValueFlowDirect(): bool + { + return $this->valueFlowDirect; + } + + /** + * The expression is the receiver of an offset read or write: a variable + * there is read as a container - its offsets are not. + */ + public function enterArrayDimFetchRoot(): self + { + return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, $this->resolveTemplateArguments, valueFlowTarget: $this->valueFlowTarget, valueFlowDirect: false, arrayDimFetchRoot: true); + } + + public function isArrayDimFetchRoot(): bool + { + return $this->arrayDimFetchRoot; + } + + /** + * The expression is an unset() target: its variable (or offset) is not + * read, the writes reaching it are discarded. + */ + public function enterUnsetTarget(): self + { + return new self($this->isDeep, $this->inAssignRightSideVariableName, $this->inAssignRightSideExpr, $this->inThrow, $this->inAssignRightSideType, $this->inAssignRightSideNativeType, $this->resolveTemplateArguments, valueFlowTarget: null, valueFlowDirect: false, arrayDimFetchRoot: false, unsetTarget: true); + } + + public function isUnsetTarget(): bool + { + return $this->unsetTarget; + } + } diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index e970bf128cd..e86cdf715f4 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -2018,7 +2018,7 @@ private function processClosureNodeInternal( $scope = $scope->assignVariable($inAssignRightSideVariableName, $variableType, $variableNativeType, TrinaryLogic::createYes()); } } - $this->processExprNode($stmt, $use->var, $useScope, $storage, $nodeCallback, $context); + $this->processExprNode($stmt, $use->var, $useScope, $storage, $nodeCallback, $context->withoutValueFlow()); if (!$use->byRef) { continue; } diff --git a/src/Analyser/StmtHandler/UnsetHandler.php b/src/Analyser/StmtHandler/UnsetHandler.php index 44d90a4ab7a..2c798f90687 100644 --- a/src/Analyser/StmtHandler/UnsetHandler.php +++ b/src/Analyser/StmtHandler/UnsetHandler.php @@ -21,15 +21,20 @@ use PHPStan\Analyser\StmtHandler; use PHPStan\Analyser\VariableFlow; use PHPStan\Analyser\VariableFlowBuilder; +use PHPStan\Analyser\VariableWriteOffset; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\DependencyInjection\Container; use PHPStan\Node\Expr\ExistingArrayDimFetch; use PHPStan\Node\Expr\ForeachValueByRefExpr; use PHPStan\Node\Expr\TypeExpr; use PHPStan\Node\Expr\UnsetOffsetExpr; +use PHPStan\Node\Variable\VariableWrite; use PHPStan\Type\ObjectType; +use PHPStan\Type\ResourceType; +use PHPStan\Type\Type; use function array_merge; use function is_string; +use function spl_object_id; /** * @implements StmtHandler @@ -63,10 +68,15 @@ public function processStmt( $variableFlows = []; foreach ($stmt->vars as $var) { $scope = $nodeScopeResolver->lookForSetAllowedUndefinedExpressions($scope, $var); - $exprResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, ExpressionContext::createDeep($context->shouldResolveTemplateArguments())); - $variableFlows[] = VariableFlowBuilder::targetRead($var, $storage, true); - if ($var instanceof Expr\Variable && is_string($var->name)) { - $variableFlows[] = VariableFlow::mention($var->name); + $exprResult = $nodeScopeResolver->processExprNode($stmt, $var, $scope, $storage, $nodeCallback, ExpressionContext::createDeep($context->shouldResolveTemplateArguments())->enterUnsetTarget()); + $variableFlows[] = VariableFlowBuilder::targetRead($var, $storage, $this->hasDestructionSideEffects($exprResult->getType())); + $root = $var; + while ($root instanceof ArrayDimFetch) { + $root = $root->var; + } + if ($root instanceof Expr\Variable && is_string($root->name)) { + $dimResult = $var instanceof ArrayDimFetch && $var->dim !== null ? $storage->findExpressionResult($var->dim) : null; + $variableFlows[] = VariableFlow::discard(new VariableWrite($root->name, $var, spl_object_id($var), VariableWrite::KIND_ASSIGN, $var instanceof ArrayDimFetch, $dimResult !== null ? VariableWriteOffset::fromType($dimResult->getType()) : null, replacesOffset: !($var instanceof ArrayDimFetch) || $var->var === $root)); } $scope = $exprResult->getScope(); $scope = $nodeScopeResolver->lookForUnsetAllowedUndefinedExpressions($scope, $var); @@ -138,4 +148,16 @@ public function processStmt( return new InternalStatementResult($scope, hasYield: $hasYield, isAlwaysTerminating: false, exitPoints: [], throwPoints: $throwPoints, impurePoints: $impurePoints, variableFlow: VariableFlow::sequence(...$variableFlows)); } + private function hasDestructionSideEffects(Type $type): bool + { + if (!$type->isObject()->no() || !(new ResourceType())->isSuperTypeOf($type)->no()) { + return true; + } + if ($type->isArray()->no()) { + return false; + } + + return $this->hasDestructionSideEffects($type->getIterableValueType()); + } + } diff --git a/src/Analyser/VariableAccessFlow.php b/src/Analyser/VariableAccessFlow.php index 14bba2a5d56..5cdb42fb95e 100644 --- a/src/Analyser/VariableAccessFlow.php +++ b/src/Analyser/VariableAccessFlow.php @@ -8,12 +8,18 @@ final class VariableAccessFlow extends VariableFlow { - /** @param VariableFlow::READ|VariableFlow::WRITE|VariableFlow::ESCAPE|VariableFlow::MENTION $kind */ + /** + * @param VariableFlow::READ|VariableFlow::WRITE|VariableFlow::ESCAPE|VariableFlow::MENTION|VariableFlow::DEFINE|VariableFlow::DISCARD $kind + * @param int|string|null $offset + */ public function __construct( string $kind, public readonly string $name, public readonly ?VariableWrite $write = null, public readonly ?Type $type = null, + public readonly ?int $targetId = null, + public readonly bool $container = false, + public readonly mixed $offset = null, ) { parent::__construct($kind); diff --git a/src/Analyser/VariableFlow.php b/src/Analyser/VariableFlow.php index 40ebd3b60d8..b73bd61fd6a 100644 --- a/src/Analyser/VariableFlow.php +++ b/src/Analyser/VariableFlow.php @@ -22,6 +22,8 @@ abstract class VariableFlow public const SWITCH = 'switch'; public const READ = 'read'; public const WRITE = 'write'; + public const DEFINE = 'define'; + public const DISCARD = 'discard'; public const ESCAPE = 'escape'; public const MENTION = 'mention'; public const READ_ALL = 'readAll'; @@ -77,13 +79,14 @@ public static function arrow(ArrowFunction $arrow, ?self $body, ?self $outputs): return new VariableControlFlow(self::ARROW, [$body, $outputs], arrow: $arrow); } - public static function read(string $name): ?self + /** @param int|string|null $offset */ + public static function read(string $name, ?int $targetId = null, bool $container = false, $offset = null): ?self { if ($name === 'this' || in_array($name, Scope::SUPERGLOBAL_VARIABLES, true)) { return null; } - return new VariableAccessFlow(self::READ, $name); + return new VariableAccessFlow(self::READ, $name, targetId: $targetId, container: $container, offset: $offset); } public static function conditional(?self $condition, ?self $if, ?self $else, ?bool $truthy): ?self @@ -106,7 +109,18 @@ public static function switch(?self $condition, array $cases, bool $exhaustive): public static function write(VariableWrite $write, ?Type $redundantType = null): self { - return new VariableAccessFlow(self::WRITE, $write->getVariableName(), $write, $redundantType); + return new VariableAccessFlow($write->getParentId() === null ? self::WRITE : self::DEFINE, $write->getVariableName(), $write, $redundantType); + } + + public static function discard(VariableWrite $write): self + { + return new VariableAccessFlow(self::DISCARD, $write->getVariableName(), $write); + } + + /** The enclosing expression consumes a write's inputs, without reading its target. */ + public static function inputs(int $writeId, ?int $targetId): self + { + return new VariableInputFlow($writeId, $targetId); } public static function escape(string $name): self diff --git a/src/Analyser/VariableFlowBuilder.php b/src/Analyser/VariableFlowBuilder.php index 124eedab194..c3020377a31 100644 --- a/src/Analyser/VariableFlowBuilder.php +++ b/src/Analyser/VariableFlowBuilder.php @@ -67,16 +67,20 @@ public static function child(?Node $node, ExpressionResultStorage $storage): ?Va return null; } - public static function targetRead(Expr $target, ExpressionResultStorage $storage, bool $read): ?VariableFlow + public static function targetRead(Expr $target, ExpressionResultStorage $storage, bool $read, ?int $targetId = null): ?VariableFlow { if ($target instanceof Expr\Variable) { - return is_string($target->name) ? ($read ? VariableFlow::read($target->name) : null) : self::child($target->name, $storage); + return is_string($target->name) ? ($read ? VariableFlow::read($target->name, $targetId) : null) : self::child($target->name, $storage); } if ($target instanceof Expr\List_ || $target instanceof Expr\Array_) { return null; } if ($target instanceof Expr\ArrayDimFetch) { - return VariableFlow::sequence(self::targetRead($target->var, $storage, true), self::child($target->dim, $storage)); + if ($target->var instanceof Expr\Variable && is_string($target->var->name)) { + $dimResult = $target->dim !== null ? $storage->findExpressionResult($target->dim) : null; + return VariableFlow::sequence(VariableFlow::read($target->var->name, $targetId, !$read, $read && $dimResult !== null ? VariableWriteOffset::fromType($dimResult->getType()) : null), self::child($target->dim, $storage)); + } + return VariableFlow::sequence(self::targetRead($target->var, $storage, true, $targetId), self::child($target->dim, $storage)); } if ($target instanceof Expr\PropertyFetch || $target instanceof Expr\NullsafePropertyFetch) { return VariableFlow::sequence(self::child($target->var, $storage), self::child($target->name, $storage)); @@ -100,26 +104,39 @@ public static function targetWrite(Expr $target, int $kind, MutatingScope $scope } return VariableFlow::sequence(...$writes); } - if ($target instanceof Expr\ArrayDimFetch) { - do { - $target = $target->var; - } while ($target instanceof Expr\ArrayDimFetch); - if (!$target instanceof Expr\Variable || !is_string($target->name) || $scope->hasVariableType($target->name)->no()) { + $write = self::writeSite($target, $kind, $scope, $storage); + return $write !== null ? VariableFlow::write($write, $redundant) : null; + } + + /** @param VariableWrite::KIND_* $kind */ + public static function writeSite(Expr $target, int $kind, MutatingScope $scope, ExpressionResultStorage $storage): ?VariableWrite + { + if ($target instanceof Expr\Variable && is_string($target->name)) { + if ($target->name === 'this' || in_array($target->name, Scope::SUPERGLOBAL_VARIABLES, true)) { return null; } - $type = $scope->getVariableType($target->name); + return new VariableWrite($target->name, $target, spl_object_id($target), $kind); + } + if (!$target instanceof Expr\ArrayDimFetch) { + return null; + } + $first = $target; + while ($first->var instanceof Expr\ArrayDimFetch) { + $first = $first->var; + } + $root = $first->var; + if (!$root instanceof Expr\Variable || !is_string($root->name) || $root->name === 'this' || in_array($root->name, Scope::SUPERGLOBAL_VARIABLES, true)) { + return null; + } + if (!$scope->hasVariableType($root->name)->no()) { + $type = $scope->getVariableType($root->name); if (!$type->isArray()->yes() && !$type->isString()->yes()) { return null; } - $kind = VariableWrite::KIND_ARRAY_DIM_WRITE; - } - if (!$target instanceof Expr\Variable || !is_string($target->name)) { - return null; } - return VariableFlow::sequence( - $kind === VariableWrite::KIND_ARRAY_DIM_WRITE ? VariableFlow::read($target->name) : null, - VariableFlow::write(new VariableWrite($target->name, $target, spl_object_id($target), $kind), $redundant), - ); + $dimResult = $first->dim !== null ? $storage->findExpressionResult($first->dim) : null; + $offset = $dimResult !== null ? VariableWriteOffset::fromType($dimResult->getType()) : null; + return new VariableWrite($root->name, $target, spl_object_id($target), $kind, true, $offset, replacesOffset: $first === $target); } public static function escapeRoot(Expr $expr): ?VariableFlow diff --git a/src/Analyser/VariableInputFlow.php b/src/Analyser/VariableInputFlow.php new file mode 100644 index 00000000000..ab60b383e2c --- /dev/null +++ b/src/Analyser/VariableInputFlow.php @@ -0,0 +1,16 @@ + */ private array $readIds = []; + /** @var array */ + private array $observedIds = []; + /** @var array */ private array $readNames = []; @@ -38,8 +44,40 @@ final class VariableLivenessResolver /** @var array */ private array $redundantTypes = []; + /** @var array> */ + private array $accesses = []; + + /** @var array> */ + private array $readKeys = []; + + /** @var array> */ + private array $nameKeys = []; + + /** @var array> */ + private array $observedKeys = []; + + /** @var array> */ + private array $killedKeys = []; + + /** @var array> */ + private array $dependencies = []; + + /** @var array> */ + private array $inputCopies = []; + + /** @var array */ + private array $inputSinks = []; + + /** @var array> */ + private array $literalItems = []; + + /** @var array */ + private array $allReadKeys = []; + private bool $opaque = false; + private bool $readsAllVariables = false; + private bool $allNamesMentioned = false; private bool $returnsByReference = false; @@ -78,27 +116,34 @@ public static function resolve(Node\FunctionLike $function, ?VariableFlow $flow) $body = VariableFlow::sequence(...[...$imports, $flow]); $self->collect($body); if ($self->writes !== [] && !$self->opaque) { + $self->compileAccesses(); $self->liveBefore($body, [], new VariableFlowContext([])); + $self->resolveDependencies(); } - return new VariableWritesNode($function, array_values($self->writes), $self->readIds, $self->readNames, $self->redundantTypes, $self->mentionedNames, $self->escapedNames, $self->opaque, $self->allNamesMentioned); + return new VariableWritesNode($function, array_values($self->writes), $self->observedIds + $self->readIds, $self->readIds, $self->readNames, $self->redundantTypes, $self->mentionedNames, $self->escapedNames, $self->opaque, $self->allNamesMentioned); } private function collect(?VariableFlow $flow, bool $dead = false): void { - if ($flow === null) { + if ($flow === null || $flow instanceof VariableInputFlow) { return; } if ($flow instanceof VariableAccessFlow && $flow->name !== 'this' && !in_array($flow->name, Scope::SUPERGLOBAL_VARIABLES, true)) { $this->mentionedNames[$flow->name] = true; + $this->accesses[$flow->name][] = $flow; if ($flow->kind === VariableFlow::READ) { $this->readNames[$flow->name] = true; } elseif ($flow->kind === VariableFlow::ESCAPE) { $this->escapedNames[$flow->name] = true; } - if ($flow->write !== null) { + if ($flow->write !== null && $flow->kind !== VariableFlow::DISCARD) { $id = $flow->write->getId(); $this->writes[$id] = $flow->write; + $parentId = $flow->write->getParentId(); + if ($parentId !== null) { + $this->literalItems[$parentId][$id] = $flow->write; + } if ($flow->type !== null) { $this->redundantTypes[$id] = $flow->type; } @@ -107,6 +152,9 @@ private function collect(?VariableFlow $flow, bool $dead = false): void } } } + if ($flow->kind === VariableFlow::READ_ALL) { + $this->readsAllVariables = true; + } if ($flow->kind === VariableFlow::OPAQUE) { $this->opaque = true; } @@ -146,16 +194,32 @@ private function liveBefore(?VariableFlow $flow, array $next, VariableFlowContex if ($flow === null || $flow->kind === VariableFlow::DEAD) { return $next; } + if ($flow instanceof VariableInputFlow) { + if ($flow->targetId === null) { + $this->inputSinks[$flow->writeId] = true; + } else { + $this->inputCopies[$flow->targetId][$flow->writeId] = true; + } + return $next; + } if ($flow instanceof VariableAccessFlow) { if (in_array($flow->kind, [VariableFlow::READ, VariableFlow::ESCAPE], true)) { // a by-reference capture aliases the variable - the value it // holds at that point is observable through the alias - $next[$flow->name] = true; - } elseif ($flow->write !== null) { - if (isset($next[$flow->name])) { - $this->readIds[$flow->write->getId()] = true; + return $next + ($this->readKeys[spl_object_id($flow)] ?? []); + } + if ($flow->write === null || $flow->kind === VariableFlow::DEFINE) { + return $next; + } + $id = $flow->write->getId(); + if ($flow->kind !== VariableFlow::DISCARD) { + $this->observeWrite($id, $next); + foreach ($this->literalItems[$id] ?? [] as $item) { + $this->observeWrite($item->getId(), $next); } - unset($next[$flow->name]); + } + foreach (array_keys($this->killedKeys[$id] ?? []) as $key) { + unset($next[$key]); } return $next; } @@ -182,7 +246,9 @@ private function liveBefore(?VariableFlow $flow, array $next, VariableFlowContex if (!$param->var instanceof Node\Expr\Variable || !is_string($param->var->name)) { continue; } - unset($names[$param->var->name]); + foreach (array_keys($this->nameKeys[$param->var->name] ?? []) as $key) { + unset($names[$key]); + } } return $next + $names; } @@ -287,9 +353,146 @@ private function liveBefore(?VariableFlow $flow, array $next, VariableFlowContex } if ($flow->kind === VariableFlow::READ_ALL) { $this->readNames += $this->mentionedNames; - return $next + $this->mentionedNames; + return $next + $this->allReadKeys; } return $next; } + /** @param int|string $offset */ + private static function offsetKey($offset): string + { + return (is_int($offset) ? 'i:' : 's:') . $offset; + } + + /** Compile each access once; loop iterations only union and remove its keys. */ + private function compileAccesses(): void + { + foreach ($this->accesses as $name => $accesses) { + $slots = ['container' => true, 'unknown' => true]; + foreach ($accesses as $access) { + $offset = $access->write !== null ? $access->write->getOffset() : $access->offset; + if ($offset === null) { + continue; + } + + $slots[self::offsetKey($offset)] = true; + } + $keysBySlot = []; + foreach ($accesses as $access) { + if (!in_array($access->kind, [VariableFlow::READ, VariableFlow::ESCAPE], true)) { + continue; + } + if ($access->container) { + $selected = ['container' => true]; + } elseif ($access->offset !== null) { + $selected = ['container' => true, self::offsetKey($access->offset) => true]; + } else { + $selected = $slots; + } + foreach (array_keys($selected) as $slot) { + $key = $name . "\0" . $slot . "\0" . ($access->targetId ?? 0); + $keysBySlot[$slot][$key] = $access->targetId; + $this->readKeys[spl_object_id($access)][$key] = true; + $this->nameKeys[$name][$key] = true; + } + } + // A dynamic observation sees every offset, including ones never named by a read. + if ($this->readsAllVariables) { + foreach (array_keys($slots) as $slot) { + $key = $name . "\0" . $slot . "\0" . 0; + $keysBySlot[$slot][$key] = null; + $this->allReadKeys[$key] = true; + $this->nameKeys[$name][$key] = true; + } + } + foreach ($accesses as $access) { + $write = $access->write; + if ($write === null) { + continue; + } + $id = $write->getId(); + $offset = $write->getOffset(); + if ($write->isOffsetWrite() && $offset !== null) { + $slot = self::offsetKey($offset); + $selectedKeys = [$slot => $keysBySlot[$slot] ?? []]; + } else { + $selectedKeys = $keysBySlot; + if ($write->isOffsetWrite()) { + unset($selectedKeys['container']); + } + } + $kills = !$write->isOffsetWrite() || ($offset !== null && $write->replacesOffset()); + foreach ($selectedKeys as $keys) { + foreach ($keys as $key => $targetId) { + $this->observedKeys[$id][$key] = $targetId; + if (!$kills) { + continue; + } + + $this->killedKeys[$id][$key] = true; + } + } + } + } + } + + /** @param array $next */ + private function observeWrite(int $id, array $next): void + { + foreach ($this->observedKeys[$id] ?? [] as $key => $targetId) { + if (!isset($next[$key])) { + continue; + } + $this->observedIds[$id] = true; + if ($targetId === null) { + $this->readIds[$id] = true; + } else { + $this->dependencies[$targetId][$id] = true; + } + } + } + + private function resolveDependencies(): void + { + $stack = []; + foreach (array_keys($this->readIds) as $id) { + $stack[] = [$id, false]; + } + foreach (array_keys($this->inputSinks) as $id) { + $stack[] = [$id, true]; + } + foreach ($this->writes as $id => $write) { + if (!isset($this->escapedNames[$write->getVariableName()])) { + continue; + } + // a write to an aliased variable is observable through the alias, + // so whatever flows into it is used; whether the write itself is + // read stays with the flow-sensitive capture read + $stack[] = [$id, false]; + $stack[] = [$id, true]; + } + $visited = []; + while ($stack !== []) { + [$id, $inputs] = array_pop($stack); + $key = $id . ($inputs ? ':inputs' : ':value'); + if (isset($visited[$key])) { + continue; + } + $visited[$key] = true; + foreach (array_keys($this->dependencies[$id] ?? []) as $dependency) { + $this->readIds[$dependency] = true; + $stack[] = [$dependency, false]; + } + foreach (array_keys($this->inputCopies[$id] ?? []) as $source) { + $stack[] = [$source, true]; + } + if (!$inputs) { + continue; + } + foreach ($this->literalItems[$id] ?? [] as $item) { + $stack[] = [$item->getId(), true]; + } + } + } + } diff --git a/src/Analyser/VariableWriteOffset.php b/src/Analyser/VariableWriteOffset.php new file mode 100644 index 00000000000..4c7c5b428b5 --- /dev/null +++ b/src/Analyser/VariableWriteOffset.php @@ -0,0 +1,39 @@ +toArrayKey(); + if (!$keyType->isConstantScalarValue()->yes()) { + return null; + } + $values = $keyType->getConstantScalarValues(); + if (count($values) !== 1) { + return null; + } + $value = $values[0]; + if (is_int($value) || is_string($value)) { + return $value; + } + + return null; + } + +} diff --git a/src/Node/Variable/VariableWrite.php b/src/Node/Variable/VariableWrite.php index 69e2b95effb..2efcb502a1b 100644 --- a/src/Node/Variable/VariableWrite.php +++ b/src/Node/Variable/VariableWrite.php @@ -2,7 +2,7 @@ namespace PHPStan\Node\Variable; -use PhpParser\Node\Expr; +use PhpParser\Node; /** * A write site of a local variable inside a function-like body. @@ -26,15 +26,21 @@ final class VariableWrite public const KIND_CATCH = 11; public const KIND_PARAMETER = 12; public const KIND_CLOSURE_USE = 13; + public const KIND_ARRAY_LITERAL_ITEM = 14; /** * @param self::KIND_* $kind + * @param int|string|null $offset */ public function __construct( private string $variableName, - private Expr\Variable $variable, + private Node $node, private int $id, private int $kind, + private bool $offsetWrite = false, + private $offset = null, + private ?int $parentId = null, + private bool $replacesOffset = true, ) { } @@ -47,9 +53,9 @@ public function getVariableName(): string /** * The target node of the write - the source of the reported line. */ - public function getVariable(): Expr\Variable + public function getNode(): Node { - return $this->variable; + return $this->node; } public function getId(): int @@ -65,4 +71,25 @@ public function getKind(): int return $this->kind; } + public function isOffsetWrite(): bool + { + return $this->offsetWrite; + } + + /** @return int|string|null */ + public function getOffset() + { + return $this->offset; + } + + public function getParentId(): ?int + { + return $this->parentId; + } + + public function replacesOffset(): bool + { + return $this->replacesOffset; + } + } diff --git a/src/Node/VariableWritesNode.php b/src/Node/VariableWritesNode.php index 38593e6b2cd..5568b291b1b 100644 --- a/src/Node/VariableWritesNode.php +++ b/src/Node/VariableWritesNode.php @@ -23,6 +23,7 @@ final class VariableWritesNode extends NodeAbstract implements VirtualNode /** * @param list $writes * @param array $readWriteIds + * @param array $usedWriteIds * @param array $readVariableNames * @param array $redundantWriteTypes * @param array $referencedVariableNames @@ -32,6 +33,7 @@ public function __construct( private Node\FunctionLike $functionLike, private array $writes, private array $readWriteIds, + private array $usedWriteIds, private array $readVariableNames, private array $redundantWriteTypes, private array $referencedVariableNames, @@ -63,7 +65,7 @@ public function getWrites(): array public function getWriteForNode(Node\Expr\Variable $variable): ?VariableWrite { foreach ($this->writes as $write) { - if ($write->getVariable() === $variable) { + if ($write->getNode() === $variable) { return $write; } } @@ -80,9 +82,13 @@ public function areAllVariableNamesReferenced(): bool return $this->allVariableNamesReferenced; } - /** - * Whether some path from the write reaches a read of the written value. - */ + /** Whether the value reaches an observable use, directly or through another write. */ + public function isUsed(VariableWrite $write): bool + { + return isset($this->usedWriteIds[$write->getId()]); + } + + /** Whether some path from the write reaches a read of the written value. */ public function isRead(VariableWrite $write): bool { return isset($this->readWriteIds[$write->getId()]); diff --git a/src/Rules/DeadCode/UnusedVariableRule.php b/src/Rules/DeadCode/UnusedVariableRule.php index 21a92958cb2..b47e4448c8e 100644 --- a/src/Rules/DeadCode/UnusedVariableRule.php +++ b/src/Rules/DeadCode/UnusedVariableRule.php @@ -4,15 +4,20 @@ use PhpParser\Node; use PHPStan\Analyser\Scope; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Node\Variable\VariableWrite; use PHPStan\Node\VariableWritesNode; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\ShouldNotHappenException; +use PHPStan\Type\Constant\ConstantIntegerType; +use PHPStan\Type\Constant\ConstantStringType; use PHPStan\Type\VerbosityLevel; use function in_array; +use function is_int; use function sprintf; use function str_starts_with; +use function ucfirst; /** * @implements Rule @@ -20,7 +25,7 @@ final class UnusedVariableRule implements Rule { - public function __construct() + public function __construct(private ExprPrinter $exprPrinter) { } @@ -36,7 +41,9 @@ public function processNode(Node $node, Scope $scope): array } $namesWithReadWrite = []; + $writesById = []; foreach ($node->getWrites() as $write) { + $writesById[$write->getId()] = $write; if (!$node->isRead($write)) { continue; } @@ -63,29 +70,53 @@ public function processNode(Node $node, Scope $scope): array continue; } - if (!$node->isRead($write)) { - // A variable that is never read at all (an unused variable) is a stronger - // finding than a single dead store to a variable the body does read. - $unusedVariable = !isset($namesWithReadWrite[$name]) && !$node->isVariableEverRead($name); - $errors[] = RuleErrorBuilder::message($this->getMessage($write->getKind(), $name, $unusedVariable)) - ->identifier($this->getIdentifier($write->getKind(), $unusedVariable)) - ->line($write->getVariable()->getStartLine()) + $redundantType = $node->getRedundantType($write); + if ($redundantType !== null && $node->isUsed($write)) { + $errors[] = RuleErrorBuilder::message(sprintf( + 'Variable $%s is assigned value %s but it already has that value.', + $name, + $redundantType->describe(VerbosityLevel::value()), + )) + ->identifier('assign.redundant') + ->line($write->getNode()->getStartLine()) ->build(); continue; } - $redundantType = $node->getRedundantType($write); - if ($redundantType === null) { + if ($node->isUsed($write)) { continue; } - $errors[] = RuleErrorBuilder::message(sprintf( - 'Variable $%s is assigned value %s but it already has that value.', - $name, - $redundantType->describe(VerbosityLevel::value()), - )) - ->identifier('assign.redundant') - ->line($write->getVariable()->getStartLine()) + $unusedVariable = !isset($namesWithReadWrite[$name]) && !$node->isVariableEverRead($name); + $parentId = $write->getParentId(); + if ($parentId !== null) { + $parent = $writesById[$parentId] ?? null; + if ($parent === null || !$node->isUsed($parent) || $write->getOffset() === null) { + continue; + } + $offset = $write->getOffset(); + $offsetType = is_int($offset) ? new ConstantIntegerType($offset) : new ConstantStringType($offset); + $message = sprintf( + 'Offset %s of array assigned to variable $%s %s.', + $offsetType->describe(VerbosityLevel::value()), + $name, + $node->isRead($write) ? 'only flows into values that are never used' : 'is never read', + ); + $identifier = $node->isRead($write) ? 'array.unusedOffsetFlow' : 'array.unusedOffset'; + } elseif ($write->isOffsetWrite()) { + $target = $write->getNode(); + if (!$target instanceof Node\Expr) { + throw new ShouldNotHappenException(); + } + $message = $this->getMessage($write->getKind(), $this->exprPrinter->printExpr($target), false, $node->isRead($write)); + $identifier = $this->getIdentifier($write->getKind(), false, $node->isRead($write)); + } else { + $message = $this->getMessage($write->getKind(), 'variable $' . $name, $unusedVariable, $node->isRead($write)); + $identifier = $this->getIdentifier($write->getKind(), $unusedVariable, $node->isRead($write)); + } + $errors[] = RuleErrorBuilder::message($message) + ->identifier($identifier) + ->line($write->getNode()->getStartLine()) ->build(); } @@ -95,30 +126,34 @@ public function processNode(Node $node, Scope $scope): array /** * @param VariableWrite::KIND_* $kind */ - private function getMessage(int $kind, string $variableName, bool $unusedVariable): string + private function getMessage(int $kind, string $target, bool $unusedVariable, bool $read): string { + // "never read": nothing looks at the written value; "only flows into + // values that are never used": something reads it, but only to compute + // values that never reach a sink themselves + $outcome = $read ? 'only flows into values that are never used' : 'is never read'; switch ($kind) { case VariableWrite::KIND_ASSIGN: case VariableWrite::KIND_READ_MODIFY_WRITE: case VariableWrite::KIND_ARRAY_DIM_WRITE: case VariableWrite::KIND_LIST_ITEM: if ($unusedVariable) { - return sprintf('Variable $%s is never read.', $variableName); + return sprintf('%s is never read.', ucfirst($target)); } - return sprintf('Value assigned to variable $%s is never read.', $variableName); + return sprintf('Value assigned to %s %s.', $target, $outcome); case VariableWrite::KIND_PRE_INC: case VariableWrite::KIND_POST_INC: - return sprintf('Value of variable $%s after ++ is never read.', $variableName); + return sprintf('Value of %s after ++ %s.', $target, $outcome); case VariableWrite::KIND_PRE_DEC: case VariableWrite::KIND_POST_DEC: - return sprintf('Value of variable $%s after -- is never read.', $variableName); + return sprintf('Value of %s after -- %s.', $target, $outcome); case VariableWrite::KIND_FOREACH_VALUE: - return sprintf('Foreach value variable $%s is never read.', $variableName); + return sprintf('Foreach value %s %s.', $target, $outcome); case VariableWrite::KIND_FOREACH_KEY: - return sprintf('Foreach key variable $%s is never read.', $variableName); + return sprintf('Foreach key %s %s.', $target, $outcome); case VariableWrite::KIND_CATCH: - return sprintf('Catch variable $%s is never read.', $variableName); + return sprintf('Catch %s %s.', $target, $outcome); } throw new ShouldNotHappenException(sprintf('Unhandled variable write kind %d', $kind)); @@ -126,9 +161,9 @@ private function getMessage(int $kind, string $variableName, bool $unusedVariabl /** * @param VariableWrite::KIND_* $kind - * @return 'variable.unused'|'assign.unused'|'preInc.unused'|'postInc.unused'|'preDec.unused'|'postDec.unused'|'foreach.unusedValue'|'foreach.unusedKey'|'catch.unusedVariable' + * @return 'variable.unused'|'assign.unused'|'assign.unusedFlow'|'preInc.unused'|'preInc.unusedFlow'|'postInc.unused'|'postInc.unusedFlow'|'preDec.unused'|'preDec.unusedFlow'|'postDec.unused'|'postDec.unusedFlow'|'foreach.unusedValue'|'foreach.unusedValueFlow'|'foreach.unusedKey'|'foreach.unusedKeyFlow'|'catch.unusedVariable'|'catch.unusedVariableFlow' */ - private function getIdentifier(int $kind, bool $unusedVariable): string + private function getIdentifier(int $kind, bool $unusedVariable, bool $read): string { switch ($kind) { case VariableWrite::KIND_ASSIGN: @@ -139,21 +174,21 @@ private function getIdentifier(int $kind, bool $unusedVariable): string return 'variable.unused'; } - return 'assign.unused'; + return $read ? 'assign.unusedFlow' : 'assign.unused'; case VariableWrite::KIND_PRE_INC: - return 'preInc.unused'; + return $read ? 'preInc.unusedFlow' : 'preInc.unused'; case VariableWrite::KIND_POST_INC: - return 'postInc.unused'; + return $read ? 'postInc.unusedFlow' : 'postInc.unused'; case VariableWrite::KIND_PRE_DEC: - return 'preDec.unused'; + return $read ? 'preDec.unusedFlow' : 'preDec.unused'; case VariableWrite::KIND_POST_DEC: - return 'postDec.unused'; + return $read ? 'postDec.unusedFlow' : 'postDec.unused'; case VariableWrite::KIND_FOREACH_VALUE: - return 'foreach.unusedValue'; + return $read ? 'foreach.unusedValueFlow' : 'foreach.unusedValue'; case VariableWrite::KIND_FOREACH_KEY: - return 'foreach.unusedKey'; + return $read ? 'foreach.unusedKeyFlow' : 'foreach.unusedKey'; case VariableWrite::KIND_CATCH: - return 'catch.unusedVariable'; + return $read ? 'catch.unusedVariableFlow' : 'catch.unusedVariable'; } throw new ShouldNotHappenException(sprintf('Unhandled variable write kind %d', $kind)); diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index c97f0898c45..e4713e2eb76 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -194,7 +194,13 @@ public function testBug12803(): void public function testArrayDestructuringArrayDimFetch(): void { $errors = $this->runAnalyse(__DIR__ . '/data/array-destructuring-array-dim-fetch.php'); - $this->assertNoErrors($errors); + $this->assertCount(2, $errors); + $this->assertSame('Value assigned to $barcodes[] is never read.', $errors[0]->getMessage()); + $this->assertSame('assign.unused', $errors[0]->getIdentifier()); + $this->assertSame(6, $errors[0]->getLine()); + $this->assertSame('Value assigned to $barcodes[] is never read.', $errors[1]->getMessage()); + $this->assertSame('assign.unused', $errors[1]->getIdentifier()); + $this->assertSame(13, $errors[1]->getLine()); } public function testNestedNamespaces(): void @@ -1399,9 +1405,18 @@ public function testBug10509(): void #[RequiresPhp('>= 8.1.0')] public function testBug10847(): void { - // false positive $errors = $this->runAnalyse(__DIR__ . '/data/bug-10847.php'); - $this->assertNoErrors($errors); + // The loop appends to $overloads instead of $processedOverloads. + $this->assertCount(3, $errors); + $this->assertSame('Foreach key variable $args only flows into values that are never used.', $errors[0]->getMessage()); + $this->assertSame('foreach.unusedKeyFlow', $errors[0]->getIdentifier()); + $this->assertSame(158, $errors[0]->getLine()); + $this->assertSame('Foreach value variable $callback only flows into values that are never used.', $errors[1]->getMessage()); + $this->assertSame('foreach.unusedValueFlow', $errors[1]->getIdentifier()); + $this->assertSame(158, $errors[1]->getLine()); + $this->assertSame('Value assigned to $overloads[] is never read.', $errors[2]->getMessage()); + $this->assertSame('assign.unused', $errors[2]->getIdentifier()); + $this->assertSame(159, $errors[2]->getLine()); } #[RequiresPhp('>= 8.1.0')] diff --git a/tests/PHPStan/Levels/data/arrayDimFetches-4.json b/tests/PHPStan/Levels/data/arrayDimFetches-4.json new file mode 100644 index 00000000000..db22ecc77d2 --- /dev/null +++ b/tests/PHPStan/Levels/data/arrayDimFetches-4.json @@ -0,0 +1,7 @@ +[ + { + "message": "Offset 'a' of array assigned to variable $arr is never read.", + "line": 34, + "ignorable": true + } +] \ No newline at end of file diff --git a/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php index fc030d3a275..0c6661162f4 100644 --- a/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php @@ -2,6 +2,7 @@ namespace PHPStan\Rules\DeadCode; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\Rule; use PHPStan\Testing\RuleTestCase; use PHPUnit\Framework\Attributes\RequiresPhp; @@ -21,7 +22,7 @@ protected function shouldPolluteScopeWithAlwaysIterableForeach(): bool protected function getRule(): Rule { - return new UnusedVariableRule(); + return new UnusedVariableRule(self::getContainer()->getByType(ExprPrinter::class)); } public function testThrowableCatchAfterDocumentedException(): void @@ -66,146 +67,47 @@ public function testOverridingThrowsReachesCatch(): void public function testRule(): void { $this->analyse([__DIR__ . '/data/unused-variable.php'], [ - [ - 'Variable $a is never read.', - 27, - ], - [ - 'Value assigned to variable $a is never read.', - 32, - ], - [ - 'Variable $a is never read.', - 40, - ], - [ - 'Value assigned to variable $x is never read.', - 46, - ], - [ - 'Variable $a is never read.', - 70, - ], - [ - 'Variable $a is never read.', - 76, - ], - [ - 'Variable $a is never read.', - 93, - ], - [ - 'Variable $a is never read.', - 95, - ], - [ - 'Variable $a is never read.', - 101, - ], - [ - 'Variable $a is never read.', - 113, - ], - [ - 'Foreach key variable $k is never read.', - 119, - ], - [ - 'Foreach value variable $v is never read.', - 126, - ], - [ - 'Foreach value variable $v is never read.', - 133, - ], - [ - 'Variable $a is never read.', - 148, - ], - [ - 'Value of variable $i after ++ is never read.', - 157, - ], - [ - 'Variable $x is never read.', - 223, - ], - [ - 'Value assigned to variable $x is never read.', - 251, - ], - [ - 'Value assigned to variable $s is never read.', - 264, - ], - [ - 'Variable $a is never read.', - 276, - ], - [ - 'Variable $f is never read.', - 283, - ], - [ - 'Variable $a is never read.', - 303, - ], - [ - 'Variable $x is never read.', - 337, - ], - [ - 'Value assigned to variable $title is never read.', - 422, - ], - [ - 'Variable $b is never read.', - 614, - ], - [ - 'Variable $a is never read.', - 632, - ], - [ - 'Variable $a is never read.', - 637, - ], - [ - 'Value assigned to variable $a is never read.', - 703, - ], - [ - 'Variable $a is never read.', - 739, - ], - [ - 'Value assigned to variable $a is never read.', - 744, - ], - [ - 'Value assigned to variable $tags is never read.', - 840, - ], - [ - 'Value of variable $i after -- is never read.', - 864, - ], - [ - 'Value assigned to variable $x is never read.', - 870, - ], - [ - 'Foreach value variable $v is never read.', - 877, - ], - [ - 'Value of variable $i after ++ is never read.', - 885, - ], - [ - 'Value of variable $i after -- is never read.', - 892, - ], + ['Variable $a is never read.', 27], + ['Value assigned to variable $a is never read.', 32], + ['Variable $a is never read.', 40], + ['Value assigned to variable $x is never read.', 46], + ['Variable $a is never read.', 70], + ['Variable $a is never read.', 76], + ['Variable $a is never read.', 93], + ['Variable $a is never read.', 95], + ['Variable $a is never read.', 101], + ['Variable $a is never read.', 113], + ['Foreach key variable $k is never read.', 119], + ['Foreach value variable $v is never read.', 126], + ['Foreach value variable $v is never read.', 133], + ['Variable $a is never read.', 148], + ['Value of variable $i after ++ is never read.', 157], + ['Variable $x is never read.', 223], + ['Value assigned to $x[] is never read.', 250], + ['Value assigned to $x[\'k\'] is never read.', 251], + ['Value assigned to variable $s only flows into values that are never used.', 263], + ['Value assigned to variable $s is never read.', 264], + ['Variable $a is never read.', 269], + ['Variable $a is never read.', 276], + ['Variable $f is never read.', 283], + ['Variable $a is never read.', 303], + ['Variable $x is never read.', 337], + ['Value assigned to variable $title is never read.', 422], + ['Variable $b is never read.', 614], + ['Variable $a is never read.', 632], + ['Variable $a is never read.', 637], + ['Value assigned to variable $a only flows into values that are never used.', 702], + ['Value assigned to variable $a is never read.', 703], + ['Value assigned to variable $b only flows into values that are never used.', 709], + ['Value assigned to variable $b only flows into values that are never used.', 719], + ['Variable $a is never read.', 739], + ['Value assigned to variable $a is never read.', 744], + ['Value assigned to variable $tags is never read.', 840], + ['Value of variable $i after -- is never read.', 864], + ['Value assigned to variable $x is never read.', 870], + ['Foreach value variable $v is never read.', 877], + ['Value of variable $i after ++ is never read.', 885], + ['Value of variable $i after -- is never read.', 892], ]); } @@ -213,14 +115,11 @@ public function testRule(): void public function testPhp8(): void { $this->analyse([__DIR__ . '/data/unused-variable-php8.php'], [ - [ - 'Catch variable $e is never read.', - 23, - ], - [ - 'Value assigned to variable $nightsFrom is never read.', - 98, - ], + ['Catch variable $e is never read.', 23], + ['Value assigned to variable $nightsFrom is never read.', 98], + ['Value assigned to variable $a only flows into values that are never used.', 107], + ['Variable $b is never read.', 108], + ['Variable $b is never read.', 116], ]); } @@ -261,14 +160,9 @@ public function testBug14258(): void public function testBug12012(): void { $this->analyse([__DIR__ . '/data/bug-12012.php'], [ - [ - 'Value assigned to variable $s1 is never read.', - 10, - ], - [ - 'Value assigned to variable $s1 is never read.', - 12, - ], + ['Value assigned to variable $s1 only flows into values that are never used.', 9], + ['Value assigned to variable $s1 is never read.', 10], + ['Value assigned to variable $s1 is never read.', 12], ]); } @@ -320,38 +214,14 @@ public function testCatchVariableReportedSincePhp80(): void public function testRedundantAssignment(): void { $this->analyse([__DIR__ . '/data/unused-variable-redundant.php'], [ - [ - 'Variable $x is assigned value true but it already has that value.', - 26, - ], - [ - 'Value assigned to variable $x is never read.', - 42, - ], - [ - 'Variable $x is assigned value 1 but it already has that value.', - 43, - ], - [ - 'Variable $x is assigned value null but it already has that value.', - 51, - ], - [ - 'Variable $s is assigned value \'a\' but it already has that value.', - 60, - ], - [ - 'Variable $a is assigned value array{k: 1} but it already has that value.', - 69, - ], - [ - 'Value assigned to variable $x is never read.', - 95, - ], - [ - 'Variable $x is assigned value 1 but it already has that value.', - 118, - ], + ['Variable $x is assigned value true but it already has that value.', 26], + ['Value assigned to variable $x is never read.', 42], + ['Variable $x is assigned value 1 but it already has that value.', 43], + ['Variable $x is assigned value null but it already has that value.', 51], + ['Variable $s is assigned value \'a\' but it already has that value.', 60], + ['Variable $a is assigned value array{k: 1} but it already has that value.', 69], + ['Value assigned to variable $x is never read.', 95], + ['Variable $x is assigned value 1 but it already has that value.', 118], ]); } @@ -428,4 +298,212 @@ public function testBroadCatchesIncludeImplicitThrows(): void ]); } + public function testValueFlow(): void + { + $this->analyse([__DIR__ . '/data/unused-variable-value-flow.php'], [ + ['Value assigned to variable $a only flows into values that are never used.', 27], + ['Value assigned to variable $a is never read.', 28], + ['Value assigned to variable $s only flows into values that are never used.', 40], + ['Value assigned to variable $s only flows into values that are never used.', 41], + ['Value assigned to variable $s is never read.', 42], + ['Value assigned to variable $i only flows into values that are never used.', 54], + ['Value of variable $i after ++ only flows into values that are never used.', 55], + ['Value of variable $i after ++ only flows into values that are never used.', 56], + ['Value of variable $i after -- only flows into values that are never used.', 57], + ['Value of variable $i after -- is never read.', 58], + ['Value of variable $i after ++ is never read.', 71], + ['Value assigned to variable $i only flows into values that are never used.', 76], + ['Value of variable $i after ++ is never read.', 77], + ['Variable $j is never read.', 77], + ['Value assigned to variable $n only flows into values that are never used.', 98], + ['Value assigned to variable $n only flows into values that are never used.', 100], + ['Value assigned to variable $a only flows into values that are never used.', 117], + ['Variable $ok is never read.', 118], + ['Value assigned to variable $a only flows into values that are never used.', 130], + ['Variable $b is never read.', 131], + ['Variable $b is never read.', 144], + ['Variable $b is never read.', 150], + ['Value assigned to variable $a only flows into values that are never used.', 155], + ['Variable $arr is never read.', 156], + ['Value assigned to variable $a only flows into values that are never used.', 168], + ['Variable $b is never read.', 169], + ['Variable $c is never read.', 170], + ['Value assigned to variable $a only flows into values that are never used.', 175], + ['Variable $b is never read.', 176], + ['Variable $c is never read.', 177], + ['Variable $d is never read.', 178], + ['Value assigned to variable $a only flows into values that are never used.', 183], + ['Variable $b is never read.', 184], + ['Value assigned to variable $a only flows into values that are never used.', 189], + ['Variable $b is never read.', 190], + ['Value assigned to variable $d only flows into values that are never used.', 195], + ['Variable $b is never read.', 196], + ['Variable $b is never read.', 202], + ['Variable $b is never read.', 209], + ['Variable $d is never read.', 210], + ['Variable $b is never read.', 216], + ['Variable $b is never read.', 222], + ['Variable $b is never read.', 228], + ['Variable $b is never read.', 234], + ['Variable $f is never read.', 240], + ['Variable $f is never read.', 248], + ['Variable $b is never read.', 254], + ['Variable $b is never read.', 260], + ['Variable $b is never read.', 266], + ['Variable $b is never read.', 272], + ['Variable $b is never read.', 278], + ['Variable $b is never read.', 284], + ['Variable $b is never read.', 289], + ['Variable $a is never read.', 295], + ['Variable $b is never read.', 302], + ['Value assigned to variable $c only flows into values that are never used.', 308], + ['Variable $b is never read.', 309], + ['Variable $a is never read.', 309], + ['Variable $b is never read.', 315], + ['Value assigned to variable $a only flows into values that are never used.', 321], + ['Variable $b is never read.', 322], + ['Value assigned to variable $v only flows into values that are never used.', 329], + ['Value assigned to $a[\'x\'] is never read.', 331], + ['Value assigned to variable $a only flows into values that are never used.', 344], + ['Variable $b is never read.', 345], + ['Value assigned to variable $i only flows into values that are never used.', 357], + ['Value assigned to variable $a only flows into values that are never used.', 358], + ['Variable $b is never read.', 359], + ['Variable $a is never read.', 364], + ['Value assigned to variable $a only flows into values that are never used.', 369], + ['Value assigned to variable $b only flows into values that are never used.', 370], + ['Value assigned to variable $c only flows into values that are never used.', 371], + ['Variable $d is never read.', 372], + ['Value assigned to variable $s only flows into values that are never used.', 395], + ['Foreach value variable $v only flows into values that are never used.', 396], + ['Value assigned to variable $s only flows into values that are never used.', 397], + ['Value assigned to variable $a only flows into values that are never used.', 403], + ['Value assigned to variable $a is never read.', 404], + ['Value assigned to variable $s is never read.', 477], + ['Value assigned to variable $s only flows into values that are never used.', 483], + ['Value assigned to variable $s is never read.', 484], + ['Variable $x is never read.', 484], + ]); + } + + public function testOffsets(): void + { + $this->analyse([__DIR__ . '/data/unused-variable-offsets.php'], [ + ['Offset \'x\' of array assigned to variable $a is never read.', 27], + ['Variable $a is never read.', 39], + ['Offset \'x\' of array assigned to variable $a is never read.', 44], + ['Offset 1 of array assigned to variable $a is never read.', 50], + ['Offset 2 of array assigned to variable $a is never read.', 50], + ['Offset \'y\' of array assigned to variable $a is never read.', 58], + ['Offset \'x\' of array assigned to variable $a is never read.', 67], + ['Offset \'y\' of array assigned to variable $a is never read.', 95], + ['Offset \'y\' of array assigned to variable $a is never read.', 103], + ['Offset \'z\' of array assigned to variable $a is never read.', 117], + ['Offset \'x\' of array assigned to variable $a is never read.', 124], + ['Offset 2 of array assigned to variable $a is never read.', 136], + ['Offset 6 of array assigned to variable $a is never read.', 142], + ['Offset \'x\' of array assigned to variable $a is never read.', 149], + ['Value assigned to variable $v only flows into values that are never used.', 156], + ['Offset \'x\' of array assigned to variable $a is never read.', 157], + ['Offset \'y\' of array assigned to variable $a is never read.', 163], + ['Offset \'y\' of array assigned to variable $a is never read.', 169], + ['Offset \'y\' of array assigned to variable $a is never read.', 175], + ['Value assigned to $a[\'x\'] is never read.', 191], + ['Value assigned to $a[\'x\'] is never read.', 211], + ['Value assigned to $a[\'x\'] is never read.', 219], + ['Value assigned to $a[$i] is never read.', 234], + ['Value assigned to $a[] is never read.', 247], + ['Value assigned to $a[\'x\'][\'y\'] is never read.', 260], + ['Offset \'x\' of array assigned to variable $a is never read.', 280], + ['Value assigned to variable $a only flows into values that are never used.', 294], + ['Value assigned to $a[\'x\'] is never read.', 295], + ['Value assigned to variable $a only flows into values that are never used.', 314], + ['Value of $a[\'n\'] after ++ is never read.', 315], + ['Value assigned to $s[0] is never read.', 328], + ['Value assigned to $a[\'x\'] is never read.', 356], + ['Value assigned to $a[\'x\'] is never read.', 380], + ['Value assigned to $a[\'y\'] is never read.', 380], + ['Foreach value $a[\'x\'] is never read.', 393], + ['Value assigned to $p[\'x\'] is never read.', 399], + ['Value assigned to $a[\'x\'] is never read.', 412], + ['Value assigned to $a[\'x\'] is never read.', 430], + ['Variable $a is never read.', 444], + ['Value assigned to variable $a is never read.', 450], + ['Offset \'x\' of array assigned to variable $a is never read.', 474], + ['Value assigned to $a[\'x\'] is never read.', 482], + ['Offset \'y\' of array assigned to variable $a is never read.', 510], + ['Offset \'y\' of array assigned to variable $a is never read.', 539], + ['Value assigned to $cache[\'x\'] is never read.', 565], + ['Variable $v is never read.', 565], + + ['Offset \'x\' of array assigned to variable $a only flows into values that are never used.', 570], + ['Variable $b is never read.', 571], + ]); + } + + public function testDynamicOffsetOverwritten(): void + { + $errors = $this->gatherAnalyserErrors([__DIR__ . '/data/unused-variable-offset-overwrite.php']); + $this->assertCount(1, $errors); + $this->assertSame('Value assigned to $a[$i] is never read.', $errors[0]->getMessage()); + $this->assertSame('assign.unused', $errors[0]->getIdentifier()); + $this->assertSame(8, $errors[0]->getLine()); + } + + public function testUnsetCallsDestructor(): void + { + $this->analyse([__DIR__ . '/data/unused-variable-destructor.php'], [ + ['Variable $unused is never read.', 33], + ]); + } + + public function testFlowMessages(): void + { + $this->analyse([__DIR__ . '/data/unused-variable-flow-messages.php'], [ + ['Value assigned to variable $a only flows into values that are never used.', 18], + ['Value assigned to variable $a only flows into values that are never used.', 20], + ['Value assigned to variable $s only flows into values that are never used.', 26], + ['Value assigned to variable $s only flows into values that are never used.', 28], + ['Offset \'k\' of array assigned to variable $a only flows into values that are never used.', 34], + ['Value assigned to $a[\'k\'] only flows into values that are never used.', 36], + ['Value assigned to variable $a only flows into values that are never used.', 42], + ['Value assigned to variable $a only flows into values that are never used.', 44], + ['Value assigned to variable $i only flows into values that are never used.', 50], + ['Value of variable $i after ++ only flows into values that are never used.', 52], + ['Value assigned to variable $i only flows into values that are never used.', 58], + ['Value of variable $i after ++ only flows into values that are never used.', 60], + ['Value assigned to variable $i only flows into values that are never used.', 66], + ['Value of variable $i after -- only flows into values that are never used.', 68], + ['Value assigned to variable $i only flows into values that are never used.', 74], + ['Value of variable $i after -- only flows into values that are never used.', 76], + ['Foreach key variable $k only flows into values that are never used.', 83], + ['Foreach value variable $v only flows into values that are never used.', 83], + ['Value assigned to variable $v only flows into values that are never used.', 85], + ['Value assigned to variable $k only flows into values that are never used.', 86], + ['Offset \'x\' of array assigned to variable $a only flows into values that are never used.', 93], + ['Value assigned to $a[\'x\'] only flows into values that are never used.', 95], + ['Value assigned to variable $a only flows into values that are never used.', 102], + ['Variable $b is never read.', 103], + ['Value assigned to variable $a only flows into values that are never used.', 108], + ['Value assigned to variable $a only flows into values that are never used.', 109], + ['Value assigned to variable $a is never read.', 110], + ['Value assigned to variable $c only flows into values that are never used.', 115], + ['Variable $a is never read.', 116], + ['Variable $b is never read.', 116], + ['Value assigned to variable $v only flows into values that are never used.', 121], + ['Offset \'x\' of array assigned to variable $a is never read.', 122], + ]); + } + + #[RequiresPhp('>= 8.0.0')] + public function testFlowMessagesCatch(): void + { + $this->analyse([__DIR__ . '/data/unused-variable-flow-messages-catch.php'], [ + ['Catch variable $e only flows into values that are never used.', 15], + ['Value assigned to variable $e only flows into values that are never used.', 17], + ['Catch variable $e only flows into values that are never used.', 26], + ['Variable $copy is never read.', 27], + ]); + } + } diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-destructor.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-destructor.php new file mode 100644 index 00000000000..ddab73aeb44 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-destructor.php @@ -0,0 +1,41 @@ + 0]; + while (rand(0, 1)) { + $a['k'] = $a['k'] + 1; + } +} + +function listItem(): void +{ + [$a] = [source()]; + while (rand(0, 1)) { + $a = $a + 1; + } +} + +function preInc(): void +{ + $i = 0; + while (rand(0, 1)) { + ++$i; + } +} + +function postInc(): void +{ + $i = 0; + while (rand(0, 1)) { + $i++; + } +} + +function preDec(): void +{ + $i = 0; + while (rand(0, 1)) { + --$i; + } +} + +function postDec(): void +{ + $i = 0; + while (rand(0, 1)) { + $i--; + } +} + +/** @param array $items */ +function foreachValueAndKey(array $items): void +{ + foreach ($items as $k => $v) { + while (rand(0, 1)) { + $v = $v + 1; + $k = $k . 'x'; + } + } +} + +function literalOffset(): void +{ + $a = ['x' => 1, 'y' => 2]; + while (rand(0, 1)) { + $a['x'] = $a['x'] + 1; + } + sink($a['y']); +} + +function coveredByNeverReadWrite(): void +{ + $a = source(); + $b = $a + 1; +} + +function coveredThroughChain(): void +{ + $a = source(); + $a = $a + 1; + $a = $a + 1; +} + +function coveredThroughNestedAssignment(): void +{ + $c = source(); + $a = $b = $c + 1; +} + +function coveredThroughLiteralItem(): void +{ + $v = source(); + $a = ['x' => $v, 'y' => 2]; + sink($a['y']); +} diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-offset-overwrite.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-offset-overwrite.php new file mode 100644 index 00000000000..4c933e2ba28 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-offset-overwrite.php @@ -0,0 +1,19 @@ + 1, 'y' => 2]; // unused offset 'x' of $a + sink($a['y']); +} + +function literalFullyRead(): void +{ + $a = ['x' => 1, 'y' => 2]; + sink($a); +} + +function literalNeverRead(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused $a +} + +function literalMissingOffsetRead(): void +{ + $a = ['x' => 1]; // unused offset 'x' of $a + sink($a['y'] ?? null); +} + +function literalListPartiallyRead(): void +{ + $a = [1, 2, 3]; // unused offset 1 of $a, offset 2 of $a + sink($a[0]); +} + +function literalMultiLine(): void +{ + $a = [ + 'x' => 1, + 'y' => 2, // unused offset 'y' of $a + 'z' => 3, + ]; + sink($a['x']); + sink($a['z']); +} + +function literalOffsetOverwritten(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'x' of $a + $a['x'] = 3; + sink($a); +} + +function literalDynamicOffsetRead(int $i): void +{ + $a = [1, 2]; + sink($a[$i]); +} + +function literalIterated(): void +{ + $a = ['x' => 1]; + foreach ($a as $v) { + sink($v); + } +} + +function literalCopiedThenOffsetRead(): void +{ + $a = ['x' => 1, 'y' => 2]; + $b = $a; + sink($b['x']); +} + +function literalIssetIsOffsetRead(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'y' of $a + if (isset($a['x'])) { + sink(1); + } +} + +function literalEmptyIsOffsetRead(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'y' of $a + if (empty($a['x'])) { + sink(1); + } +} + +function literalPassedToFunction(): void +{ + $a = ['x' => 1]; + sink(count($a)); +} + +function literalNestedOffsetRead(): void +{ + $a = ['x' => ['y' => 1], 'z' => 2]; // unused offset 'z' of $a + sink($a['x']['y']); +} + +/** @param array $in */ +function literalWithSpread(array $in): void +{ + $a = [...$in, 'x' => 1]; // unused offset 'x' of $a + sink($a['y'] ?? null); +} + +function literalWithUnknownKey(string $k): void +{ + $a = [$k => 1, 'x' => 2]; + sink($a['x']); +} + +function literalNumericStringKey(): void +{ + $a = ['1' => 'a', 2 => 'b']; // unused offset 2 of $a + sink($a[1]); +} + +function literalImplicitIndexAfterExplicit(): void +{ + $a = [5 => 'a', 'b', 'c']; // unused offset 6 of $a + sink($a[5]); + sink($a[7]); +} + +function literalOffsetUnset(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'x' of $a + unset($a['x']); + sink($a); +} + +function literalItemValueFlow(): void +{ + $v = source(); // unused $v + $a = ['x' => $v, 'y' => 2]; // unused offset 'x' of $a + sink($a['y']); +} + +function literalInTernary(): void +{ + $a = cond() ? ['x' => 1] : ['x' => 2, 'y' => 3]; // unused offset 'y' of $a + sink($a['x']); +} + +function literalReturnedFromOffsetRead(): int +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'y' of $a + return $a['x']; +} + +function literalOffsetReadInLoop(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'y' of $a + while (cond()) { + sink($a['x']); + } +} + +function literalAssignedToOffset(): void +{ + $a = []; + $a['k'] = ['x' => 1, 'y' => 2]; + sink($a['k']['x']); +} + +function dimWriteUnread(): void +{ + $a = []; + $a['x'] = 1; // unused $a['x'] +} + +function dimWriteRead(): void +{ + $a = []; + $a['x'] = 1; + sink($a['x']); +} + +function dimWriteWholeRead(): void +{ + $a = []; + $a['x'] = 1; + sink($a); +} + +function dimWriteOverwritten(): void +{ + $a = []; + $a['x'] = 1; // unused $a['x'] + $a['x'] = 2; + sink($a); +} + +function dimWriteOtherOffsetRead(): void +{ + $a = []; + $a['x'] = 1; // unused $a['x'] + $a['y'] = 2; + sink($a['y']); +} + +function dimWriteDynamicKey(int $i): void +{ + $a = []; + $a[$i] = 1; + sink($a['x'] ?? null); +} + +function dimWriteDynamicKeyUnread(int $i): void +{ + $a = []; + $a[$i] = 1; // unused $a[$i] +} + +function dimWriteAppendThenOffsetRead(): void +{ + $a = []; + $a[] = 1; + sink($a[0]); +} + +function dimWriteAppendUnread(): void +{ + $a = []; + $a[] = 1; // unused $a[] +} + +function dimWriteNested(): void +{ + $a = []; + $a['x']['y'] = 1; + sink($a['x']); +} + +function dimWriteNestedUnread(): void +{ + $a = []; + $a['x']['y'] = 1; // unused $a['x']['y'] +} + +function dimWriteNestedExtendsLiteralOffset(): void +{ + $a = ['x' => ['y' => 1]]; + $a['x']['z'] = 2; + sink($a); +} + +function dimWriteNestedExtendsEarlierDimWrite(): void +{ + $a = []; + $a['x'] = ['y' => 1]; + $a['x']['z'] = 2; + sink($a); +} + +function dimWriteReplacesLiteralOffset(): void +{ + $a = ['x' => 1]; // unused offset 'x' of $a + $a['x'] = 2; + sink($a['x']); +} + +function dimWriteReadModifyWrite(): void +{ + $a = ['x' => 'a']; + $a['x'] .= 'b'; + sink($a); +} + +function dimWriteReadModifyWriteUnread(): void +{ + $a = ['x' => 'a']; + $a['x'] .= 'b'; // unused $a['x'] +} + +function dimWriteCoalesceAssign(): void +{ + $a = []; + $a['x'] ??= 1; + sink($a); +} + +function dimWriteIncrement(): void +{ + $a = ['n' => 0]; + $a['n']++; + sink($a['n']); +} + +function dimWriteIncrementUnread(): void +{ + $a = ['n' => 0]; + $a['n']++; // unused $a['n'] +} + +function stringOffsetWrite(): void +{ + $s = 'abc'; + $s[0] = 'x'; + sink($s); +} + +function stringOffsetWriteUnread(): void +{ + $s = 'abc'; + $s[0] = 'x'; // unused $s[0] +} + +function stringOffsetRead(): void +{ + $s = 'abc'; + sink($s[0]); +} + +function dimWriteOnArrayAccessIsNotASite(\ArrayAccess $o): void +{ + $o['x'] = 1; +} + +/** @param mixed $m */ +function dimWriteOnMixedIsNotASite($m): void +{ + $m['x'] = 1; +} + +function dimWriteWithoutInit(): void +{ + $a['x'] = 1; + sink($a); +} + +function dimWriteWithoutInitUnread(): void +{ + $a['x'] = 1; // unused $a['x'] +} + +function dimWriteInLoop(): void +{ + $a = []; + foreach ([1, 2] as $v) { + $a[$v] = $v; + } + sink($a); +} + +function dimWriteInLoopOffsetReadAfter(): void +{ + $a = []; + while (cond()) { + $a['x'] = 1; + } + sink($a['x'] ?? null); +} + +function dimWriteListTargets(): void +{ + $a = []; + [$a['x'], $a['y']] = [1, 2]; // unused $a['x'], $a['y'] +} + +function dimWriteListTargetsRead(): void +{ + $a = []; + [$a['x'], $a['y']] = [1, 2]; + sink($a); +} + +function dimWriteForeachTarget(): void +{ + $a = []; + foreach ([1, 2] as $a['x']) { // unused $a['x'] + } +} + +function dimWriteOnParameter(array $p): void +{ + $p['x'] = 1; // unused $p['x'] +} + +function dimWriteOnParameterReturned(array $p): array +{ + $p['x'] = 1; + + return $p; +} + +function dimWriteThenWholeOverwrite(): void +{ + $a = []; + $a['x'] = 1; // unused $a['x'] + $a = []; + sink($a); +} + +function dimWriteInBranchRead(): void +{ + $a = []; + if (cond()) { + $a['x'] = 1; + } + sink($a['x'] ?? null); +} + +function dimWriteInBranchUnread(): void +{ + $a = []; + if (cond()) { + $a['x'] = 1; // unused $a['x'] + } + sink($a['y'] ?? null); +} + +function offsetPassedByReference(): void +{ + $a = ['x' => [2, 1]]; + sort($a['x']); + sink($a); +} + +function unsetVariable(): void +{ + $a = 1; // unused $a + unset($a); +} + +function unsetThenReassign(): void +{ + $a = 1; // unused $a + unset($a); + $a = 2; + sink($a); +} + +function unsetAfterRead(): void +{ + $a = 1; + sink($a); + unset($a); +} + +function unsetInBranch(): void +{ + $a = 1; + if (cond()) { + unset($a); + } + sink($a ?? null); +} + +function unsetOffsetThenWholeRead(): void +{ + $a = ['x' => 1, 'y' => 2]; // unused offset 'x' of $a + unset($a['x']); + sink($a); +} + +function unsetDimWriteOffset(): void +{ + $a = []; + $a['x'] = 1; // unused $a['x'] + unset($a['x']); + sink($a); +} + +function unsetDynamicOffset(int $i): void +{ + $a = [1, 2]; + unset($a[$i]); + sink($a); +} + +function unsetNestedOffset(): void +{ + $a = ['x' => ['y' => 1]]; + unset($a['x']['y']); + sink($a); +} + +function unsetOffsetSelectedByOffsetRead(): void +{ + $a = ['x' => 1]; + unset($a[$a['x']]); + sink($a); +} + +function foreachOverOffset(): void +{ + $a = ['x' => [1, 2], 'y' => 3]; // unused offset 'y' of $a + foreach ($a['x'] as $v) { + sink($v); + } +} + +function offsetReadThroughCompact(): array +{ + $a = ['x' => 1, 'y' => 2]; + + return compact('a'); +} + +function offsetsOfVariableVariableAreRead(): void +{ + $a = ['x' => 1, 'y' => 2]; + $name = 'a'; + sink($$name); +} + +function literalOffsetsWhenTargetIsOffsetWriteAreNotTracked(): void +{ + $a = []; + $a['k'] = ['x' => 1, 'y' => 2]; + sink($a['k']['x']); +} + +function callOnOffset(): void +{ + $a = ['x' => static fn (): int => 1, 'y' => 2]; // unused offset 'y' of $a + sink($a['x']()); +} + +function dimWriteCoalesceAssignReadInLoop(): void +{ + $cache = []; + foreach (['a', 'b'] as $k) { + $v = $cache[$k] ??= source(); + sink($v); + } +} + +function dimWriteCoalesceAssignResultConsumed(): void +{ + $cache = []; + while (cond()) { + if (($cache['x'] ??= cond()) === true) { + sink(1); + } + } +} + +function dimWriteCoalesceAssignValueFlow(): void +{ + $cache = []; + $v = $cache['x'] ??= source(); // unused $v, $cache['x'] +} + +function literalItemReadIntoUnused(): void +{ + $a = ['x' => 1, 'y' => 2]; // offset 'x' of $a only flows into values that are never used + $b = $a['x']; // unused $b + sink($a['y']); +} diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php index af4f6c58c73..b34c76a790f 100644 --- a/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php @@ -101,3 +101,19 @@ public function unreadArgumentVariable(): void } } + +function matchArmFlow(): void +{ + $a = source(); // unused $a + $b = match (true) { // unused $b + default => $a, + }; +} + +function matchConditionIsSink(): void +{ + $a = source(); + $b = match ($a) { // unused $b + default => 1, + }; +} diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php new file mode 100644 index 00000000000..9dbb51fc618 --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php @@ -0,0 +1,509 @@ + 3) { + break; + } + } +} + +function comparisonResultUnused(): void +{ + $a = source(); // unused $a + $ok = $a === 1; // unused $ok +} + +function comparisonResultSunk(): void +{ + $a = source(); + $ok = $a === 1; + sink($ok); +} + +function ternaryBranchFlow(): void +{ + $a = source(); // unused $a + $b = cond() ? $a : 0; // unused $b +} + +function ternaryBranchFlowSunk(): void +{ + $a = source(); + $b = cond() ? $a : 0; + sink($b); +} + +function ternaryConditionIsSink(): void +{ + $a = source(); + $b = $a ? 1 : 0; // unused $b +} + +function shortTernaryConditionIsSink(): void +{ + $a = source(); + $b = $a ?: 1; // unused $b +} + +function arrayLiteralFlow(): void +{ + $a = source(); // unused $a + $arr = [$a, 'k' => $a]; // unused $arr +} + +function arrayLiteralFlowSunk(): void +{ + $a = source(); + $arr = [$a]; + sink($arr); +} + +function castFlow(): void +{ + $a = source(); // unused $a + $b = (int) $a; // unused $b + $c = (string) $a; // unused $c +} + +function unaryFlow(): void +{ + $a = 1; // unused $a + $b = -$a; // unused $b + $c = !$a; // unused $c + $d = ~$a; // unused $d +} + +function interpolationFlow(): void +{ + $a = 'x'; // unused $a + $b = "v: $a"; // unused $b +} + +function errorSuppressFlow(): void +{ + $a = source(); // unused $a + $b = @$a; // unused $b +} + +function coalesceRightSideFlow(): void +{ + $d = 1; // unused $d + $b = source() ?? $d; // unused $b +} + +function coalesceLeftSideIsSink(): void +{ + $a = source(); + $b = $a ?? 1; // unused $b +} + +function booleanOperandsAreSinks(): void +{ + $a = cond(); + $c = cond(); + $b = $a && $c; // unused $b + $d = $a || $c; // unused $d +} + +function callArgumentIsSink(): void +{ + $a = 'x'; + $b = strlen($a); // unused $b +} + +function methodReceiverIsSink(): void +{ + $o = new \ArrayObject([]); + $b = $o->count(); // unused $b +} + +function propertyReceiverIsSink(): void +{ + $o = new \stdClass(); + $b = $o->foo ?? null; // unused $b +} + +function newArgumentIsSink(): void +{ + $a = []; + $b = new \ArrayObject($a); // unused $b +} + +function closureUseIsSink(): void +{ + $a = 1; + $f = function () use ($a): int { // unused $f + return $a; + }; +} + +function arrowFunctionCaptureIsSink(): void +{ + $a = 1; + $f = fn (): int => $a; // unused $f +} + +function instanceofIsSink(): void +{ + $o = source(); + $b = $o instanceof \stdClass; // unused $b +} + +function printIsSink(): void +{ + $a = 'x'; + $b = print $a; // unused $b +} + +function yieldIsSink(): iterable +{ + $a = 1; + $b = yield $a; // unused $b +} + +function cloneIsSink(): void +{ + $o = new \stdClass(); + $b = clone $o; // unused $b +} + +function issetIsSink(): void +{ + $a = source(); + $b = isset($a); // unused $b +} + +function emptyIsSink(): void +{ + $a = source(); + $b = empty($a); // unused $b +} + +function nestedAssignOuterSunk(): void +{ + $a = $b = source(); // unused $b + sink($a); +} + +function nestedAssignInnerSunk(): void +{ + $a = $b = source(); // unused $a + sink($b); +} + +function nestedAssignValueFlowsToOuter(): void +{ + $c = source(); + $a = $b = $c + 1; // unused $b + sink($a); +} + +function nestedAssignAllUnused(): void +{ + $c = source(); // unused $c + $a = $b = $c + 1; // unused $a, $b +} + +function readInFlowThenSunk(): void +{ + $a = 1; + $b = $a + 1; // unused $b + sink($a); +} + +function flowThenOverwrite(): void +{ + $a = 1; // unused $a + $b = $a + 1; // unused $b + $a = 2; + sink($a); +} + +function flowIntoArrayOffsetWrite(): void +{ + $v = source(); // unused $v + $a = []; + $a['x'] = $v; // unused $a['x'] +} + +function flowIntoArrayOffsetWriteSunk(): void +{ + $v = source(); + $a = []; + $a['x'] = $v; + sink($a); +} + +function offsetReadFlow(): void +{ + $a = ['k' => 1]; // unused $a + $b = $a['k']; // unused $b +} + +function offsetReadFlowSunk(): void +{ + $a = ['k' => 1]; + $b = $a['k']; + sink($b); +} + +function dimensionFlow(): void +{ + $i = 0; // unused $i + $a = source(); // unused $a + $b = $a[$i]; // unused $b +} + +function parameterInFlow(int $p): void +{ + $a = $p + 1; // unused $a +} + +function flowThroughSeveralVariables(): void +{ + $a = 1; // unused $a + $b = $a * 2; // unused $b + $c = $b + $a; // unused $c + $d = $c; // unused $d +} + +function flowThroughSeveralVariablesSunk(): void +{ + $a = 1; + $b = $a * 2; + $c = $b + $a; + $d = $c; + sink($d); +} + +function flowInLoopSunkAfter(): void +{ + $s = ''; + foreach ([1, 2] as $v) { + $s = $s . $v; + } + sink($s); +} + +function flowInLoopNeverSunk(): void +{ + $s = ''; // unused $s + foreach ([1, 2] as $v) { // unused $v + $s = $s . $v; // unused $s + } +} + +function chainFedByFunctionCallIsStillUnused(): void +{ + $a = source(); // unused $a + $a = $a + 1; // unused $a +} + +function chainKeptAliveByReference(): void +{ + $a = 1; + $r = &$a; + $a = $a + 1; + sink($r); +} + +function compactReadsChain(): array +{ + $a = 1; + $a = $a + 1; + return compact('a'); +} + +function returnIsSink(): int +{ + $a = 1; + $a = $a + 1; + return $a; +} + +function throwIsSink(): void +{ + $m = 'x'; + $m = $m . 'y'; + throw new \RuntimeException($m); +} + +function echoIsSink(): void +{ + $a = 1; + $a = $a + 1; + echo $a; +} + +function ifConditionIsSink(): void +{ + $a = 1; + $a = $a + 1; + if ($a > 1) { + sink(1); + } +} + +function propertyWriteIsSink(\stdClass $o): void +{ + $a = 1; + $a = $a + 1; + $o->x = $a; +} + +function staticPropertyWriteIsSink(): void +{ + $a = 1; + $a = $a + 1; + Holder::$x = $a; +} + +class Holder +{ + + /** @var int */ + public static $x = 0; + +} + +function assignOpValueFlowsToOuter(): void +{ + $s = ''; + $x = ($s .= 'a'); // unused $s + sink($x); +} + +function assignOpValueFlowsToOuterUnused(): void +{ + $s = ''; // unused $s + $x = ($s .= 'a'); // unused $x, $s +} + +function superglobalOffsetIsSink(): void +{ + $value = source(); + $_GET['value'] = $value; +} + +function superglobalAssignmentIsSink(): void +{ + $value = source(); + $_POST = ['value' => $value]; +} + +function flowIntoAliasedVariable(): void +{ + $reasons = []; + $callback = function () use (&$reasons): array { + return $reasons; + }; + foreach ([1, 2] as $reason) { + $reasons[] = $reason; + } + sink($callback); +} diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable.php index 9cb48ef6fd1..c107d27e2cd 100644 --- a/tests/PHPStan/Rules/DeadCode/data/unused-variable.php +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable.php @@ -247,8 +247,8 @@ function arrayBuildReturned(): array function arrayBuildUnused(): void { $x = []; - $x[] = 1; - $x['k'] = 2; // unused $x + $x[] = 1; // unused $x[] + $x['k'] = 2; // unused $x['k'] } function stringAppendReturned(): string @@ -260,13 +260,13 @@ function stringAppendReturned(): string function stringAppendUnused(): void { - $s = 'a'; + $s = 'a'; // unused $s $s .= 'b'; // unused $s } function unsetAfterWrite(): void { - $a = 1; // known false negative: unset() walks the variable as a read + $a = 1; // unused $a unset($a); } @@ -698,15 +698,15 @@ function cloneRead(): object function selfReferentialChain(): void { - // the Psalm layer will also report the first write; phase 1 sees it read by the second - $a = 5; + // the first write only feeds the second, which is never used + $a = 5; // unused $a $a = $a + 1; // unused $a } function articleExample(): void { - // Psalm reports every write of $b; phase 1 sees each read by the self-chain - $b = $a = 0; + // every write of $b only feeds the self-chain - Psalm's example + $b = $a = 0; // unused $b while (cond()) { if (cond() && cond()) { $a = 5; @@ -716,7 +716,7 @@ function articleExample(): void continue; } $a = $a + 1; - $b = $b + 1; + $b = $b + 1; // unused $b } sink($a); } From 66ff8bb4f6b959c09a8bce519327631ec7de3744 Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 11 Sep 2026 11:32:05 +0200 Subject: [PATCH 2/4] Report constant-offset assignments that are redundant `$a = [1, 2]; $a[0] = 1;` assigns a value the offset already holds, the same finding the rule already reports for whole variables. The check walks the constant dimensions of the target down the array type in both type flavours; append, dynamic keys, by-reference items and PhpDoc-only certainty are left alone. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018CZ79ZBiMt4KWgU3uy4Y4j --- src/Analyser/ExprHandler/AssignHandler.php | 44 +++++++--- src/Rules/DeadCode/UnusedVariableRule.php | 13 ++- .../Rules/DeadCode/UnusedVariableRuleTest.php | 17 ++++ .../unused-variable-redundant-offsets.php | 87 +++++++++++++++++++ 4 files changed, 147 insertions(+), 14 deletions(-) create mode 100644 tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-offsets.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index 83d87f9ff4a..884720e2d8a 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -259,8 +259,8 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } } - $redundantType = $expr instanceof Assign && $expr->var instanceof Variable && is_string($expr->var->name) - ? self::redundant($assignedExprResult, $expr->var->name) + $redundantType = $expr instanceof Assign + ? self::redundant($assignedExprResult, $expr->var, $storage) : null; $variableFlow = VariableFlow::sequence( VariableFlowBuilder::targetRead($expr->var, $storage, false), @@ -2500,21 +2500,45 @@ private static function hasArrayReference(Expr\Array_ $array): bool return false; } - private static function redundant(ExpressionResult $rhs, string $name): ?Type + private static function redundant(ExpressionResult $rhs, Expr $target, ExpressionResultStorage $storage): ?Type { - $scope = $rhs->getScope(); - if (!$scope->hasVariableType($name)->yes()) { - return null; + $dimensions = []; + while ($target instanceof ArrayDimFetch) { + if ($target->dim === null) { + return null; + } + $dimResult = $storage->findExpressionResult($target->dim); + if ($dimResult === null || count($dimResult->getType()->toArrayKey()->getConstantScalarValues()) !== 1) { + return null; + } + $dimensions[] = $dimResult; + $target = $target->var; } - $values = $scope->getVariableType($name)->getFiniteTypes(); - if (count($values) !== 1 || !$values[0]->equals($rhs->getType())) { + if (!$target instanceof Variable || !is_string($target->name)) { return null; } + $name = $target->name; + $scope = $rhs->getScope(); $nativeScope = $scope->doNotTreatPhpDocTypesAsCertain(); - if (!$nativeScope->hasVariableType($name)->yes()) { + if (!$scope->hasVariableType($name)->yes() || !$nativeScope->hasVariableType($name)->yes()) { + return null; + } + $type = $scope->getVariableType($name); + $nativeType = $nativeScope->getVariableType($name); + foreach (array_reverse($dimensions) as $dimension) { + $offset = $dimension->getType()->toArrayKey(); + $nativeOffset = $dimension->getNativeType()->toArrayKey(); + if (!$type->isArray()->yes() || !$nativeType->isArray()->yes() || !$type->hasOffsetValueType($offset)->yes() || !$nativeType->hasOffsetValueType($nativeOffset)->yes()) { + return null; + } + $type = $type->getOffsetValueType($offset); + $nativeType = $nativeType->getOffsetValueType($nativeOffset); + } + $values = $type->getFiniteTypes(); + if (count($values) !== 1 || !$values[0]->equals($rhs->getType())) { return null; } - $nativeValues = $nativeScope->getVariableType($name)->getFiniteTypes(); + $nativeValues = $nativeType->getFiniteTypes(); return count($nativeValues) === 1 && $nativeValues[0]->equals($rhs->getNativeType()) ? $rhs->getType() : null; } diff --git a/src/Rules/DeadCode/UnusedVariableRule.php b/src/Rules/DeadCode/UnusedVariableRule.php index b47e4448c8e..311d893247a 100644 --- a/src/Rules/DeadCode/UnusedVariableRule.php +++ b/src/Rules/DeadCode/UnusedVariableRule.php @@ -71,14 +71,19 @@ public function processNode(Node $node, Scope $scope): array } $redundantType = $node->getRedundantType($write); - if ($redundantType !== null && $node->isUsed($write)) { + if ($redundantType !== null && ($write->isOffsetWrite() || $node->isUsed($write))) { + $target = $write->getNode(); + if (!$target instanceof Node\Expr) { + throw new ShouldNotHappenException(); + } + $description = $write->isOffsetWrite() ? 'Offset ' . $this->exprPrinter->printExpr($target) : 'Variable $' . $name; $errors[] = RuleErrorBuilder::message(sprintf( - 'Variable $%s is assigned value %s but it already has that value.', - $name, + '%s is assigned value %s but it already has that value.', + $description, $redundantType->describe(VerbosityLevel::value()), )) ->identifier('assign.redundant') - ->line($write->getNode()->getStartLine()) + ->line($target->getStartLine()) ->build(); continue; } diff --git a/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php index 0c6661162f4..56b194035a5 100644 --- a/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php @@ -441,6 +441,23 @@ public function testOffsets(): void ]); } + public function testRedundantOffsets(): void + { + $redundant = []; + foreach ($this->gatherAnalyserErrors([__DIR__ . '/data/unused-variable-redundant-offsets.php']) as $error) { + if ($error->getIdentifier() !== 'assign.redundant') { + continue; + } + $redundant[] = [$error->getMessage(), $error->getLine()]; + } + $this->assertSame([ + ['Offset $a[0] is assigned value 1 but it already has that value.', 8], + ['Offset $a[0] is assigned value 1 but it already has that value.', 14], + ['Offset $a[\'x\'][\'y\'] is assigned value 1 but it already has that value.', 21], + ['Offset $a[\'0\'] is assigned value 1 but it already has that value.', 28], + ], $redundant); + } + public function testDynamicOffsetOverwritten(): void { $errors = $this->gatherAnalyserErrors([__DIR__ . '/data/unused-variable-offset-overwrite.php']); diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-offsets.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-offsets.php new file mode 100644 index 00000000000..50c0565437a --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-offsets.php @@ -0,0 +1,87 @@ + ['y' => 1]]; + $a['x']['y'] = 1; + return $a; +} + +function coercedKey(): array +{ + $a = [1]; + $a['0'] = 1; + return $a; +} + +function differentValue(): array +{ + $a = [1, 2]; + $a[0] = 3; + return $a; +} + +function newOffset(): array +{ + $a = [1]; + $a[1] = 1; + return $a; +} + +/** @param array{0?: 1} $a */ +function optionalOffset(array $a): array +{ + $a[0] = 1; + return $a; +} + +/** @param array{0: 1} $a */ +function phpDocOnly(array $a): array +{ + $a[0] = 1; + return $a; +} + +function append(): array +{ + $a = [1]; + $a[] = 1; + return $a; +} + +function dynamicKey(int $i): array +{ + $a = [1, 2]; + $a[$i] = 1; + return $a; +} + +function reference(): array +{ + $x = 1; + $a = [&$x]; + $a[0] = 1; + return $a; +} + +function stringOffset(): string +{ + $a = '12'; + $a[0] = '1'; + return $a; +} From a3fca1979c27df9b353cabdc3fab7d27a5a37aad Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 11 Sep 2026 11:33:03 +0200 Subject: [PATCH 3/4] Report parameters and captures that only flow into unused values UnusedParametersCheck and UnusedClosureUsesRule ask whether the bound value is used rather than merely read, so a parameter or by-value capture whose value only feeds unused values is reported as "... has a parameter $x that only flows into values that are never used." under function/method/constructor.unusedParameterFlow and closure.unusedUseFlow. The constructor-parameter and closure-use rules predate the value-flow analysis, so they report this class only when the unusedParameters bleeding-edge toggle is on; the function and method parameter rules exist only under that toggle. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018CZ79ZBiMt4KWgU3uy4Y4j --- .../UnusedConstructorParametersRule.php | 12 ++-- src/Rules/Functions/UnusedClosureUsesRule.php | 19 +++++- .../UnusedFunctionParametersRule.php | 2 + .../Methods/UnusedMethodParametersRule.php | 14 +++-- src/Rules/UnusedParametersCheck.php | 22 ++++++- .../UnusedConstructorParametersRuleTest.php | 21 ++++++- ...used-constructor-parameters-value-flow.php | 21 +++++++ .../Functions/UnusedClosureUsesRuleTest.php | 21 ++++++- .../UnusedFunctionParametersRuleTest.php | 9 +++ .../data/unused-input-value-flow.php | 59 +++++++++++++++++++ .../UnusedMethodParametersRuleTest.php | 9 +++ .../unused-method-parameters-value-flow.php | 40 +++++++++++++ 12 files changed, 232 insertions(+), 17 deletions(-) create mode 100644 tests/PHPStan/Rules/Classes/data/unused-constructor-parameters-value-flow.php create mode 100644 tests/PHPStan/Rules/Functions/data/unused-input-value-flow.php create mode 100644 tests/PHPStan/Rules/Methods/data/unused-method-parameters-value-flow.php diff --git a/src/Rules/Classes/UnusedConstructorParametersRule.php b/src/Rules/Classes/UnusedConstructorParametersRule.php index 4fb30320936..50b03342544 100644 --- a/src/Rules/Classes/UnusedConstructorParametersRule.php +++ b/src/Rules/Classes/UnusedConstructorParametersRule.php @@ -24,6 +24,8 @@ public function __construct( private UnusedParametersCheck $check, #[AutowiredParameter(ref: '%featureToggles.reportPreciseLineForUnusedFunctionParameter%')] private bool $reportExactLine, + #[AutowiredParameter(ref: '%featureToggles.unusedParameters%')] + private bool $reportUnusedFlow, ) { } @@ -67,21 +69,23 @@ public function processNode(Node $node, Scope $scope): array } } - $message = sprintf( - 'Constructor of class %s has an unused parameter $%%s.', + $constructorDescription = sprintf( + 'Constructor of class %s', SprintfHelper::escapeFormatString($classReflection->getDisplayName()), ); if ($classReflection->isAnonymous()) { - $message = 'Constructor of an anonymous class has an unused parameter $%s.'; + $constructorDescription = 'Constructor of an anonymous class'; } return $this->check->getUnusedParameterErrors( $node, $method, $originalNode->params, - $message, + sprintf('%s has an unused parameter $%%s.', $constructorDescription), 'constructor.unusedParameter', $this->reportExactLine, + $this->reportUnusedFlow ? sprintf('%s has a parameter $%%s that only flows into values that are never used.', $constructorDescription) : null, + $this->reportUnusedFlow ? 'constructor.unusedParameterFlow' : null, ); } diff --git a/src/Rules/Functions/UnusedClosureUsesRule.php b/src/Rules/Functions/UnusedClosureUsesRule.php index 05e3509a4f7..6da0e6b2340 100644 --- a/src/Rules/Functions/UnusedClosureUsesRule.php +++ b/src/Rules/Functions/UnusedClosureUsesRule.php @@ -22,6 +22,8 @@ final class UnusedClosureUsesRule implements Rule public function __construct( #[AutowiredParameter(ref: '%featureToggles.reportPreciseLineForUnusedFunctionParameter%')] private bool $reportExactLine, + #[AutowiredParameter(ref: '%featureToggles.unusedParameters%')] + private bool $reportUnusedFlow, ) { } @@ -46,20 +48,31 @@ public function processNode(Node $node, Scope $scope): array if (!is_string($use->var->name)) { continue; } + $message = 'Anonymous function has an unused use $%s.'; + $identifier = 'closure.unusedUse'; $write = $node->getWriteForNode($use->var); if ($write !== null) { // a by-value use imports a value - it is unused unless that // value is read on some path (overwriting it first is not a use) - if ($node->isRead($write) || $node->areAllVariableNamesReferenced()) { + if ($node->isUsed($write) || $node->areAllVariableNamesReferenced()) { continue; } + if ($node->isRead($write)) { + // read, but only into values that never reach a sink - a + // newer finding than the rule, so bleeding edge only + if (!$this->reportUnusedFlow) { + continue; + } + $message = 'Anonymous function has a use $%s that only flows into values that are never used.'; + $identifier = 'closure.unusedUseFlow'; + } } elseif ($node->isVariableReferenced($use->var->name)) { // a by-ref use aliases the outer variable - any mention counts continue; } - $errorBuilder = RuleErrorBuilder::message(sprintf('Anonymous function has an unused use $%s.', $use->var->name)) - ->identifier('closure.unusedUse'); + $errorBuilder = RuleErrorBuilder::message(sprintf($message, $use->var->name)) + ->identifier($identifier); if ($this->reportExactLine) { $errorBuilder->line($use->var->getStartLine()); } diff --git a/src/Rules/Functions/UnusedFunctionParametersRule.php b/src/Rules/Functions/UnusedFunctionParametersRule.php index ebd8b68d082..4311eee0b97 100644 --- a/src/Rules/Functions/UnusedFunctionParametersRule.php +++ b/src/Rules/Functions/UnusedFunctionParametersRule.php @@ -50,6 +50,8 @@ public function processNode(Node $node, Scope $scope): array sprintf('Function %s() has an unused parameter $%%s.', SprintfHelper::escapeFormatString($function->getName())), 'function.unusedParameter', true, + sprintf('Function %s() has a parameter $%%s that only flows into values that are never used.', SprintfHelper::escapeFormatString($function->getName())), + 'function.unusedParameterFlow', ); } diff --git a/src/Rules/Methods/UnusedMethodParametersRule.php b/src/Rules/Methods/UnusedMethodParametersRule.php index 6444f69da0d..4e7bad0ccec 100644 --- a/src/Rules/Methods/UnusedMethodParametersRule.php +++ b/src/Rules/Methods/UnusedMethodParametersRule.php @@ -57,17 +57,21 @@ public function processNode(Node $node, Scope $scope): array return []; } + $methodDescription = sprintf( + '%s::%s()', + SprintfHelper::escapeFormatString($scope->getClassReflection()->getDisplayName()), + SprintfHelper::escapeFormatString($originalNode->name->toString()), + ); + return $this->check->getUnusedParameterErrors( $node, $method, $originalNode->params, - sprintf( - 'Method %s::%s() has an unused parameter $%%s.', - SprintfHelper::escapeFormatString($scope->getClassReflection()->getDisplayName()), - SprintfHelper::escapeFormatString($originalNode->name->toString()), - ), + sprintf('Method %s has an unused parameter $%%s.', $methodDescription), 'method.unusedParameter', true, + sprintf('Method %s has a parameter $%%s that only flows into values that are never used.', $methodDescription), + 'method.unusedParameterFlow', ); } diff --git a/src/Rules/UnusedParametersCheck.php b/src/Rules/UnusedParametersCheck.php index fdc456b6a74..21f5204e41c 100644 --- a/src/Rules/UnusedParametersCheck.php +++ b/src/Rules/UnusedParametersCheck.php @@ -21,6 +21,10 @@ * that value is read on some path (overwriting it first is not a use); * func_get_args() observes every parameter's original value; a by-ref * parameter gives the caller the variable, so any mention counts. + * + * A parameter whose value is read, but only into values that never reach a + * sink, is a separate finding - callers opt into it, because the rules that + * predate the value-flow analysis report it only under bleeding edge. */ #[AutowiredService] final class UnusedParametersCheck @@ -29,6 +33,7 @@ final class UnusedParametersCheck /** * @param Param[] $parameters * @param 'constructor.unusedParameter'|'function.unusedParameter'|'method.unusedParameter' $identifier + * @param 'constructor.unusedParameterFlow'|'function.unusedParameterFlow'|'method.unusedParameterFlow'|null $unusedFlowIdentifier null = do not report parameters whose value only flows into values that are never used * @return list */ public function getUnusedParameterErrors( @@ -38,6 +43,8 @@ public function getUnusedParameterErrors( string $unusedParameterMessage, string $identifier, bool $reportExactLine, + ?string $unusedFlowMessage = null, + ?string $unusedFlowIdentifier = null, ): array { if ($node->isOpaque()) { @@ -57,17 +64,26 @@ public function getUnusedParameterErrors( if (isset($contractParameterNames[$parameter->var->name])) { continue; } + $message = $unusedParameterMessage; + $errorIdentifier = $identifier; $write = $node->getWriteForNode($parameter->var); if ($write !== null) { - if ($node->isRead($write) || $node->areAllVariableNamesReferenced()) { + if ($node->isUsed($write) || $node->areAllVariableNamesReferenced()) { continue; } + if ($node->isRead($write)) { + if ($unusedFlowMessage === null || $unusedFlowIdentifier === null) { + continue; + } + $message = $unusedFlowMessage; + $errorIdentifier = $unusedFlowIdentifier; + } } elseif ($node->isVariableReferenced($parameter->var->name)) { continue; } - $errorBuilder = RuleErrorBuilder::message(sprintf($unusedParameterMessage, $parameter->var->name)) - ->identifier($identifier); + $errorBuilder = RuleErrorBuilder::message(sprintf($message, $parameter->var->name)) + ->identifier($errorIdentifier); if ($reportExactLine) { $errorBuilder->line($parameter->var->getStartLine()); } diff --git a/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php index 63644db9729..0bbe1359e87 100644 --- a/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php +++ b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php @@ -14,9 +14,11 @@ class UnusedConstructorParametersRuleTest extends RuleTestCase private bool $reportExactLine = true; + private bool $reportUnusedFlow = true; + protected function getRule(): Rule { - return new UnusedConstructorParametersRule(self::getContainer()->getByType(UnusedParametersCheck::class), $this->reportExactLine); + return new UnusedConstructorParametersRule(self::getContainer()->getByType(UnusedParametersCheck::class), $this->reportExactLine, $this->reportUnusedFlow); } public function testUnusedConstructorParametersNoExactLine(): void @@ -92,4 +94,21 @@ public function testResolvedDynamicUsages(): void ]); } + public function testValueFlow(): void + { + $this->analyse([__DIR__ . '/data/unused-constructor-parameters-value-flow.php'], [ + ['Constructor of class UnusedConstructorParametersValueFlow\Foo has a parameter $covered that only flows into values that are never used.', 10], + ['Constructor of class UnusedConstructorParametersValueFlow\Foo has a parameter $input that only flows into values that are never used.', 10], + ['Constructor of class UnusedConstructorParametersValueFlow\Foo has an unused parameter $overwritten.', 10], + ]); + } + + public function testValueFlowWithoutBleedingEdge(): void + { + $this->reportUnusedFlow = false; + $this->analyse([__DIR__ . '/data/unused-constructor-parameters-value-flow.php'], [ + ['Constructor of class UnusedConstructorParametersValueFlow\Foo has an unused parameter $overwritten.', 10], + ]); + } + } diff --git a/tests/PHPStan/Rules/Classes/data/unused-constructor-parameters-value-flow.php b/tests/PHPStan/Rules/Classes/data/unused-constructor-parameters-value-flow.php new file mode 100644 index 00000000000..f4d7631308c --- /dev/null +++ b/tests/PHPStan/Rules/Classes/data/unused-constructor-parameters-value-flow.php @@ -0,0 +1,21 @@ +value = $used + 1; + $overwritten = 1; + $this->value += $overwritten; + } + +} diff --git a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php index b29ffd478e5..4c07ef6da9e 100644 --- a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php @@ -11,9 +11,11 @@ class UnusedClosureUsesRuleTest extends RuleTestCase { + private bool $reportUnusedFlow = true; + protected function getRule(): Rule { - return new UnusedClosureUsesRule(true); + return new UnusedClosureUsesRule(true, $this->reportUnusedFlow); } public function testCapturesAssignedThroughVariableVariables(): void @@ -64,4 +66,21 @@ public function testReferenceCapturedInSkippedCatch(): void ]); } + public function testValueFlow(): void + { + $this->analyse([__DIR__ . '/data/unused-input-value-flow.php'], [ + ['Anonymous function has a use $input that only flows into values that are never used.', 25], + ['Anonymous function has a use $input that only flows into values that are never used.', 34], + ['Anonymous function has an unused use $input.', 49], + ]); + } + + public function testValueFlowWithoutBleedingEdge(): void + { + $this->reportUnusedFlow = false; + $this->analyse([__DIR__ . '/data/unused-input-value-flow.php'], [ + ['Anonymous function has an unused use $input.', 49], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php index 3bd7b6b7b36..ed5243e2a79 100644 --- a/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php @@ -43,4 +43,13 @@ public function testRule(): void ]); } + public function testValueFlow(): void + { + $this->analyse([__DIR__ . '/data/unused-input-value-flow.php'], [ + ['Function UnusedInputValueFlow\unusedParameter() has a parameter $input that only flows into values that are never used.', 5], + ['Function UnusedInputValueFlow\coveredParameter() has a parameter $input that only flows into values that are never used.', 12], + ['Function UnusedInputValueFlow\overwrittenParameter() has an unused parameter $input.', 55], + ]); + } + } diff --git a/tests/PHPStan/Rules/Functions/data/unused-input-value-flow.php b/tests/PHPStan/Rules/Functions/data/unused-input-value-flow.php new file mode 100644 index 00000000000..2e9080a2947 --- /dev/null +++ b/tests/PHPStan/Rules/Functions/data/unused-input-value-flow.php @@ -0,0 +1,59 @@ +analyse([__DIR__ . '/data/unused-method-parameters-value-flow.php'], [ + ['Method UnusedMethodParametersValueFlow\Foo::unusedParameter() has a parameter $input that only flows into values that are never used.', 8], + ['Method UnusedMethodParametersValueFlow\Foo::coveredParameter() has a parameter $input that only flows into values that are never used.', 15], + ['Method UnusedMethodParametersValueFlow\Foo::overwrittenParameter() has an unused parameter $input.', 26], + ]); + } + } diff --git a/tests/PHPStan/Rules/Methods/data/unused-method-parameters-value-flow.php b/tests/PHPStan/Rules/Methods/data/unused-method-parameters-value-flow.php new file mode 100644 index 00000000000..35b6a44d39f --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/unused-method-parameters-value-flow.php @@ -0,0 +1,40 @@ +unusedParameter(1); + $this->coveredParameter(1); + $this->usedParameter(1); + $this->overwrittenParameter(1); + } + +} From 59b243fab985f1a0cf850c1f458b6ae165ea664b Mon Sep 17 00:00:00 2001 From: Ondrej Mirtes Date: Fri, 11 Sep 2026 11:34:45 +0200 Subject: [PATCH 4/4] Do not report values flowing into a write that is reported as never read A write whose value only flows into unused values was reported even when one of those values is itself never read - `$copy = $e` already reports $copy, reporting $e as well is noise. The liveness resolver now walks the dependency edges backwards from every never-read write and marks the writes feeding it as covered; the rules skip covered flow reports. A chain that only feeds itself, like Psalm's `$b = $b + 1` loop, has no never-read write to point at and stays reported at every write. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018CZ79ZBiMt4KWgU3uy4Y4j --- src/Analyser/VariableLivenessResolver.php | 42 ++++++++++++++- src/Node/VariableWritesNode.php | 11 ++++ src/Rules/DeadCode/UnusedVariableRule.php | 2 +- src/Rules/Functions/UnusedClosureUsesRule.php | 2 +- src/Rules/UnusedParametersCheck.php | 2 +- .../Analyser/AnalyserIntegrationTest.php | 14 ++--- .../UnusedConstructorParametersRuleTest.php | 1 - .../Rules/DeadCode/UnusedVariableRuleTest.php | 41 -------------- .../DeadCode/data/unused-variable-offsets.php | 4 +- .../DeadCode/data/unused-variable-php8.php | 2 +- .../data/unused-variable-value-flow.php | 54 +++++++++---------- .../Rules/DeadCode/data/unused-variable.php | 4 +- .../Functions/UnusedClosureUsesRuleTest.php | 1 - .../UnusedFunctionParametersRuleTest.php | 1 - .../UnusedMethodParametersRuleTest.php | 1 - 15 files changed, 91 insertions(+), 91 deletions(-) diff --git a/src/Analyser/VariableLivenessResolver.php b/src/Analyser/VariableLivenessResolver.php index 789e61388c1..48e20c70e66 100644 --- a/src/Analyser/VariableLivenessResolver.php +++ b/src/Analyser/VariableLivenessResolver.php @@ -71,6 +71,9 @@ final class VariableLivenessResolver /** @var array> */ private array $literalItems = []; + /** @var array */ + private array $coveredIds = []; + /** @var array */ private array $allReadKeys = []; @@ -119,9 +122,10 @@ public static function resolve(Node\FunctionLike $function, ?VariableFlow $flow) $self->compileAccesses(); $self->liveBefore($body, [], new VariableFlowContext([])); $self->resolveDependencies(); + $self->resolveCoverage(); } - return new VariableWritesNode($function, array_values($self->writes), $self->observedIds + $self->readIds, $self->readIds, $self->readNames, $self->redundantTypes, $self->mentionedNames, $self->escapedNames, $self->opaque, $self->allNamesMentioned); + return new VariableWritesNode($function, array_values($self->writes), $self->observedIds + $self->readIds, $self->readIds, $self->coveredIds, $self->readNames, $self->redundantTypes, $self->mentionedNames, $self->escapedNames, $self->opaque, $self->allNamesMentioned); } private function collect(?VariableFlow $flow, bool $dead = false): void @@ -495,4 +499,40 @@ private function resolveDependencies(): void } } + /** + * A write whose value flows only into unused writes is covered when one of + * them is never read at all: reporting that write already points at the + * dead chain. Walks the dependency edges backwards from every never-read + * write; a chain that only feeds itself (Psalm's `$b = $b + 1` loop) has + * no such write and stays uncovered. + */ + private function resolveCoverage(): void + { + $stack = []; + foreach (array_keys($this->writes) as $id) { + if (isset($this->observedIds[$id]) || isset($this->readIds[$id])) { + continue; + } + + $stack[] = $id; + } + $visited = []; + while ($stack !== []) { + $id = array_pop($stack); + if (isset($visited[$id])) { + continue; + } + $visited[$id] = true; + $sources = array_keys($this->dependencies[$id] ?? []); + foreach (array_keys($this->inputCopies[$id] ?? []) as $copied) { + // the inputs of $copied flow into $id as well + $sources = [...$sources, ...array_keys($this->dependencies[$copied] ?? [])]; + } + foreach ($sources as $source) { + $this->coveredIds[$source] = true; + $stack[] = $source; + } + } + } + } diff --git a/src/Node/VariableWritesNode.php b/src/Node/VariableWritesNode.php index 5568b291b1b..f27a72f4af3 100644 --- a/src/Node/VariableWritesNode.php +++ b/src/Node/VariableWritesNode.php @@ -24,6 +24,7 @@ final class VariableWritesNode extends NodeAbstract implements VirtualNode * @param list $writes * @param array $readWriteIds * @param array $usedWriteIds + * @param array $coveredWriteIds * @param array $readVariableNames * @param array $redundantWriteTypes * @param array $referencedVariableNames @@ -34,6 +35,7 @@ public function __construct( private array $writes, private array $readWriteIds, private array $usedWriteIds, + private array $coveredWriteIds, private array $readVariableNames, private array $redundantWriteTypes, private array $referencedVariableNames, @@ -88,6 +90,15 @@ public function isUsed(VariableWrite $write): bool return isset($this->usedWriteIds[$write->getId()]); } + /** + * Whether the value flows into a write that is never read at all - that + * write is the one to report, this one only feeds it. + */ + public function flowsIntoNeverReadWrite(VariableWrite $write): bool + { + return isset($this->coveredWriteIds[$write->getId()]); + } + /** Whether some path from the write reaches a read of the written value. */ public function isRead(VariableWrite $write): bool { diff --git a/src/Rules/DeadCode/UnusedVariableRule.php b/src/Rules/DeadCode/UnusedVariableRule.php index 311d893247a..d602e54371c 100644 --- a/src/Rules/DeadCode/UnusedVariableRule.php +++ b/src/Rules/DeadCode/UnusedVariableRule.php @@ -88,7 +88,7 @@ public function processNode(Node $node, Scope $scope): array continue; } - if ($node->isUsed($write)) { + if ($node->isUsed($write) || $node->flowsIntoNeverReadWrite($write)) { continue; } diff --git a/src/Rules/Functions/UnusedClosureUsesRule.php b/src/Rules/Functions/UnusedClosureUsesRule.php index 6da0e6b2340..66aa25f6f58 100644 --- a/src/Rules/Functions/UnusedClosureUsesRule.php +++ b/src/Rules/Functions/UnusedClosureUsesRule.php @@ -60,7 +60,7 @@ public function processNode(Node $node, Scope $scope): array if ($node->isRead($write)) { // read, but only into values that never reach a sink - a // newer finding than the rule, so bleeding edge only - if (!$this->reportUnusedFlow) { + if (!$this->reportUnusedFlow || $node->flowsIntoNeverReadWrite($write)) { continue; } $message = 'Anonymous function has a use $%s that only flows into values that are never used.'; diff --git a/src/Rules/UnusedParametersCheck.php b/src/Rules/UnusedParametersCheck.php index 21f5204e41c..37b4791a1cb 100644 --- a/src/Rules/UnusedParametersCheck.php +++ b/src/Rules/UnusedParametersCheck.php @@ -72,7 +72,7 @@ public function getUnusedParameterErrors( continue; } if ($node->isRead($write)) { - if ($unusedFlowMessage === null || $unusedFlowIdentifier === null) { + if ($unusedFlowMessage === null || $unusedFlowIdentifier === null || $node->flowsIntoNeverReadWrite($write)) { continue; } $message = $unusedFlowMessage; diff --git a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index e4713e2eb76..5879bb66aac 100644 --- a/tests/PHPStan/Analyser/AnalyserIntegrationTest.php +++ b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php @@ -1407,16 +1407,10 @@ public function testBug10847(): void { $errors = $this->runAnalyse(__DIR__ . '/data/bug-10847.php'); // The loop appends to $overloads instead of $processedOverloads. - $this->assertCount(3, $errors); - $this->assertSame('Foreach key variable $args only flows into values that are never used.', $errors[0]->getMessage()); - $this->assertSame('foreach.unusedKeyFlow', $errors[0]->getIdentifier()); - $this->assertSame(158, $errors[0]->getLine()); - $this->assertSame('Foreach value variable $callback only flows into values that are never used.', $errors[1]->getMessage()); - $this->assertSame('foreach.unusedValueFlow', $errors[1]->getIdentifier()); - $this->assertSame(158, $errors[1]->getLine()); - $this->assertSame('Value assigned to $overloads[] is never read.', $errors[2]->getMessage()); - $this->assertSame('assign.unused', $errors[2]->getIdentifier()); - $this->assertSame(159, $errors[2]->getLine()); + $this->assertCount(1, $errors); + $this->assertSame('Value assigned to $overloads[] is never read.', $errors[0]->getMessage()); + $this->assertSame('assign.unused', $errors[0]->getIdentifier()); + $this->assertSame(159, $errors[0]->getLine()); } #[RequiresPhp('>= 8.1.0')] diff --git a/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php index 0bbe1359e87..8a06e07784f 100644 --- a/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php +++ b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php @@ -97,7 +97,6 @@ public function testResolvedDynamicUsages(): void public function testValueFlow(): void { $this->analyse([__DIR__ . '/data/unused-constructor-parameters-value-flow.php'], [ - ['Constructor of class UnusedConstructorParametersValueFlow\Foo has a parameter $covered that only flows into values that are never used.', 10], ['Constructor of class UnusedConstructorParametersValueFlow\Foo has a parameter $input that only flows into values that are never used.', 10], ['Constructor of class UnusedConstructorParametersValueFlow\Foo has an unused parameter $overwritten.', 10], ]); diff --git a/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php index 56b194035a5..0df8d42932a 100644 --- a/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php +++ b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php @@ -85,7 +85,6 @@ public function testRule(): void ['Variable $x is never read.', 223], ['Value assigned to $x[] is never read.', 250], ['Value assigned to $x[\'k\'] is never read.', 251], - ['Value assigned to variable $s only flows into values that are never used.', 263], ['Value assigned to variable $s is never read.', 264], ['Variable $a is never read.', 269], ['Variable $a is never read.', 276], @@ -96,7 +95,6 @@ public function testRule(): void ['Variable $b is never read.', 614], ['Variable $a is never read.', 632], ['Variable $a is never read.', 637], - ['Value assigned to variable $a only flows into values that are never used.', 702], ['Value assigned to variable $a is never read.', 703], ['Value assigned to variable $b only flows into values that are never used.', 709], ['Value assigned to variable $b only flows into values that are never used.', 719], @@ -117,7 +115,6 @@ public function testPhp8(): void $this->analyse([__DIR__ . '/data/unused-variable-php8.php'], [ ['Catch variable $e is never read.', 23], ['Value assigned to variable $nightsFrom is never read.', 98], - ['Value assigned to variable $a only flows into values that are never used.', 107], ['Variable $b is never read.', 108], ['Variable $b is never read.', 116], ]); @@ -160,7 +157,6 @@ public function testBug14258(): void public function testBug12012(): void { $this->analyse([__DIR__ . '/data/bug-12012.php'], [ - ['Value assigned to variable $s1 only flows into values that are never used.', 9], ['Value assigned to variable $s1 is never read.', 10], ['Value assigned to variable $s1 is never read.', 12], ]); @@ -301,42 +297,26 @@ public function testBroadCatchesIncludeImplicitThrows(): void public function testValueFlow(): void { $this->analyse([__DIR__ . '/data/unused-variable-value-flow.php'], [ - ['Value assigned to variable $a only flows into values that are never used.', 27], ['Value assigned to variable $a is never read.', 28], - ['Value assigned to variable $s only flows into values that are never used.', 40], - ['Value assigned to variable $s only flows into values that are never used.', 41], ['Value assigned to variable $s is never read.', 42], - ['Value assigned to variable $i only flows into values that are never used.', 54], - ['Value of variable $i after ++ only flows into values that are never used.', 55], - ['Value of variable $i after ++ only flows into values that are never used.', 56], - ['Value of variable $i after -- only flows into values that are never used.', 57], ['Value of variable $i after -- is never read.', 58], ['Value of variable $i after ++ is never read.', 71], - ['Value assigned to variable $i only flows into values that are never used.', 76], ['Value of variable $i after ++ is never read.', 77], ['Variable $j is never read.', 77], ['Value assigned to variable $n only flows into values that are never used.', 98], ['Value assigned to variable $n only flows into values that are never used.', 100], - ['Value assigned to variable $a only flows into values that are never used.', 117], ['Variable $ok is never read.', 118], - ['Value assigned to variable $a only flows into values that are never used.', 130], ['Variable $b is never read.', 131], ['Variable $b is never read.', 144], ['Variable $b is never read.', 150], - ['Value assigned to variable $a only flows into values that are never used.', 155], ['Variable $arr is never read.', 156], - ['Value assigned to variable $a only flows into values that are never used.', 168], ['Variable $b is never read.', 169], ['Variable $c is never read.', 170], - ['Value assigned to variable $a only flows into values that are never used.', 175], ['Variable $b is never read.', 176], ['Variable $c is never read.', 177], ['Variable $d is never read.', 178], - ['Value assigned to variable $a only flows into values that are never used.', 183], ['Variable $b is never read.', 184], - ['Value assigned to variable $a only flows into values that are never used.', 189], ['Variable $b is never read.', 190], - ['Value assigned to variable $d only flows into values that are never used.', 195], ['Variable $b is never read.', 196], ['Variable $b is never read.', 202], ['Variable $b is never read.', 209], @@ -356,31 +336,20 @@ public function testValueFlow(): void ['Variable $b is never read.', 289], ['Variable $a is never read.', 295], ['Variable $b is never read.', 302], - ['Value assigned to variable $c only flows into values that are never used.', 308], ['Variable $b is never read.', 309], ['Variable $a is never read.', 309], ['Variable $b is never read.', 315], - ['Value assigned to variable $a only flows into values that are never used.', 321], ['Variable $b is never read.', 322], - ['Value assigned to variable $v only flows into values that are never used.', 329], ['Value assigned to $a[\'x\'] is never read.', 331], - ['Value assigned to variable $a only flows into values that are never used.', 344], ['Variable $b is never read.', 345], - ['Value assigned to variable $i only flows into values that are never used.', 357], - ['Value assigned to variable $a only flows into values that are never used.', 358], ['Variable $b is never read.', 359], ['Variable $a is never read.', 364], - ['Value assigned to variable $a only flows into values that are never used.', 369], - ['Value assigned to variable $b only flows into values that are never used.', 370], - ['Value assigned to variable $c only flows into values that are never used.', 371], ['Variable $d is never read.', 372], ['Value assigned to variable $s only flows into values that are never used.', 395], ['Foreach value variable $v only flows into values that are never used.', 396], ['Value assigned to variable $s only flows into values that are never used.', 397], - ['Value assigned to variable $a only flows into values that are never used.', 403], ['Value assigned to variable $a is never read.', 404], ['Value assigned to variable $s is never read.', 477], - ['Value assigned to variable $s only flows into values that are never used.', 483], ['Value assigned to variable $s is never read.', 484], ['Variable $x is never read.', 484], ]); @@ -403,7 +372,6 @@ public function testOffsets(): void ['Offset 2 of array assigned to variable $a is never read.', 136], ['Offset 6 of array assigned to variable $a is never read.', 142], ['Offset \'x\' of array assigned to variable $a is never read.', 149], - ['Value assigned to variable $v only flows into values that are never used.', 156], ['Offset \'x\' of array assigned to variable $a is never read.', 157], ['Offset \'y\' of array assigned to variable $a is never read.', 163], ['Offset \'y\' of array assigned to variable $a is never read.', 169], @@ -415,9 +383,7 @@ public function testOffsets(): void ['Value assigned to $a[] is never read.', 247], ['Value assigned to $a[\'x\'][\'y\'] is never read.', 260], ['Offset \'x\' of array assigned to variable $a is never read.', 280], - ['Value assigned to variable $a only flows into values that are never used.', 294], ['Value assigned to $a[\'x\'] is never read.', 295], - ['Value assigned to variable $a only flows into values that are never used.', 314], ['Value of $a[\'n\'] after ++ is never read.', 315], ['Value assigned to $s[0] is never read.', 328], ['Value assigned to $a[\'x\'] is never read.', 356], @@ -436,7 +402,6 @@ public function testOffsets(): void ['Value assigned to $cache[\'x\'] is never read.', 565], ['Variable $v is never read.', 565], - ['Offset \'x\' of array assigned to variable $a only flows into values that are never used.', 570], ['Variable $b is never read.', 571], ]); } @@ -499,15 +464,10 @@ public function testFlowMessages(): void ['Value assigned to variable $k only flows into values that are never used.', 86], ['Offset \'x\' of array assigned to variable $a only flows into values that are never used.', 93], ['Value assigned to $a[\'x\'] only flows into values that are never used.', 95], - ['Value assigned to variable $a only flows into values that are never used.', 102], ['Variable $b is never read.', 103], - ['Value assigned to variable $a only flows into values that are never used.', 108], - ['Value assigned to variable $a only flows into values that are never used.', 109], ['Value assigned to variable $a is never read.', 110], - ['Value assigned to variable $c only flows into values that are never used.', 115], ['Variable $a is never read.', 116], ['Variable $b is never read.', 116], - ['Value assigned to variable $v only flows into values that are never used.', 121], ['Offset \'x\' of array assigned to variable $a is never read.', 122], ]); } @@ -518,7 +478,6 @@ public function testFlowMessagesCatch(): void $this->analyse([__DIR__ . '/data/unused-variable-flow-messages-catch.php'], [ ['Catch variable $e only flows into values that are never used.', 15], ['Value assigned to variable $e only flows into values that are never used.', 17], - ['Catch variable $e only flows into values that are never used.', 26], ['Variable $copy is never read.', 27], ]); } diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-offsets.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-offsets.php index 150e244b083..283c507b954 100644 --- a/tests/PHPStan/Rules/DeadCode/data/unused-variable-offsets.php +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-offsets.php @@ -153,7 +153,7 @@ function literalOffsetUnset(): void function literalItemValueFlow(): void { - $v = source(); // unused $v + $v = source(); $a = ['x' => $v, 'y' => 2]; // unused offset 'x' of $a sink($a['y']); } @@ -567,7 +567,7 @@ function dimWriteCoalesceAssignValueFlow(): void function literalItemReadIntoUnused(): void { - $a = ['x' => 1, 'y' => 2]; // offset 'x' of $a only flows into values that are never used + $a = ['x' => 1, 'y' => 2]; $b = $a['x']; // unused $b sink($a['y']); } diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php index b34c76a790f..f4575e631ac 100644 --- a/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-php8.php @@ -104,7 +104,7 @@ public function unreadArgumentVariable(): void function matchArmFlow(): void { - $a = source(); // unused $a + $a = source(); $b = match (true) { // unused $b default => $a, }; diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php index 9dbb51fc618..8a4d80450ce 100644 --- a/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php @@ -24,7 +24,7 @@ function sink($v): void function chainNeverSunk(): void { - $a = 5; // unused $a + $a = 5; $a = $a + 1; // unused $a } @@ -37,8 +37,8 @@ function chainSunk(): void function concatChain(): void { - $s = 'a'; // unused $s - $s .= 'b'; // unused $s + $s = 'a'; + $s .= 'b'; $s = $s . 'c'; // unused $s } @@ -51,10 +51,10 @@ function concatChainSunk(): string function incrementChain(): void { - $i = 0; // unused $i - $i++; // unused $i - ++$i; // unused $i - $i--; // unused $i + $i = 0; + $i++; + ++$i; + $i--; --$i; // unused $i } @@ -73,7 +73,7 @@ function incrementConsumedBySink(): void function incrementIntoAssignment(): void { - $i = 0; // unused $i + $i = 0; $j = $i++; // unused $j, $i } @@ -114,7 +114,7 @@ function loopAccumulatorSunkByBreak(): void function comparisonResultUnused(): void { - $a = source(); // unused $a + $a = source(); $ok = $a === 1; // unused $ok } @@ -127,7 +127,7 @@ function comparisonResultSunk(): void function ternaryBranchFlow(): void { - $a = source(); // unused $a + $a = source(); $b = cond() ? $a : 0; // unused $b } @@ -152,7 +152,7 @@ function shortTernaryConditionIsSink(): void function arrayLiteralFlow(): void { - $a = source(); // unused $a + $a = source(); $arr = [$a, 'k' => $a]; // unused $arr } @@ -165,14 +165,14 @@ function arrayLiteralFlowSunk(): void function castFlow(): void { - $a = source(); // unused $a + $a = source(); $b = (int) $a; // unused $b $c = (string) $a; // unused $c } function unaryFlow(): void { - $a = 1; // unused $a + $a = 1; $b = -$a; // unused $b $c = !$a; // unused $c $d = ~$a; // unused $d @@ -180,19 +180,19 @@ function unaryFlow(): void function interpolationFlow(): void { - $a = 'x'; // unused $a + $a = 'x'; $b = "v: $a"; // unused $b } function errorSuppressFlow(): void { - $a = source(); // unused $a + $a = source(); $b = @$a; // unused $b } function coalesceRightSideFlow(): void { - $d = 1; // unused $d + $d = 1; $b = source() ?? $d; // unused $b } @@ -305,7 +305,7 @@ function nestedAssignValueFlowsToOuter(): void function nestedAssignAllUnused(): void { - $c = source(); // unused $c + $c = source(); $a = $b = $c + 1; // unused $a, $b } @@ -318,7 +318,7 @@ function readInFlowThenSunk(): void function flowThenOverwrite(): void { - $a = 1; // unused $a + $a = 1; $b = $a + 1; // unused $b $a = 2; sink($a); @@ -326,7 +326,7 @@ function flowThenOverwrite(): void function flowIntoArrayOffsetWrite(): void { - $v = source(); // unused $v + $v = source(); $a = []; $a['x'] = $v; // unused $a['x'] } @@ -341,7 +341,7 @@ function flowIntoArrayOffsetWriteSunk(): void function offsetReadFlow(): void { - $a = ['k' => 1]; // unused $a + $a = ['k' => 1]; $b = $a['k']; // unused $b } @@ -354,8 +354,8 @@ function offsetReadFlowSunk(): void function dimensionFlow(): void { - $i = 0; // unused $i - $a = source(); // unused $a + $i = 0; + $a = source(); $b = $a[$i]; // unused $b } @@ -366,9 +366,9 @@ function parameterInFlow(int $p): void function flowThroughSeveralVariables(): void { - $a = 1; // unused $a - $b = $a * 2; // unused $b - $c = $b + $a; // unused $c + $a = 1; + $b = $a * 2; + $c = $b + $a; $d = $c; // unused $d } @@ -400,7 +400,7 @@ function flowInLoopNeverSunk(): void function chainFedByFunctionCallIsStillUnused(): void { - $a = source(); // unused $a + $a = source(); $a = $a + 1; // unused $a } @@ -480,7 +480,7 @@ function assignOpValueFlowsToOuter(): void function assignOpValueFlowsToOuterUnused(): void { - $s = ''; // unused $s + $s = ''; $x = ($s .= 'a'); // unused $x, $s } diff --git a/tests/PHPStan/Rules/DeadCode/data/unused-variable.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable.php index c107d27e2cd..434611fddd6 100644 --- a/tests/PHPStan/Rules/DeadCode/data/unused-variable.php +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable.php @@ -260,7 +260,7 @@ function stringAppendReturned(): string function stringAppendUnused(): void { - $s = 'a'; // unused $s + $s = 'a'; $s .= 'b'; // unused $s } @@ -699,7 +699,7 @@ function cloneRead(): object function selfReferentialChain(): void { // the first write only feeds the second, which is never used - $a = 5; // unused $a + $a = 5; $a = $a + 1; // unused $a } diff --git a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php index 4c07ef6da9e..73cd781fb87 100644 --- a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php @@ -70,7 +70,6 @@ public function testValueFlow(): void { $this->analyse([__DIR__ . '/data/unused-input-value-flow.php'], [ ['Anonymous function has a use $input that only flows into values that are never used.', 25], - ['Anonymous function has a use $input that only flows into values that are never used.', 34], ['Anonymous function has an unused use $input.', 49], ]); } diff --git a/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php b/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php index ed5243e2a79..6db6bb57b77 100644 --- a/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php @@ -47,7 +47,6 @@ public function testValueFlow(): void { $this->analyse([__DIR__ . '/data/unused-input-value-flow.php'], [ ['Function UnusedInputValueFlow\unusedParameter() has a parameter $input that only flows into values that are never used.', 5], - ['Function UnusedInputValueFlow\coveredParameter() has a parameter $input that only flows into values that are never used.', 12], ['Function UnusedInputValueFlow\overwrittenParameter() has an unused parameter $input.', 55], ]); } diff --git a/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php b/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php index eb2e6daeaa7..75994dcbe65 100644 --- a/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php +++ b/tests/PHPStan/Rules/Methods/UnusedMethodParametersRuleTest.php @@ -58,7 +58,6 @@ public function testValueFlow(): void { $this->analyse([__DIR__ . '/data/unused-method-parameters-value-flow.php'], [ ['Method UnusedMethodParametersValueFlow\Foo::unusedParameter() has a parameter $input that only flows into values that are never used.', 8], - ['Method UnusedMethodParametersValueFlow\Foo::coveredParameter() has a parameter $input that only flows into values that are never used.', 15], ['Method UnusedMethodParametersValueFlow\Foo::overwrittenParameter() has an unused parameter $input.', 26], ]); }