From 085abe37af120ab0b5141b4ddeb19be459a09d82 Mon Sep 17 00:00:00 2001 From: phpstan-bot <79867460+phpstan-bot@users.noreply.github.com> Date: Tue, 1 Sep 2026 10:00:53 +0000 Subject: [PATCH] Widen `never` generic type arguments to their template bounds after an object might have been mutated - Added `TemplateTypeHelper::widenNeverTypeArguments()`, which maps a type and replaces `never` type arguments of `GenericObjectType`/`GenericStaticType` with the bounds of the corresponding `@template` tags. - Added `NodeScopeResolver::widenNeverTypeArguments()`, which re-assigns the widened PHPDoc and native types of an expression in the scope. - Called it wherever PHPStan already assumes an object may have been mutated: - `NodeScopeResolver::processArgs()` - arguments of a call with side effects (functions, methods, static methods); also applies to objects nested inside an array argument, which the existing invalidation does not cover. - `MethodCallHandler` - the receiver of a method with side effects and of an unresolved method call. - `AssignHandler` - writing an offset on an `ArrayAccess` object (`$o['k'] = ...`, `$o[] = ...`), which goes through `offsetSet()` and previously left the receiver's type untouched. - Probed and left unchanged: constructor arguments (`new Foo($coll)`), where PHPStan deliberately assumes an unmarked constructor does not mutate its arguments (see `nsrt/impure-constructor.php`); an `@phpstan-impure` constructor does widen. Nullsafe method calls (`$o?->add()`) still lose the widening because `NonNullabilityHelper::revertNonNullability()` restores the receiver's entry type wholesale - the same pre-existing limitation already drops `@phpstan-self-out` there. --- src/Analyser/ExprHandler/AssignHandler.php | 3 + .../ExprHandler/MethodCallHandler.php | 2 + src/Analyser/NodeScopeResolver.php | 32 ++- src/Type/Generic/TemplateTypeHelper.php | 71 ++++++ tests/PHPStan/Analyser/nsrt/bug-15147.php | 226 ++++++++++++++++++ 5 files changed, 330 insertions(+), 4 deletions(-) create mode 100644 tests/PHPStan/Analyser/nsrt/bug-15147.php diff --git a/src/Analyser/ExprHandler/AssignHandler.php b/src/Analyser/ExprHandler/AssignHandler.php index b3de7b4b461..03141dfcb2e 100644 --- a/src/Analyser/ExprHandler/AssignHandler.php +++ b/src/Analyser/ExprHandler/AssignHandler.php @@ -1099,6 +1099,9 @@ public function applyWrite( $scope = $scope->assignInitializedProperty($scope->getType($var->var), $var->name->toString()); } } + + // offsetSet() filled the ArrayAccess object, so its `never` generics no longer hold + $scope = $nodeScopeResolver->widenNeverTypeArguments($scope, $var); } foreach ($additionalExpressions as $k => $additionalExpression) { diff --git a/src/Analyser/ExprHandler/MethodCallHandler.php b/src/Analyser/ExprHandler/MethodCallHandler.php index 8a07d219db7..99420cd4e4f 100644 --- a/src/Analyser/ExprHandler/MethodCallHandler.php +++ b/src/Analyser/ExprHandler/MethodCallHandler.php @@ -176,6 +176,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex if ($methodReflection->getName() === '__construct' || $methodReflection->hasSideEffects()->yes()) { $nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage); $scope = $scope->invalidateExpression($normalizedExpr->var, true, $methodReflection->getDeclaringClass()); + $scope = $nodeScopeResolver->widenNeverTypeArguments($scope, $normalizedExpr->var); } elseif ($this->rememberPossiblyImpureFunctionValues && $methodReflection->hasSideEffects()->maybe() && !$methodReflection->getDeclaringClass()->isBuiltin()) { // the remembered call value and the @phpstan-self-out type are // generic-sensitive: resolve them from the type-driven acceptor @@ -208,6 +209,7 @@ public function processExpr(NodeScopeResolver $nodeScopeResolver, Stmt $stmt, Ex } else { $nodeScopeResolver->callNodeCallback($nodeCallback, new InvalidateExprNode($normalizedExpr->var), $scope, $storage); $scope = $scope->invalidateExpression($normalizedExpr->var, true); + $scope = $nodeScopeResolver->widenNeverTypeArguments($scope, $normalizedExpr->var); $throwPoints[] = InternalThrowPoint::createImplicit($scope, $expr); } if ( diff --git a/src/Analyser/NodeScopeResolver.php b/src/Analyser/NodeScopeResolver.php index 4ed253d91a6..c2af263e785 100644 --- a/src/Analyser/NodeScopeResolver.php +++ b/src/Analyser/NodeScopeResolver.php @@ -80,6 +80,7 @@ use PHPStan\Type\FunctionParameterClosureThisExtension; use PHPStan\Type\FunctionParameterClosureTypeExtension; use PHPStan\Type\FunctionParameterOutTypeExtension; +use PHPStan\Type\Generic\TemplateTypeHelper; use PHPStan\Type\MethodParameterClosureThisExtension; use PHPStan\Type\MethodParameterClosureTypeExtension; use PHPStan\Type\MethodParameterOutTypeExtension; @@ -1443,6 +1444,24 @@ private function refineClosureNodeScope( return $closureScope->withAnonymousFunctionReflection($refinedClosureType); } + /** + * An object created empty - like `new ArrayObject()` - has `never` generic type + * arguments. After it might have been mutated they no longer describe its contents, + * so they're widened to the bounds of the class' template types. + */ + public function widenNeverTypeArguments(MutatingScope $scope, Expr $expr): MutatingScope + { + $type = $scope->getType($expr); + $widenedType = TemplateTypeHelper::widenNeverTypeArguments($type); + if ($widenedType === $type) { + return $scope; + } + + $nativeType = $scope->getNativeType($expr); + + return $scope->assignExpression($expr, $widenedType, TemplateTypeHelper::widenNeverTypeArguments($nativeType)); + } + /** * @param InvalidateExprNode[] $invalidatedExpressions * @param string[] $uses @@ -2307,6 +2326,7 @@ public function processArgs( } } elseif ($calleeReflection !== null && $calleeReflection->hasSideEffects()->yes()) { $argType = $scope->getType($arg->value); + $mayMutate = true; if (!$argType->isObject()->no()) { $nakedReturnType = null; if ($nakedMethodReflection !== null) { @@ -2319,11 +2339,10 @@ public function processArgs( ); $nakedReturnType = $nakedParametersAcceptor->getReturnType(); } - if ( - $nakedReturnType === null + $mayMutate = $nakedReturnType === null || !(new ThisType($nakedMethodReflection->getDeclaringClass()))->isSuperTypeOf($nakedReturnType)->yes() - || $nakedMethodReflection->isPure()->no() - ) { + || $nakedMethodReflection->isPure()->no(); + if ($mayMutate) { $this->callNodeCallback($nodeCallback, new InvalidateExprNode($arg->value), $scope, $storage); $scope = $scope->invalidateExpression($arg->value, true); } @@ -2331,6 +2350,11 @@ public function processArgs( $this->callNodeCallback($nodeCallback, new InvalidateExprNode($arg->value), $scope, $storage); $scope = $scope->invalidateExpression($arg->value, true); } + + if ($mayMutate) { + // objects reachable from the argument might have been filled in by the callee + $scope = $this->widenNeverTypeArguments($scope, $arg->value); + } } } } diff --git a/src/Type/Generic/TemplateTypeHelper.php b/src/Type/Generic/TemplateTypeHelper.php index 41028207124..5292a36be66 100644 --- a/src/Type/Generic/TemplateTypeHelper.php +++ b/src/Type/Generic/TemplateTypeHelper.php @@ -2,13 +2,16 @@ namespace PHPStan\Type\Generic; +use PHPStan\Reflection\ClassReflection; use PHPStan\Reflection\ParametersAcceptor; use PHPStan\Type\ErrorType; use PHPStan\Type\GeneralizePrecision; +use PHPStan\Type\NeverType; use PHPStan\Type\NonAcceptingNeverType; use PHPStan\Type\Type; use PHPStan\Type\TypeTraverser; use PHPStan\Type\VerbosityLevel; +use function array_values; final class TemplateTypeHelper { @@ -94,6 +97,74 @@ public static function resolveToBounds(Type $type): Type }); } + /** + * Widens `never` type arguments of generic objects to their template bounds. + * + * An object constructed empty - like `new ArrayObject()` - gets `never` type + * arguments inferred. Once the object might have been mutated, `never` no longer + * describes what it can contain, but a wider type inferred from the call site + * would be unsound, so the bounds are the safest thing to fall back to. + */ + public static function widenNeverTypeArguments(Type $type): Type + { + return TypeTraverser::map($type, static function (Type $type, callable $traverse): Type { + if ($type instanceof GenericObjectType) { + $widenedTypes = self::widenNeverTypesToBounds($type->getTypes(), $type->getClassReflection()); + if ($widenedTypes !== null) { + return $traverse(new GenericObjectType( + $type->getClassName(), + $widenedTypes, + $type->getSubtractedType(), + variances: $type->getVariances(), + )); + } + } elseif ($type instanceof GenericStaticType) { + $widenedTypes = self::widenNeverTypesToBounds($type->getTypes(), $type->getClassReflection()); + if ($widenedTypes !== null) { + return $traverse(new GenericStaticType( + $type->getClassReflection(), + $widenedTypes, + $type->getSubtractedType(), + $type->getVariances(), + )); + } + } + + return $traverse($type); + }); + } + + /** + * @param array $typeArguments + * @return array|null null when nothing was widened + */ + private static function widenNeverTypesToBounds(array $typeArguments, ?ClassReflection $classReflection): ?array + { + if ($classReflection === null) { + return null; + } + + $templateTypes = array_values($classReflection->getTemplateTypeMap()->getTypes()); + $widened = false; + foreach ($typeArguments as $i => $typeArgument) { + if (!$typeArgument instanceof NeverType) { + continue; + } + if (!isset($templateTypes[$i])) { + continue; + } + $templateType = $templateTypes[$i]; + if (!$templateType instanceof TemplateType) { + continue; + } + + $typeArguments[$i] = $templateType->getBound(); + $widened = true; + } + + return $widened ? $typeArguments : null; + } + /** * @template T of Type * @param T $type diff --git a/tests/PHPStan/Analyser/nsrt/bug-15147.php b/tests/PHPStan/Analyser/nsrt/bug-15147.php new file mode 100644 index 00000000000..72574dab928 --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-15147.php @@ -0,0 +1,226 @@ +|mixed> $alias + */ + public function populate(ArrayObject $alias): void + { + if (rand(1, 10) > 5) { + // Append an example array + $alias->append(['item', 13]); + } else { + $alias->append('void'); + } + } + + public function test(): void + { + $alias = new ArrayObject(); + assertType('ArrayObject<*NEVER*, *NEVER*>', $alias); + $this->populate($alias); + assertType('ArrayObject<(int|string), mixed>', $alias); + $alias = $alias->getArrayCopy(); + assertType('array', $alias); + + foreach ($alias as $k => $v) { + if (!is_array($v)) { + $alias[$k] = []; + } + } + } + +} + +/** @template T */ +class Coll +{ + + /** @var array */ + private array $items; + + /** @param array $items */ + public function __construct(array $items = []) + { + $this->items = $items; + } + + /** @param T $item */ + public function add($item): void + { + } + + /** @return array */ + public function all(): array + { + return $this->items; + } + +} + +class Consumer +{ + + /** @param Coll $c */ + public function __construct(Coll $c) + { + } + + /** @param Coll $c */ + public static function consumeStatically(Coll $c): void + { + } + +} + +class ImpureConsumer +{ + + /** + * @param Coll $c + * @phpstan-impure + */ + public function __construct(Coll $c) + { + } + +} + +/** @param Coll $c */ +function consume(Coll $c): void +{ +} + +/** + * @param Coll $c + * @phpstan-pure + */ +function consumePurely(Coll $c): int +{ + return 1; +} + +/** @param array> $colls */ +function consumeMany(array $colls): void +{ +} + +class Holder +{ + + /** @var Coll */ + public static Coll $staticColl; + + /** @var Coll */ + public Coll $coll; + +} + +function methodWithSideEffectsCalledOnIt(): void +{ + $c = new Coll(); + assertType('Bug15147\Coll<*NEVER*>', $c); + $c->add('foo'); + assertType('Bug15147\Coll', $c); + assertType('array', $c->all()); +} + +function passedToFunction(): void +{ + $c = new Coll(); + consume($c); + assertType('Bug15147\Coll', $c); +} + +function passedToStaticMethod(): void +{ + $c = new Coll(); + Consumer::consumeStatically($c); + assertType('Bug15147\Coll', $c); +} + +function passedToConstructor(): void +{ + // an unmarked constructor is assumed not to mutate its arguments, + // see nsrt/impure-constructor.php + $c = new Coll(); + new Consumer($c); + assertType('Bug15147\Coll<*NEVER*>', $c); + + $c2 = new Coll(); + new ImpureConsumer($c2); + assertType('Bug15147\Coll', $c2); +} + +function passedInsideArray(): void +{ + $colls = [new Coll()]; + consumeMany($colls); + assertType('array{Bug15147\Coll}', $colls); +} + +function passedToPureFunction(): void +{ + $c = new Coll(); + consumePurely($c); + assertType('Bug15147\Coll<*NEVER*>', $c); +} + +function inProperty(Holder $h): void +{ + $h->coll = new Coll(); + assertType('Bug15147\Coll<*NEVER*>', $h->coll); + consume($h->coll); + assertType('Bug15147\Coll', $h->coll); +} + +function inStaticProperty(): void +{ + Holder::$staticColl = new Coll(); + assertType('Bug15147\Coll<*NEVER*>', Holder::$staticColl); + consume(Holder::$staticColl); + assertType('Bug15147\Coll', Holder::$staticColl); +} + +function inUnion(): void +{ + $c = rand(0, 1) === 0 ? new Coll() : null; + assertType('Bug15147\Coll<*NEVER*>|null', $c); + if ($c !== null) { + consume($c); + assertType('Bug15147\Coll', $c); + } +} + +function writtenThroughArrayAccess(): void +{ + $a = new ArrayObject(); + $a['x'] = 1; + assertType('ArrayObject<(int|string), mixed>', $a); + + $b = new ArrayObject(); + $b[] = 1; + assertType('ArrayObject<(int|string), mixed>', $b); +} + +function otherSplClasses(): void +{ + $s = new SplObjectStorage(); + $s->attach(new stdClass()); + assertType('SplObjectStorage', $s); + + $i = new ArrayIterator(); + $i->append(1); + assertType('ArrayIterator<(int|string), mixed>', $i); +}