diff --git a/src/Analyser/ExprHandler/ArrayDimFetchHandler.php b/src/Analyser/ExprHandler/ArrayDimFetchHandler.php index b26e3d3781..f5e26d83d8 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 f02c51c187..f49935fe6e 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 6069449452..884720e2d8 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(); @@ -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)) { @@ -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()); @@ -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; } diff --git a/src/Analyser/ExprHandler/AssignOpHandler.php b/src/Analyser/ExprHandler/AssignOpHandler.php index e587f91956..7d3e732c67 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 1884ce0e3a..3312bc3082 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 e8bae880b9..5087008231 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 cf828e121e..73d476d940 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 6709df2595..730f39d931 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 185d32d54b..c648430b0e 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 af5242b29b..fe4a960419 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 9521ca1348..392b10b984 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 c3680a3f4f..798ffeb4a4 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 3e937b5659..6a155f322d 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 0239e02e61..eb6cbc6cd8 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 a096541a3d..ed81367258 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 e323b36068..6c97d45b54 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 257f52dc87..8336883977 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 10d39d39c6..c646ea565c 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 beba3f0b13..ba7b208166 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 29c0218a4f..743e215c29 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 1cbca23fd9..254d6303cb 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 8622c66c1b..4d41b7db60 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 6955241a9a..da509bccfa 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 e970bf128c..e86cdf715f 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 44d90a4ab7..2c798f9068 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 14bba2a5d5..5cdb42fb95 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 40ebd3b60d..b73bd61fd6 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 124eedab19..c3020377a3 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 0000000000..ab60b383e2 --- /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,43 @@ 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 $coveredIds = []; + + /** @var array */ + private array $allReadKeys = []; + private bool $opaque = false; + private bool $readsAllVariables = false; + private bool $allNamesMentioned = false; private bool $returnsByReference = false; @@ -78,27 +119,35 @@ 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(); + $self->resolveCoverage(); } - 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->coveredIds, $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 +156,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 +198,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 +250,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 +357,182 @@ 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]; + } + } + } + + /** + * 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/Analyser/VariableWriteOffset.php b/src/Analyser/VariableWriteOffset.php new file mode 100644 index 0000000000..4c7c5b428b --- /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 69e2b95eff..2efcb502a1 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 38593e6b2c..f27a72f4af 100644 --- a/src/Node/VariableWritesNode.php +++ b/src/Node/VariableWritesNode.php @@ -23,6 +23,8 @@ 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 @@ -32,6 +34,8 @@ public function __construct( private Node\FunctionLike $functionLike, private array $writes, private array $readWriteIds, + private array $usedWriteIds, + private array $coveredWriteIds, private array $readVariableNames, private array $redundantWriteTypes, private array $referencedVariableNames, @@ -63,7 +67,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 +84,22 @@ public function areAllVariableNamesReferenced(): bool return $this->allVariableNamesReferenced; } + /** 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. + * 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 { return isset($this->readWriteIds[$write->getId()]); diff --git a/src/Rules/Classes/UnusedConstructorParametersRule.php b/src/Rules/Classes/UnusedConstructorParametersRule.php index 4fb3032093..50b0334254 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/DeadCode/UnusedVariableRule.php b/src/Rules/DeadCode/UnusedVariableRule.php index 21a92958cb..d602e54371 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,58 @@ 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 && ($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( + '%s is assigned value %s but it already has that value.', + $description, + $redundantType->describe(VerbosityLevel::value()), + )) + ->identifier('assign.redundant') + ->line($target->getStartLine()) ->build(); continue; } - $redundantType = $node->getRedundantType($write); - if ($redundantType === null) { + if ($node->isUsed($write) || $node->flowsIntoNeverReadWrite($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 +131,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 +166,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 +179,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/src/Rules/Functions/UnusedClosureUsesRule.php b/src/Rules/Functions/UnusedClosureUsesRule.php index 05e3509a4f..66aa25f6f5 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 || $node->flowsIntoNeverReadWrite($write)) { + 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 ebd8b68d08..4311eee0b9 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 6444f69da0..4e7bad0cce 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 fdc456b6a7..37b4791a1c 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 || $node->flowsIntoNeverReadWrite($write)) { + 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/Analyser/AnalyserIntegrationTest.php b/tests/PHPStan/Analyser/AnalyserIntegrationTest.php index c97f0898c4..5879bb66aa 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,12 @@ 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(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/Levels/data/arrayDimFetches-4.json b/tests/PHPStan/Levels/data/arrayDimFetches-4.json new file mode 100644 index 0000000000..db22ecc77d --- /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/Classes/UnusedConstructorParametersRuleTest.php b/tests/PHPStan/Rules/Classes/UnusedConstructorParametersRuleTest.php index 63644db972..8a06e07784 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,20 @@ 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 $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 0000000000..f4d7631308 --- /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/DeadCode/UnusedVariableRuleTest.php b/tests/PHPStan/Rules/DeadCode/UnusedVariableRuleTest.php index fc030d3a27..0df8d42932 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,45 @@ 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 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 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 +113,10 @@ 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], + ['Variable $b is never read.', 108], + ['Variable $b is never read.', 116], ]); } @@ -261,14 +157,8 @@ 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 is never read.', 10], + ['Value assigned to variable $s1 is never read.', 12], ]); } @@ -320,38 +210,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 +294,192 @@ public function testBroadCatchesIncludeImplicitThrows(): void ]); } + public function testValueFlow(): void + { + $this->analyse([__DIR__ . '/data/unused-variable-value-flow.php'], [ + ['Value assigned to variable $a is never read.', 28], + ['Value assigned to variable $s is never read.', 42], + ['Value of variable $i after -- is never read.', 58], + ['Value of variable $i after ++ is never read.', 71], + ['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], + ['Variable $ok is never read.', 118], + ['Variable $b is never read.', 131], + ['Variable $b is never read.', 144], + ['Variable $b is never read.', 150], + ['Variable $arr is never read.', 156], + ['Variable $b is never read.', 169], + ['Variable $c is never read.', 170], + ['Variable $b is never read.', 176], + ['Variable $c is never read.', 177], + ['Variable $d is never read.', 178], + ['Variable $b is never read.', 184], + ['Variable $b is never read.', 190], + ['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], + ['Variable $b is never read.', 309], + ['Variable $a is never read.', 309], + ['Variable $b is never read.', 315], + ['Variable $b is never read.', 322], + ['Value assigned to $a[\'x\'] is never read.', 331], + ['Variable $b is never read.', 345], + ['Variable $b is never read.', 359], + ['Variable $a is never read.', 364], + ['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 is never read.', 404], + ['Value assigned to variable $s is never read.', 477], + ['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], + ['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 $a[\'x\'] is never read.', 295], + ['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], + + ['Variable $b is never read.', 571], + ]); + } + + 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']); + $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], + ['Variable $b is never read.', 103], + ['Value assigned to variable $a is never read.', 110], + ['Variable $a is never read.', 116], + ['Variable $b is never read.', 116], + ['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], + ['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 0000000000..ddab73aeb4 --- /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 0000000000..4c933e2ba2 --- /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(); + $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]; + $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 af4f6c58c7..f4575e631a 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(); + $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-redundant-offsets.php b/tests/PHPStan/Rules/DeadCode/data/unused-variable-redundant-offsets.php new file mode 100644 index 0000000000..50c0565437 --- /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; +} 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 0000000000..8a4d80450c --- /dev/null +++ b/tests/PHPStan/Rules/DeadCode/data/unused-variable-value-flow.php @@ -0,0 +1,509 @@ + 3) { + break; + } + } +} + +function comparisonResultUnused(): void +{ + $a = source(); + $ok = $a === 1; // unused $ok +} + +function comparisonResultSunk(): void +{ + $a = source(); + $ok = $a === 1; + sink($ok); +} + +function ternaryBranchFlow(): void +{ + $a = source(); + $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(); + $arr = [$a, 'k' => $a]; // unused $arr +} + +function arrayLiteralFlowSunk(): void +{ + $a = source(); + $arr = [$a]; + sink($arr); +} + +function castFlow(): void +{ + $a = source(); + $b = (int) $a; // unused $b + $c = (string) $a; // unused $c +} + +function unaryFlow(): void +{ + $a = 1; + $b = -$a; // unused $b + $c = !$a; // unused $c + $d = ~$a; // unused $d +} + +function interpolationFlow(): void +{ + $a = 'x'; + $b = "v: $a"; // unused $b +} + +function errorSuppressFlow(): void +{ + $a = source(); + $b = @$a; // unused $b +} + +function coalesceRightSideFlow(): void +{ + $d = 1; + $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(); + $a = $b = $c + 1; // unused $a, $b +} + +function readInFlowThenSunk(): void +{ + $a = 1; + $b = $a + 1; // unused $b + sink($a); +} + +function flowThenOverwrite(): void +{ + $a = 1; + $b = $a + 1; // unused $b + $a = 2; + sink($a); +} + +function flowIntoArrayOffsetWrite(): void +{ + $v = source(); + $a = []; + $a['x'] = $v; // unused $a['x'] +} + +function flowIntoArrayOffsetWriteSunk(): void +{ + $v = source(); + $a = []; + $a['x'] = $v; + sink($a); +} + +function offsetReadFlow(): void +{ + $a = ['k' => 1]; + $b = $a['k']; // unused $b +} + +function offsetReadFlowSunk(): void +{ + $a = ['k' => 1]; + $b = $a['k']; + sink($b); +} + +function dimensionFlow(): void +{ + $i = 0; + $a = source(); + $b = $a[$i]; // unused $b +} + +function parameterInFlow(int $p): void +{ + $a = $p + 1; // unused $a +} + +function flowThroughSeveralVariables(): void +{ + $a = 1; + $b = $a * 2; + $c = $b + $a; + $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(); + $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 = ''; + $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 9cb48ef6fd..434611fddd 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 @@ -266,7 +266,7 @@ function stringAppendUnused(): void 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 + // the first write only feeds the second, which is never used $a = 5; $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); } diff --git a/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php b/tests/PHPStan/Rules/Functions/UnusedClosureUsesRuleTest.php index b29ffd478e..73cd781fb8 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,20 @@ 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 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 3bd7b6b7b3..6db6bb57b7 100644 --- a/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php +++ b/tests/PHPStan/Rules/Functions/UnusedFunctionParametersRuleTest.php @@ -43,4 +43,12 @@ 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\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 0000000000..2e9080a294 --- /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::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 0000000000..35b6a44d39 --- /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); + } + +}