diff --git a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php index 9a4947d5216..4eaca375543 100644 --- a/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php +++ b/src/Rules/Arrays/NonexistentOffsetInArrayDimFetchRule.php @@ -3,6 +3,7 @@ namespace PHPStan\Rules\Arrays; use PhpParser\Node; +use PhpParser\Node\Expr; use PhpParser\Node\Expr\ArrayDimFetch; use PhpParser\Node\Expr\Variable; use PHPStan\Analyser\NullsafeOperatorHelper; @@ -10,6 +11,7 @@ use PHPStan\DependencyInjection\AutowiredParameter; use PHPStan\DependencyInjection\RegisteredRule; use PHPStan\Internal\SprintfHelper; +use PHPStan\Node\Printer\ExprPrinter; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleErrorBuilder; use PHPStan\Rules\RuleLevelHelper; @@ -33,6 +35,7 @@ final class NonexistentOffsetInArrayDimFetchRule implements Rule public function __construct( private RuleLevelHelper $ruleLevelHelper, private NonexistentOffsetInArrayDimFetchCheck $nonexistentOffsetInArrayDimFetchCheck, + private ExprPrinter $exprPrinter, #[AutowiredParameter] private bool $reportMaybes, ) @@ -120,10 +123,8 @@ public function processNode(Node $node, Scope $scope): array $arrayArg = $node->dim->getArgs()[0]->value; $arrayType = $scope->getType($arrayArg); if ( - $arrayArg instanceof Node\Expr\Variable - && $node->var instanceof Node\Expr\Variable - && is_string($arrayArg->name) - && $arrayArg->name === $node->var->name + $this->isDeterministicExpr($node->var, $scope) + && $this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var) && $arrayType->isArray()->yes() && $arrayType->isIterableAtLeastOnce()->yes() ) { @@ -146,10 +147,8 @@ public function processNode(Node $node, Scope $scope): array $arrayType = $scope->getType($arrayArg); if ( - $arrayArg instanceof Node\Expr\Variable - && $node->var instanceof Node\Expr\Variable - && is_string($arrayArg->name) - && $arrayArg->name === $node->var->name + $this->isDeterministicExpr($node->var, $scope) + && $this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var) && $arrayType->isArray()->yes() && $arrayType->isIterableAtLeastOnce()->yes() && ($numArg === null || $one->isSuperTypeOf($scope->getType($numArg))->yes()) @@ -170,10 +169,8 @@ public function processNode(Node $node, Scope $scope): array $arrayArg = $node->dim->left->getArgs()[0]->value; $arrayType = $scope->getType($arrayArg); if ( - $arrayArg instanceof Node\Expr\Variable - && $node->var instanceof Node\Expr\Variable - && is_string($arrayArg->name) - && $arrayArg->name === $node->var->name + $this->isDeterministicExpr($node->var, $scope) + && $this->exprPrinter->printExpr($arrayArg) === $this->exprPrinter->printExpr($node->var) && $arrayType->isList()->yes() && $arrayType->isIterableAtLeastOnce()->yes() ) { @@ -189,6 +186,67 @@ public function processNode(Node $node, Scope $scope): array ); } + private function isDeterministicExpr(Expr $expr, Scope $scope): bool + { + if ($expr instanceof Variable) { + return true; + } + + if ($expr instanceof Expr\PropertyFetch) { + return $this->isDeterministicExpr($expr->var, $scope); + } + + if ($expr instanceof Expr\StaticPropertyFetch) { + return true; + } + + if ($expr instanceof Expr\ArrayDimFetch) { + if ($expr->dim === null) { + return false; + } + + return $this->isDeterministicExpr($expr->var, $scope) + && $this->isDeterministicExpr($expr->dim, $scope); + } + + if ($expr instanceof Expr\MethodCall && $expr->name instanceof Node\Identifier) { + $callerType = $scope->getType($expr->var); + $methodReflection = $scope->getMethodReflection($callerType, $expr->name->name); + if ($methodReflection !== null && $methodReflection->hasSideEffects()->no()) { + return $this->isDeterministicExpr($expr->var, $scope) + && $this->areDeterministicArgs($expr->getArgs(), $scope); + } + + return false; + } + + if ($expr instanceof Expr\StaticCall && $expr->name instanceof Node\Identifier && $expr->class instanceof Node\Name) { + $classType = $scope->resolveTypeByName($expr->class); + $methodReflection = $scope->getMethodReflection($classType, $expr->name->name); + if ($methodReflection !== null && $methodReflection->hasSideEffects()->no()) { + return $this->areDeterministicArgs($expr->getArgs(), $scope); + } + + return false; + } + + return false; + } + + /** + * @param Node\Arg[] $args + */ + private function areDeterministicArgs(array $args, Scope $scope): bool + { + foreach ($args as $arg) { + if (!$this->isDeterministicExpr($arg->value, $scope)) { + return false; + } + } + + return true; + } + private function isImplicitArrayCreation(Node\Expr\ArrayDimFetch $node, Scope $scope): TrinaryLogic { $varNode = $node->var; diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 7def992a257..f247c4cbc79 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -2,6 +2,8 @@ namespace PHPStan\Rules\Arrays; +use PHPStan\Node\Printer\ExprPrinter; +use PHPStan\Node\Printer\Printer; use PHPStan\Rules\Rule; use PHPStan\Rules\RuleLevelHelper; use PHPStan\Testing\RuleTestCase; @@ -44,6 +46,7 @@ protected function getRule(): Rule reportPossiblyNonexistentGeneralArrayOffset: $this->reportPossiblyNonexistentGeneralArrayOffset, reportPossiblyNonexistentConstantArrayOffset: $this->reportPossiblyNonexistentConstantArrayOffset, ), + new ExprPrinter(new Printer()), true, ); } @@ -1277,4 +1280,25 @@ public function testBug14308(): void $this->analyse([__DIR__ . '/data/bug-14308.php'], []); } + #[RequiresPhp('>= 8.2')] + public function testBug14390(): void + { + $this->reportPossiblyNonexistentGeneralArrayOffset = true; + + $this->analyse([__DIR__ . '/data/bug-14390.php'], [ + [ + 'Offset (int|string) might not exist on non-empty-array.', + 135, + ], + [ + 'Offset (int|string) might not exist on non-empty-array.', + 136, + ], + [ + 'Offset string might not exist on non-empty-array.', + 169, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/bug-14390.php b/tests/PHPStan/Rules/Arrays/data/bug-14390.php new file mode 100644 index 00000000000..9f74b332253 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/bug-14390.php @@ -0,0 +1,171 @@ += 8.2 + +declare(strict_types = 1); + +namespace Bug14390; + +readonly class Sample +{ + /** + * @param array $fields + */ + public function __construct( + public array $fields = [], + ) { + } +} + +class Foo +{ + public function bar( + Sample $sample, + ): void { + if ($sample->fields !== []) { + echo $sample->fields[array_key_first($sample->fields)]; + } + } + + /** + * @param array $fields + */ + public function zoo( + array $fields, + ): void { + if ($fields !== []) { + echo $fields[array_key_first($fields)]; + } + } + + public function withKey( + Sample $sample, + ): void { + if ($sample->fields !== []) { + $key = array_key_first($sample->fields); + echo $sample->fields[$key]; + } + } + + public function arrayKeyLast( + Sample $sample, + ): void { + if ($sample->fields !== []) { + echo $sample->fields[array_key_last($sample->fields)]; + } + } + + public function arrayRand( + Sample $sample, + ): void { + if ($sample->fields !== []) { + echo $sample->fields[array_rand($sample->fields)]; + } + } + + /** + * @param non-empty-list $list + */ + public function countMinus1( + array $list, + ): void { + echo $list[count($list) - 1]; + } +} + +readonly class SampleList +{ + /** + * @param list $items + */ + public function __construct( + public array $items = [], + ) { + } +} + +class Bar +{ + public function countMinus1Property( + SampleList $sample, + ): void { + if ($sample->items !== []) { + echo $sample->items[count($sample->items) - 1]; + } + } +} + +class StaticProps +{ + /** @var array */ + public static array $fields = []; + + /** @var list */ + public static array $items = []; + + public function arrayKeyFirstStatic(): void + { + if (self::$fields !== []) { + echo self::$fields[array_key_first(self::$fields)]; + } + } + + public function arrayKeyLastStatic(): void + { + if (self::$fields !== []) { + echo self::$fields[array_key_last(self::$fields)]; + } + } + + public function arrayRandStatic(): void + { + if (self::$fields !== []) { + echo self::$fields[array_rand(self::$fields)]; + } + } + + public function countMinus1Static(): void + { + if (self::$items !== []) { + echo self::$items[count(self::$items) - 1]; + } + } +} + +function doWithMethods(WithMethods $withMethods) { + echo $withMethods->pureMethod()[array_key_first($withMethods->pureMethod())]; + echo $withMethods->impureMethod()[array_key_first($withMethods->impureMethod())]; + echo $withMethods->pureMethod($withMethods->impureMethod())[array_key_first($withMethods->pureMethod($withMethods->impureMethod()))]; +} + +class WithMethods { + /** + * @phpstan-pure + * @return non-empty-array + */ + public function pureMethod(mixed ...$args):array {} + /** + * @phpstan-impure + * @return non-empty-array + */ + public function impureMethod():array {} +} + +class NestedArray +{ + /** + * @param array> $nested + */ + public function dimFetchDeterministic(array $nested, string $key): void + { + if (isset($nested[$key])) { + echo $nested[$key][array_key_first($nested[$key])]; + } + } + + /** + * @param non-empty-array> $nested + */ + public function dimFetchWithMethodKey(WithMethods $w, array $nested): void + { + echo $nested[$w->impureMethod()][array_key_first($nested[$w->impureMethod()])]; + } +}