Fix false positive non-existing-offset after count() - 1

phpstan · Jan 5, 2025 · 98f3ba8 · 98f3ba8
1 parent 4b02fa3
commit 98f3ba8
Show file tree

Hide file tree

Showing 4 changed files with 132 additions and 13 deletions.
diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php
@@ -667,7 +667,7 @@ public function specifyTypesInCondition(
 				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;
@@ -677,6 +677,32 @@ 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),
+						);
+					}
+				}
+
+				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
@@ -787,7 +787,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<string>.',
+				26,
+			],
+			[
+				'Offset int<-1, max> might not exist on array<string>.',
+				35,
+			],
+			[
+				'Offset int<0, max> might not exist on non-empty-array<string>.',
+				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
@@ -1,4 +1,6 @@
-<?php declare(strict_types = 1);
+<?php // lint >= 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<string> $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<string> $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
@@ -0,0 +1,45 @@
+<?php declare(strict_types = 1);
+
+namespace ArrayDimFetchOnCount;
+
+class HelloWorld
+{
+	/**
+	 * @param list<string> $hellos
+	 */
+	public function works(array $hellos): string
+	{
+		if ($hellos === []) {
+			return 'nothing';
+		}
+
+		$count = count($hellos) - 1;
+		return $hellos[$count];
+	}
+
+	/**
+	 * @param list<string> $hellos
+	 */
+	public function offByOne(array $hellos): string
+	{
+		$count = count($hellos);
+		return $hellos[$count];
+	}
+
+	/**
+	 * @param array<string> $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];
+	}
+
+}