From 74648f7ec8863826d9d68b4878398d06d7965ade Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 3 Jan 2025 10:15:14 +0100 Subject: [PATCH 1/4] Fix false positive non-existing-offset after array_key_last --- src/Analyser/TypeSpecifier.php | 24 +++++++++- ...nexistentOffsetInArrayDimFetchRuleTest.php | 12 +++++ ...rray-dim-after-array-key-first-or-last.php | 47 +++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 2f57e11abd..d22c706437 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -665,7 +665,29 @@ public function specifyTypesInCondition( throw new ShouldNotHappenException(); } if ($context->null()) { - return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr); + $specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr); + + if ( + $expr->expr instanceof FuncCall + && $expr->expr->name instanceof Name + && $expr->expr->name->toLowerString() === 'array_key_last' + && count($expr->expr->getArgs()) >= 1 + ) { + $arrayArg = $expr->expr->getArgs()[0]->value; + $arrayType = $scope->getType($arrayArg); + if ( + $arrayType->isArray()->yes() + && $arrayType->isIterableAtLeastOnce()->yes() + ) { + $dimFetch = new ArrayDimFetch($arrayArg, $expr->var); + + return $specifiedTypes->unionWith( + $this->create($dimFetch, $arrayType->getLastIterableValueType(), TypeSpecifierContext::createTrue(), $scope), + ); + } + } + + return $specifiedTypes; } return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr); diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 79e015fbfe..8ec605dab9 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -785,4 +785,16 @@ public function testBug12122(): void $this->analyse([__DIR__ . '/data/bug-12122.php'], []); } + public function testArrayDimFetchAfterArrayKeyFirstOrLast(): void + { + $this->reportPossiblyNonexistentGeneralArrayOffset = true; + + $this->analyse([__DIR__ . '/data/array-dim-after-array-key-first-or-last.php'], [ + [ + 'Offset null does not exist on array{}.', + 17, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php b/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php new file mode 100644 index 0000000000..62389b820f --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php @@ -0,0 +1,47 @@ + $hellos + */ + public function last(array $hellos): string + { + if ($hellos !== []) { + $lastHelloKey = array_key_last($hellos); + return $hellos[$lastHelloKey]; + } else { + $lastHelloKey = array_key_last($hellos); + return $hellos[$lastHelloKey]; + } + } + + /** + * @param list $hellos + */ + public function first(array $hellos): string + { + if ($hellos !== []) { + $firstHelloKey = array_key_first($hellos); + return $hellos[$firstHelloKey]; + } + + return 'nothing'; + } + + /** + * @param array{first: int, middle: float, last: bool} $hellos + */ + public function shape(array $hellos): int|bool + { + $firstHelloKey = array_key_first($hellos); + $lastHelloKey = array_key_last($hellos); + + if (rand(0,1)) { + return $hellos[$firstHelloKey]; + } + return $hellos[$lastHelloKey]; + } +} From a8d153b396d908db4da8937c3810c5729e4766e3 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Fri, 3 Jan 2025 11:46:43 +0100 Subject: [PATCH 2/4] Fix false positive non-existing-offset after count() - 1 --- src/Analyser/TypeSpecifier.php | 30 ++++++++++- ...nexistentOffsetInArrayDimFetchRuleTest.php | 22 +++++++- ...rray-dim-after-array-key-first-or-last.php | 50 +++++++++++++++---- .../Arrays/data/array-dim-after-count.php | 45 +++++++++++++++++ 4 files changed, 134 insertions(+), 13 deletions(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/array-dim-after-count.php diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index d22c706437..377640e179 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -667,10 +667,11 @@ public function specifyTypesInCondition( if ($context->null()) { $specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr); + // infer $arr[$key] after $key = array_key_first/last($arr) if ( $expr->expr instanceof FuncCall && $expr->expr->name instanceof Name - && $expr->expr->name->toLowerString() === 'array_key_last' + && in_array($expr->expr->name->toLowerString(), ['array_key_first', 'array_key_last'], true) && count($expr->expr->getArgs()) >= 1 ) { $arrayArg = $expr->expr->getArgs()[0]->value; @@ -680,6 +681,33 @@ public function specifyTypesInCondition( && $arrayType->isIterableAtLeastOnce()->yes() ) { $dimFetch = new ArrayDimFetch($arrayArg, $expr->var); + $iterableValueType = $expr->expr->name->toLowerString() === 'array_key_first' + ? $arrayType->getFirstIterableValueType() + : $arrayType->getLastIterableValueType(); + + return $specifiedTypes->unionWith( + $this->create($dimFetch, $iterableValueType, TypeSpecifierContext::createTrue(), $scope), + ); + } + } + + // infer $list[$count] after $count = count($list) - 1 + if ( + $expr->expr instanceof Expr\BinaryOp\Minus + && $expr->expr->left instanceof FuncCall + && $expr->expr->left->name instanceof Name + && in_array($expr->expr->left->name->toLowerString(), ['count', 'sizeof'], true) + && count($expr->expr->left->getArgs()) >= 1 + && $expr->expr->right instanceof Node\Scalar\Int_ + && $expr->expr->right->value === 1 + ) { + $arrayArg = $expr->expr->left->getArgs()[0]->value; + $arrayType = $scope->getType($arrayArg); + if ( + $arrayType->isList()->yes() + && $arrayType->isIterableAtLeastOnce()->yes() + ) { + $dimFetch = new ArrayDimFetch($arrayArg, $expr->var); return $specifiedTypes->unionWith( $this->create($dimFetch, $arrayType->getLastIterableValueType(), TypeSpecifierContext::createTrue(), $scope), diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 8ec605dab9..75e34ec6f3 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -792,7 +792,27 @@ public function testArrayDimFetchAfterArrayKeyFirstOrLast(): void $this->analyse([__DIR__ . '/data/array-dim-after-array-key-first-or-last.php'], [ [ 'Offset null does not exist on array{}.', - 17, + 19, + ], + ]); + } + + public function testArrayDimFetchAfterCount(): void + { + $this->reportPossiblyNonexistentGeneralArrayOffset = true; + + $this->analyse([__DIR__ . '/data/array-dim-after-count.php'], [ + [ + 'Offset int<0, max> might not exist on list.', + 26, + ], + [ + 'Offset int<-1, max> might not exist on array.', + 35, + ], + [ + 'Offset int<0, max> might not exist on non-empty-array.', + 42, ], ]); } diff --git a/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php b/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php index 62389b820f..e27fcfa175 100644 --- a/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php +++ b/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-key-first-or-last.php @@ -1,4 +1,6 @@ -= 8.0 + +declare(strict_types = 1); namespace ArrayDimAfterArrayKeyFirstOrLast; @@ -10,12 +12,25 @@ class HelloWorld public function last(array $hellos): string { if ($hellos !== []) { - $lastHelloKey = array_key_last($hellos); - return $hellos[$lastHelloKey]; + $last = array_key_last($hellos); + return $hellos[$last]; } else { - $lastHelloKey = array_key_last($hellos); - return $hellos[$lastHelloKey]; + $last = array_key_last($hellos); + return $hellos[$last]; + } + } + + /** + * @param array $hellos + */ + public function lastOnArray(array $hellos): string + { + if ($hellos !== []) { + $last = array_key_last($hellos); + return $hellos[$last]; } + + return 'nothing'; } /** @@ -24,8 +39,21 @@ public function last(array $hellos): string public function first(array $hellos): string { if ($hellos !== []) { - $firstHelloKey = array_key_first($hellos); - return $hellos[$firstHelloKey]; + $first = array_key_first($hellos); + return $hellos[$first]; + } + + return 'nothing'; + } + + /** + * @param array $hellos + */ + public function firstOnArray(array $hellos): string + { + if ($hellos !== []) { + $first = array_key_first($hellos); + return $hellos[$first]; } return 'nothing'; @@ -36,12 +64,12 @@ public function first(array $hellos): string */ public function shape(array $hellos): int|bool { - $firstHelloKey = array_key_first($hellos); - $lastHelloKey = array_key_last($hellos); + $first = array_key_first($hellos); + $last = array_key_last($hellos); if (rand(0,1)) { - return $hellos[$firstHelloKey]; + return $hellos[$first]; } - return $hellos[$lastHelloKey]; + return $hellos[$last]; } } diff --git a/tests/PHPStan/Rules/Arrays/data/array-dim-after-count.php b/tests/PHPStan/Rules/Arrays/data/array-dim-after-count.php new file mode 100644 index 0000000000..4f52d30b24 --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/array-dim-after-count.php @@ -0,0 +1,45 @@ + $hellos + */ + public function works(array $hellos): string + { + if ($hellos === []) { + return 'nothing'; + } + + $count = count($hellos) - 1; + return $hellos[$count]; + } + + /** + * @param list $hellos + */ + public function offByOne(array $hellos): string + { + $count = count($hellos); + return $hellos[$count]; + } + + /** + * @param array $hellos + */ + public function maybeInvalid(array $hellos): string + { + $count = count($hellos) - 1; + echo $hellos[$count]; + + if ($hellos === []) { + return 'nothing'; + } + + $count = count($hellos) - 1; + return $hellos[$count]; + } + +} From 74b11d6df7d74df6aabdf685c84eeca2ff6638a2 Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sun, 5 Jan 2025 12:59:07 +0100 Subject: [PATCH 3/4] Fix false positive non-existing-offset after array_search() --- src/Analyser/TypeSpecifier.php | 26 ++++++++++++- ...nexistentOffsetInArrayDimFetchRuleTest.php | 12 ++++++ .../data/array-dim-after-array-search.php | 37 +++++++++++++++++++ 3 files changed, 74 insertions(+), 1 deletion(-) create mode 100644 tests/PHPStan/Rules/Arrays/data/array-dim-after-array-search.php diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index 377640e179..dc3fcc76e2 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -664,6 +664,7 @@ public function specifyTypesInCondition( if (!$scope instanceof MutatingScope) { throw new ShouldNotHappenException(); } + if ($context->null()) { $specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->expr, $context)->setRootExpr($expr); @@ -718,7 +719,30 @@ public function specifyTypesInCondition( return $specifiedTypes; } - return $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr); + $specifiedTypes = $this->specifyTypesInCondition($scope->exitFirstLevelStatements(), $expr->var, $context)->setRootExpr($expr); + + if ($context->true()) { + // infer $arr[$key] after $key = array_search($needle, $arr) + if ( + $expr->expr instanceof FuncCall + && $expr->expr->name instanceof Name + && in_array($expr->expr->name->toLowerString(), ['array_search'], true) + && count($expr->expr->getArgs()) >= 2 + ) { + $arrayArg = $expr->expr->getArgs()[1]->value; + $arrayType = $scope->getType($arrayArg); + + if ($arrayType->isArray()->yes()) { + $dimFetch = new ArrayDimFetch($arrayArg, $expr->var); + $iterableValueType = $arrayType->getIterableValueType(); + + return $specifiedTypes->unionWith( + $this->create($dimFetch, $iterableValueType, TypeSpecifierContext::createTrue(), $scope), + ); + } + } + } + return $specifiedTypes; } elseif ( $expr instanceof Expr\Isset_ && count($expr->vars) > 0 diff --git a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php index 75e34ec6f3..518fabc0b9 100644 --- a/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php +++ b/tests/PHPStan/Rules/Arrays/NonexistentOffsetInArrayDimFetchRuleTest.php @@ -817,4 +817,16 @@ public function testArrayDimFetchAfterCount(): void ]); } + public function testArrayDimFetchAfterArraySearch(): void + { + $this->reportPossiblyNonexistentGeneralArrayOffset = true; + + $this->analyse([__DIR__ . '/data/array-dim-after-array-search.php'], [ + [ + 'Offset int|string might not exist on array.', + 20, + ], + ]); + } + } diff --git a/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-search.php b/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-search.php new file mode 100644 index 0000000000..3aa2d4c21b --- /dev/null +++ b/tests/PHPStan/Rules/Arrays/data/array-dim-after-array-search.php @@ -0,0 +1,37 @@ += 8.0 + +declare(strict_types = 1); + +namespace ArrayDimAfterArraySeach; + +class HelloWorld +{ + public function doFoo(array $arr, string $needle): string + { + if (($key = array_search($needle, $arr, true)) !== false) { + echo $arr[$key]; + } + } + + public function doBar(array $arr, string $needle): string + { + $key = array_search($needle, $arr, true); + if ($key !== false) { + echo $arr[$key]; + } + } + + public function doFooBar(array $arr, string $needle): string + { + if (($key = array_search($needle, $arr, false)) !== false) { + echo $arr[$key]; + } + } + + public function doBaz(array $arr, string $needle): string + { + if (($key = array_search($needle, $arr)) !== false) { + echo $arr[$key]; + } + } +} From 612fcba8e7a3e73dcf79def0b761523353c2197d Mon Sep 17 00:00:00 2001 From: Markus Staab Date: Sat, 25 Jan 2025 16:49:49 +0100 Subject: [PATCH 4/4] fix --- src/Analyser/TypeSpecifier.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php index dc3fcc76e2..3c1dc42d5f 100644 --- a/src/Analyser/TypeSpecifier.php +++ b/src/Analyser/TypeSpecifier.php @@ -726,7 +726,7 @@ public function specifyTypesInCondition( if ( $expr->expr instanceof FuncCall && $expr->expr->name instanceof Name - && in_array($expr->expr->name->toLowerString(), ['array_search'], true) + && $expr->expr->name->toLowerString() === 'array_search' && count($expr->expr->getArgs()) >= 2 ) { $arrayArg = $expr->expr->getArgs()[1]->value;