Skip to content

Report variable writes whose value never reaches a use, per array offset #6334

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ondrejmirtes merged 4 commits into from
Sep 11, 2026
+2,649 −349
Merged

Report variable writes whose value never reaches a use, per array offset #6334

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a323b66
Track unused values and array offset assignments
ondrejmirtes Sep 11, 2026
66ff8bb
Report constant-offset assignments that are redundant
ondrejmirtes Sep 11, 2026
a3fca19
Report parameters and captures that only flow into unused values
ondrejmirtes Sep 11, 2026
59b243f
Do not report values flowing into a write that is reported as never read
ondrejmirtes Sep 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/Analyser/ExprHandler/ArrayDimFetchHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,6 +35,7 @@
use PHPStan\Type\Type;
use PHPStan\Type\TypeCombinator;
use function array_merge;
use function is_string;

/**
* @implements ExprHandler<ArrayDimFetch>
Expand All @@ -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);
}
Expand All @@ -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(),
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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);
}

}
36 changes: 34 additions & 2 deletions src/Analyser/ExprHandler/ArrayHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

/**
Expand Down Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
63 changes: 45 additions & 18 deletions src/Analyser/ExprHandler/AssignHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -257,12 +259,13 @@ 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),
$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)) {
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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());
Expand Down Expand Up @@ -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();
}

Expand All @@ -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();
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -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());
Expand Down Expand Up @@ -2497,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;
}

Expand Down
7 changes: 5 additions & 2 deletions src/Analyser/ExprHandler/AssignOpHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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,
);

Expand Down
4 changes: 2 additions & 2 deletions src/Analyser/ExprHandler/BinaryOpHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/ExprHandler/BitwiseNotHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/ExprHandler/BooleanAndHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion src/Analyser/ExprHandler/BooleanNotHandler.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Loading
Loading