diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index b5931147beb..b6ff5c797bd 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -10,6 +10,7 @@ use PhpParser\Node\Expr\MethodCall; use PhpParser\Node\Expr\PropertyFetch; use PhpParser\Node\Expr\StaticCall; +use PhpParser\Node\Identifier; use PhpParser\Node\Name; use PHPStan\DependencyInjection\AutowiredService; use PHPStan\DependencyInjection\Container; @@ -45,6 +46,7 @@ use function array_merge; use function count; use function in_array; +use function is_array; use function strtolower; use function substr; use const COUNT_NORMAL; @@ -53,6 +55,8 @@ final class TypeSpecifier { + private const CONTAINS_CALL_ATTRIBUTE_NAME = 'containsCall'; + /** @var MethodTypeSpecifyingExtension[][]|null */ private ?array $methodTypeSpecifyingExtensionsByClass = null; @@ -573,91 +577,17 @@ private function createForExpr( if ( $expr instanceof FuncCall && $expr->name instanceof Name + && !$this->reflectionProvider->hasFunction($expr->name, $scope) ) { - $has = $this->reflectionProvider->hasFunction($expr->name, $scope); - if (!$has) { - // backwards compatibility with previous behaviour - return new SpecifiedTypes([], []); - } - - $functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope); - $hasSideEffects = $functionReflection->hasSideEffects(); - if ($hasSideEffects->yes()) { - return new SpecifiedTypes([], []); - } - - if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) { - return new SpecifiedTypes([], []); - } - } - - if ( - $expr instanceof FuncCall - && !$expr->name instanceof Name - ) { - $nameType = $scope->getType($expr->name); - if ($nameType->isCallable()->yes()) { - $isPure = null; - foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { - $variantIsPure = $variant->isPure(); - $isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure); - } - - if ($isPure !== null) { - if ($isPure->no()) { - return new SpecifiedTypes([], []); - } - - if (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()) { - return new SpecifiedTypes([], []); - } - } - } - } - - if ( - $expr instanceof MethodCall - && $expr->name instanceof Node\Identifier - ) { - $methodName = $expr->name->toString(); - $calledOnType = $scope->getType($expr->var); - $methodReflection = $scope->getMethodReflection($calledOnType, $methodName); - if ( - $methodReflection === null - || $methodReflection->hasSideEffects()->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) - ) { - if (isset($containsNull) && !$containsNull) { - return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); - } - - return new SpecifiedTypes([], []); - } + return new SpecifiedTypes([], []); } - if ( - $expr instanceof StaticCall - && $expr->name instanceof Node\Identifier - ) { - $methodName = $expr->name->toString(); - if ($expr->class instanceof Name) { - $calledOnType = $scope->resolveTypeByName($expr->class); - } else { - $calledOnType = $scope->getType($expr->class); + if (!($expr instanceof AlwaysRememberedExpr) && $this->expressionContainsNonPureCall($expr, $scope)) { + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); } - $methodReflection = $scope->getMethodReflection($calledOnType, $methodName); - if ( - $methodReflection === null - || $methodReflection->hasSideEffects()->yes() - || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) - ) { - if (isset($containsNull) && !$containsNull) { - return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); - } - - return new SpecifiedTypes([], []); - } + return new SpecifiedTypes([], []); } $sureTypes = []; @@ -688,6 +618,132 @@ private function createForExpr( return $types; } + private function expressionContainsNonPureCall(Expr $expr, Scope $scope): bool + { + // The answer for an expression without any call in it cannot change between + // scopes, and most specified expressions (plain variables, property fetches, + // constant fetches) are of that shape, so it's remembered on the node itself. + if ($expr->getAttribute(self::CONTAINS_CALL_ATTRIBUTE_NAME) === false) { + return false; + } + + $containsCall = false; + $containsNonPureCall = $this->findNonPureCall($expr, $scope, $containsCall); + if (!$containsCall) { + $expr->setAttribute(self::CONTAINS_CALL_ATTRIBUTE_NAME, false); + } + + return $containsNonPureCall; + } + + /** + * Depth-first pre-order search for a call that isn't known to be pure, replacing a + * NodeFinder::findFirst() call - this runs for every expression being specified, + * so the traverser/visitor machinery overhead was significant. + * + * $containsCall is set when the sub-tree contains a call of any kind. + */ + private function findNonPureCall(Node $node, Scope $scope, bool &$containsCall): bool + { + if ($node instanceof Expr\CallLike) { + $containsCall = true; + + if ($this->callIsNotPure($node, $scope)) { + return true; + } + } + + foreach ($node->getSubNodeNames() as $subNodeName) { + $subNode = $node->$subNodeName; + if ($subNode instanceof Node) { + if ($this->findNonPureCall($subNode, $scope, $containsCall)) { + return true; + } + } elseif (is_array($subNode)) { + foreach ($subNode as $subNodeItem) { + if ( + $subNodeItem instanceof Node + && $this->findNonPureCall($subNodeItem, $scope, $containsCall) + ) { + return true; + } + } + } + } + + return false; + } + + private function callIsNotPure(Expr\CallLike $call, Scope $scope): bool + { + if ($call instanceof FuncCall) { + if ($call->name instanceof Name) { + if (!$this->reflectionProvider->hasFunction($call->name, $scope)) { + return false; + } + + return $this->isNotPure($this->reflectionProvider->getFunction($call->name, $scope)->hasSideEffects()); + } + + $nameType = $scope->getType($call->name); + if ($nameType->isCallable()->yes()) { + $isPure = null; + foreach ($nameType->getCallableParametersAcceptors($scope) as $variant) { + $variantIsPure = $variant->isPure(); + $isPure = $isPure === null ? $variantIsPure : $isPure->and($variantIsPure); + } + if ($isPure !== null) { + return $this->isNotPure($isPure->negate()); + } + } + + return false; + } + + if ($call instanceof MethodCall) { + if (!$call->name instanceof Identifier) { + return true; + } + + $methodReflection = $scope->getMethodReflection($scope->getType($call->var), $call->name->name); + if ($methodReflection === null) { + return true; + } + + return $this->isNotPure($methodReflection->hasSideEffects()); + } + + if ($call instanceof StaticCall) { + if (!$call->name instanceof Identifier) { + return true; + } + + if ($call->class instanceof Name) { + $calledOnType = $scope->resolveTypeByName($call->class); + } else { + $calledOnType = $scope->getType($call->class); + } + + $methodReflection = $scope->getMethodReflection($calledOnType, $call->name->name); + if ($methodReflection === null) { + return true; + } + + return $this->isNotPure($methodReflection->hasSideEffects()); + } + + return false; + } + + private function isNotPure(TrinaryLogic $hasSideEffects): bool + { + if ($hasSideEffects->yes()) { + return true; + } + + return !$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no(); + } + private function createNullsafeTypes(Expr $expr, Scope $scope, TypeSpecifierContext $context, ?Type $type): SpecifiedTypes { if ($expr instanceof Expr\NullsafePropertyFetch) { diff --git a/tests/PHPStan/Analyser/nsrt/bug-13416.php b/tests/PHPStan/Analyser/nsrt/bug-13416.php new file mode 100644 index 00000000000..899c815f0ad --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-13416.php @@ -0,0 +1,147 @@ + */ + private static array $storage = []; + + /** + * @return list + * @phpstan-impure + */ + public static function find(): array + { + return self::$storage; + } + + /** @phpstan-impure */ + public function insert(): void + { + self::$storage[] = $this; + } + + /** + * @return non-empty-string + * @phpstan-impure + */ + public function getName(): string + { + return 'test'; + } +} + +class Repository +{ + /** + * @return list + * @phpstan-impure + */ + public function findAll(): array + { + return []; + } + + /** @phpstan-impure */ + public function save(MyRecord $record): void + { + } +} + +function testImpureStaticCallNotNarrowedByCount(): void +{ + assert(count(MyRecord::find()) === 1); + // Impure call result should not be narrowed + assertType('int<0, max>', count(MyRecord::find())); +} + +function testImpureMethodCallNotNarrowedByCount(): void +{ + $repo = new Repository(); + + assert(count($repo->findAll()) === 1); + // Impure call result should not be narrowed + assertType('int<0, max>', count($repo->findAll())); +} + +function testStrlenOfImpureCallNotNarrowed(): void +{ + $record = new MyRecord(); + + assert(strlen($record->getName()) === 3); + // strlen wrapping an impure call should not be narrowed + assertType('int<1, max>', strlen($record->getName())); +} + +function testPureFunctionStaysNarrowed(): void +{ + /** @var list $arr */ + $arr = [1]; + assert(count($arr) === 1); + assertType('1', count($arr)); + + $x = rand(0, 10); + + // Pure expressions stay narrowed + assertType('1', count($arr)); +} + +function testImpureArrowFunctionIIFE(): void +{ + assert(count((fn() => MyRecord::find())()) === 1); + assertType('int<0, max>', count((fn() => MyRecord::find())())); +} + +function testImpureClosureIIFE(): void +{ + assert(count((function() { return MyRecord::find(); })()) === 1); + assertType('int<0, max>', count((function() { return MyRecord::find(); })())); +} + +function testStrlenOfImpureArrowFunctionIIFE(): void +{ + $record = new MyRecord(); + assert(strlen((fn() => $record->getName())()) === 3); + assertType('int<1, max>', strlen((fn() => $record->getName())())); +} + +function testImpureClosureViaVariable(): void +{ + $fn = function(): array { return MyRecord::find(); }; + assert(count($fn()) === 1); + assertType('int<0, max>', count($fn())); +} + +function testImpureClosureWithEchoIIFE(): void +{ + assert(strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })()) === 5); + assertType('int<1, max>', strlen((function() { echo 'side-effect'; return MyRecord::find()[0]->getName(); })())); +} + +function testPureClosureIIFEStaysNarrowed(): void +{ + /** @var list $arr */ + $arr = [1, 2, 3]; + assert(count((fn() => $arr)()) === 3); + assertType('3', count((fn() => $arr)())); +} + +/** + * @param string|null $val + * @phpstan-impure + */ +function impureFunction(?string $val): ?string +{ + return $val; +} + +function testPureOfImpureNotNarrowedByCoalesce(): void +{ + $a = strlen(impureFunction('hello') ?? '') > 0; + assertType('bool', strlen(impureFunction('hello') ?? '') > 0); +}