diff --git a/src/Analyser/ExprSideEffectsHelper.php b/src/Analyser/ExprSideEffectsHelper.php new file mode 100644 index 00000000000..0bbdabb3a78 --- /dev/null +++ b/src/Analyser/ExprSideEffectsHelper.php @@ -0,0 +1,275 @@ +name instanceof Name) { + if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) { + return false; + } + + $functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope); + $hasSideEffects = $functionReflection->hasSideEffects(); + if ($hasSideEffects->yes()) { + return false; + } + + if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) { + return false; + } + } else { + $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 false; + } + + if (!$this->rememberPossiblyImpureFunctionValues && !$isPure->yes()) { + return false; + } + } + } + } + + return !$this->callLikeArgsHaveSideEffects($expr, $scope); + } + + public function rememberMethodCall(MethodCall $expr, Scope $scope): bool + { + if (!$expr->name instanceof Node\Identifier) { + return false; + } + + $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()) + || $this->expressionHasSideEffects($expr->var, $scope) + || $this->callLikeArgsHaveSideEffects($expr, $scope) + ) { + return false; + } + + return true; + } + + public function rememberStaticCall(StaticCall $expr, Scope $scope): bool + { + if (!$expr->name instanceof Node\Identifier) { + return false; + } + + $methodName = $expr->name->toString(); + if ($expr->class instanceof Name) { + $calledOnType = $scope->resolveTypeByName($expr->class); + } else { + $calledOnType = $scope->getType($expr->class); + } + + $methodReflection = $scope->getMethodReflection($calledOnType, $methodName); + + if ( + $methodReflection === null + || $methodReflection->hasSideEffects()->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + || ($expr->class instanceof Expr && $this->expressionHasSideEffects($expr->class, $scope)) + || $this->callLikeArgsHaveSideEffects($expr, $scope) + ) { + return false; + } + + return true; + } + + public function subExpressionsHaveSideEffects(Expr $expr, Scope $scope): bool + { + if ( + $expr instanceof MethodCall + || $expr instanceof Expr\NullsafeMethodCall + || $expr instanceof PropertyFetch + || $expr instanceof Expr\NullsafePropertyFetch + || $expr instanceof ArrayDimFetch + ) { + if ($this->expressionHasSideEffects($expr->var, $scope)) { + return true; + } + } elseif ( + $expr instanceof StaticCall + || $expr instanceof StaticPropertyFetch + ) { + if ($expr->class instanceof Expr && $this->expressionHasSideEffects($expr->class, $scope)) { + return true; + } + } + + if ($expr instanceof Expr\CallLike && $this->callLikeArgsHaveSideEffects($expr, $scope)) { + return true; + } + + return false; + } + + private function callLikeArgsHaveSideEffects(Expr\CallLike $expr, Scope $scope): bool + { + if ($expr->isFirstClassCallable()) { + return false; + } + + foreach ($expr->getArgs() as $arg) { + if ($this->expressionHasSideEffects($arg->value, $scope)) { + return true; + } + } + + return false; + } + + private function expressionHasSideEffects(Expr $expr, Scope $scope): bool + { + if ($expr instanceof Expr\New_) { + return true; + } + + if ($expr instanceof FuncCall) { + if ($expr->isFirstClassCallable()) { + return false; + } + if (!($expr->name instanceof Name)) { + return true; + } + + if (!$this->reflectionProvider->hasFunction($expr->name, $scope)) { + return true; + } + $functionReflection = $this->reflectionProvider->getFunction($expr->name, $scope); + $hasSideEffects = $functionReflection->hasSideEffects(); + if ($hasSideEffects->yes()) { + return true; + } + if (!$this->rememberPossiblyImpureFunctionValues && !$hasSideEffects->no()) { + return true; + } + foreach ($expr->getArgs() as $arg) { + if ($this->expressionHasSideEffects($arg->value, $scope)) { + return true; + } + } + return false; + } + + if ($expr instanceof MethodCall || $expr instanceof Expr\NullsafeMethodCall) { + if ($expr->isFirstClassCallable()) { + return $this->expressionHasSideEffects($expr->var, $scope); + } + if (!($expr->name instanceof Node\Identifier)) { + return true; + } + + $calledOnType = $scope->getType($expr->var); + $methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString()); + if ( + $methodReflection === null + || $methodReflection->hasSideEffects()->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + ) { + return true; + } + foreach ($expr->getArgs() as $arg) { + if ($this->expressionHasSideEffects($arg->value, $scope)) { + return true; + } + } + return $this->expressionHasSideEffects($expr->var, $scope); + } + + if ($expr instanceof StaticCall) { + if ($expr->isFirstClassCallable()) { + if ($expr->class instanceof Expr) { + return $this->expressionHasSideEffects($expr->class, $scope); + } + return false; + } + if (!($expr->name instanceof Node\Identifier)) { + return true; + } + + if ($expr->class instanceof Name) { + $calledOnType = $scope->resolveTypeByName($expr->class); + } else { + $calledOnType = $scope->getType($expr->class); + } + $methodReflection = $scope->getMethodReflection($calledOnType, $expr->name->toString()); + if ( + $methodReflection === null + || $methodReflection->hasSideEffects()->yes() + || (!$this->rememberPossiblyImpureFunctionValues && !$methodReflection->hasSideEffects()->no()) + ) { + return true; + } + foreach ($expr->getArgs() as $arg) { + if ($this->expressionHasSideEffects($arg->value, $scope)) { + return true; + } + } + if ($expr->class instanceof Expr) { + return $this->expressionHasSideEffects($expr->class, $scope); + } + return false; + } + + if ($expr instanceof PropertyFetch || $expr instanceof Expr\NullsafePropertyFetch) { + return $this->expressionHasSideEffects($expr->var, $scope); + } + + if ($expr instanceof ArrayDimFetch) { + return $this->expressionHasSideEffects($expr->var, $scope); + } + + if ($expr instanceof StaticPropertyFetch) { + if ($expr->class instanceof Expr) { + return $this->expressionHasSideEffects($expr->class, $scope); + } + return false; + } + + return false; + } + +} diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 3e50294c94b..a92bca567cb 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -120,7 +120,7 @@ public function __construct( private array $functionTypeSpecifyingExtensions, private array $methodTypeSpecifyingExtensions, private array $staticMethodTypeSpecifyingExtensions, - private bool $rememberPossiblyImpureFunctionValues, + private ExprSideEffectsHelper $exprSideEffectsHelper, ) { } @@ -2319,94 +2319,40 @@ private function createForExpr( } } - if ( - $expr instanceof FuncCall - && $expr->name instanceof Name - ) { - $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 FuncCall && !$this->exprSideEffectsHelper->rememberFuncCall($expr, $scope)) { + return new SpecifiedTypes([], []); } if ( $expr instanceof MethodCall && $expr->name instanceof Node\Identifier + && !$this->exprSideEffectsHelper->rememberMethodCall($expr, $scope) ) { - $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([], []); + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); } + + return new SpecifiedTypes([], []); } if ( $expr instanceof StaticCall && $expr->name instanceof Node\Identifier + && !$this->exprSideEffectsHelper->rememberStaticCall($expr, $scope) ) { - $methodName = $expr->name->toString(); - if ($expr->class instanceof Name) { - $calledOnType = $scope->resolveTypeByName($expr->class); - } else { - $calledOnType = $scope->getType($expr->class); + 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([], []); + if ($this->exprSideEffectsHelper->subExpressionsHaveSideEffects($expr, $scope)) { + if (isset($containsNull) && !$containsNull) { + return $this->createNullsafeTypes($originalExpr, $scope, $context, $type); } + + return new SpecifiedTypes([], []); } $sureTypes = []; diff --git a/src/Analyser/TypeSpecifierFactory.php b/src/Analyser/TypeSpecifierFactory.php index c42a66cb1b7..76bffaf1a7e 100644 --- a/src/Analyser/TypeSpecifierFactory.php +++ b/src/Analyser/TypeSpecifierFactory.php @@ -35,7 +35,7 @@ public function create(): TypeSpecifier $functionTypeSpecifying, $methodTypeSpecifying, $staticMethodTypeSpecifying, - $this->container->getParameter('rememberPossiblyImpureFunctionValues'), + $this->container->getByType(ExprSideEffectsHelper::class), ); foreach (array_merge( diff --git a/tests/PHPStan/Analyser/nsrt/bug-8985.php b/tests/PHPStan/Analyser/nsrt/bug-8985.php new file mode 100644 index 00000000000..54ad92afdbe --- /dev/null +++ b/tests/PHPStan/Analyser/nsrt/bug-8985.php @@ -0,0 +1,148 @@ += 8.0 + +declare(strict_types = 1); + +namespace Bug8985; + +use function PHPStan\Testing\assertType; + +class Entity +{ + public string $value; + + public function __construct(string $value) + { + $this->value = $value; + } + + public function getValue(): string + { + return $this->value; + } +} + +class Repository +{ + /** @return array */ + public function getAll(): array + { + return [new Entity('test')]; + } + + /** @return array */ + public function getAllFor(mixed $filter): array + { + return [new Entity('test')]; + } + + public string $name = 'default'; + + /** @return array */ + public static function staticGetAll(): array + { + return [new Entity('test')]; + } + + public function getEntity(): Entity + { + return new Entity('test'); + } + + /** @phpstan-impure */ + public static function create(): self + { + return new self(); + } + + public const MY_CONST = 'const_value'; +} + +function testMethodCall(): void { + assert((new Repository())->getAll() === []); + + $all = (new Repository())->getAll(); + assertType('array', $all); + $value = $all[0]->getValue(); +} + +function testNullsafeMethodCall(): void { + assert((new Repository())?->getEntity()?->getValue() === 'specific'); + + assertType('string', (new Repository())?->getEntity()?->getValue()); +} + +function testPropertyFetch(): void { + assert((new Repository())->name === 'foo'); + + assertType('string', (new Repository())->name); +} + +function testNullsafePropertyFetch(): void { + assert((new Repository())?->name === 'foo'); + + assertType('string', (new Repository())?->name); +} + +function testArrayDimFetch(): void { + assert((new Repository())->getAll()[0]->getValue() === 'specific'); + + assertType('string', (new Repository())->getAll()[0]->getValue()); +} + +function testStaticCall(): void { + assert((new Repository())::staticGetAll() === []); + + assertType('array', (new Repository())::staticGetAll()); +} + +function testChainedMethodCalls(): void { + assert((new Repository())->getEntity()->getValue() === 'specific'); + + assertType('string', (new Repository())->getEntity()->getValue()); +} + +function testChainedPropertyOnMethodCall(): void { + assert((new Repository())->getEntity()->value === 'specific'); + + assertType('string', (new Repository())->getEntity()->value); +} + +function testClassConstFetch(): void { + assert((new Repository())::MY_CONST === 'const_value'); + + assertType("'const_value'", (new Repository())::MY_CONST); +} + +function testClassConstFetchOnUnknownClass(string $class, string $anotherClass): void { + assert((new $class())::MY_CONST === 'const_value'); + + assertType("'const_value'", (new $class())::MY_CONST); + + $class = $anotherClass; + assertType("*ERROR*", (new $class())::MY_CONST); +} + +function testNamedConstructor(): void { + assert(Repository::create()->getAll() === []); + + $all = Repository::create()->getAll(); + assertType('array', $all); +} + +function testNamedConstructorChained(): void { + assert(Repository::create()->getEntity()->getValue() === 'specific'); + + assertType('string', Repository::create()->getEntity()->getValue()); +} + +function testNamedConstructorProperty(): void { + assert(Repository::create()->name === 'foo'); + + assertType('string', Repository::create()->name); +} + +function testImpureArgument(Repository $repository): void { + assert($repository->getAllFor(Repository::create()) === []); + + assertType('array', $repository->getAllFor(Repository::create())); +} diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 7cd71f4123e..44588063e86 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -1270,6 +1270,13 @@ public function testBug13773(): void ]); } + public function testBug8985(): void + { + $this->reportPossiblyNonexistentConstantArrayOffset = true; + + $this->analyse([__DIR__ . '/data/bug-8985.php'], []); + } + public function testBug14308(): void { $this->reportPossiblyNonexistentConstantArrayOffset = true; diff --git a/tests/PHPStan/Rules/Arrays/data/bug-8985.php b/tests/PHPStan/Rules/Arrays/data/bug-8985.php new file mode 100644 index 00000000000..26f4cefdf56 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-8985.php @@ -0,0 +1,55 @@ += 8.0 + +declare(strict_types=1); + +namespace Bug8985c; + +class Entity +{ + public function __construct(private string $value) + { + } + + public function getValue(): string + { + return $this->value; + } +} + +class Repository +{ + /** @return array */ + public function getAll(): array + { + return [new Entity('test')]; + } + + /** @return array */ + public function getAllFor(mixed $filter): array + { + return [new Entity('test')]; + } + + /** @phpstan-impure */ + public static function create(): self + { + return new self(); + } +} + +assert((new Repository())->getAll() === []); + +$all = (new Repository())->getAll(); +$value = $all[0]->getValue(); + +assert(Repository::create()->getAll() === []); + +$all2 = Repository::create()->getAll(); +$value2 = $all2[0]->getValue(); + +function testImpureArgument(Repository $repository): void { + assert($repository->getAllFor(Repository::create()) === []); + + $all = $repository->getAllFor(Repository::create()); + $value = $all[0]->getValue(); +} diff --git a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php index e381f321f02..56ef1890054 100644 --- a/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php +++ b/tests/PHPStan/Rules/Methods/ReturnTypeRuleTest.php @@ -1326,6 +1326,13 @@ public function testBug10924(): void $this->analyse([__DIR__ . '/data/bug-10924.php'], []); } + #[RequiresPhp('>= 8.0.0')] + public function testBug8985(): void + { + $this->checkExplicitMixed = true; + $this->analyse([__DIR__ . '/data/bug-8985.php'], []); + } + public function testBug11430(): void { $this->analyse([__DIR__ . '/../../Analyser/nsrt/bug-11430.php'], []); diff --git a/tests/PHPStan/Rules/Methods/data/bug-8985.php b/tests/PHPStan/Rules/Methods/data/bug-8985.php new file mode 100644 index 00000000000..2dd4d713e51 --- /dev/null +++ b/tests/PHPStan/Rules/Methods/data/bug-8985.php @@ -0,0 +1,38 @@ + + */ + protected function getDefaultFunctions(): array + { + /** @var array $x */ + $x = (new Defaults())->getFunctions(); + return $x; + } +} + +class HelloWorld2 +{ + /** + * @return array + */ + protected function getDefaultFunctions(): array + { + /** @var array */ + return (new Defaults())->getFunctions(); + } +} + +class Defaults +{ + public function getFunctions(): mixed + { + return []; + } +}