Widen the key type array_key_first() and array_key_last() return

PHP casts a decimal-integer string array key ("123") to int, so
`array_key_first([$string => null])` is not necessarily a string. PHPStan
inferred `string` for it and reported `is_int()` on the result as always false.

UnsafeArrayStringKeyCastingTraverser already modelled that cast, but only for
the key type an array *has* — a type that also decides how the array describes
itself and what it accepts, which is why only
`reportUnsafeArrayStringKeyCasting: detect` widens it. Widening it with the
toggle off turns `array<string, X>` into `array<X>` everywhere.

castReadKeyType() is the second entry point, for a key that leaves the array as
a value of its own. With the toggle off it widens `string` to the benevolent
`(int|string)`, so neither branch reports anything; `detect` and `prevent` keep
the types they have today. unionWithReadKeyType() adds the `null` an empty array
gives back without losing the benevolence on the way.

The remaining accessors follow in the next commit.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>

Author: zonuexe

src/Type/Php/ArrayFirstLastDynamicReturnTypeExtension.php

@@ -9,6 +9,7 @@
 use PHPStan\ShouldNotHappenException;
 use PHPStan\Type\DynamicFunctionReturnTypeExtension;
 use PHPStan\Type\NullType;
+use PHPStan\Type\Traverser\UnsafeArrayStringKeyCastingTraverser;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
 use function count;
@@ -46,8 +47,12 @@ public function getTypeFromFunctionCall(FunctionReflection $functionReflection,
 		switch ($functionReflection->getName()) {
 			case 'array_key_first':
 			case 'array_key_last':
-				$resultType = $argType->getIterableKeyType();
-				break;
+				$keyType = $argType->getIterableKeyType();
+				if ($iterableAtLeastOnce->yes()) {
+					return UnsafeArrayStringKeyCastingTraverser::castReadKeyType($keyType);
+				}
+
+				return UnsafeArrayStringKeyCastingTraverser::unionWithReadKeyType($keyType, new NullType());
 			case 'array_first':
 			case 'array_last':
 				$resultType = $argType->getIterableValueType();

src/Type/Traverser/UnsafeArrayStringKeyCastingTraverser.php

@@ -4,6 +4,7 @@
 
 use PHPStan\DependencyInjection\ReportUnsafeArrayStringKeyCastingToggle;
 use PHPStan\Type\Accessory\AccessoryDecimalIntegerStringType;
+use PHPStan\Type\BenevolentUnionType;
 use PHPStan\Type\IntegerType;
 use PHPStan\Type\Type;
 use PHPStan\Type\TypeCombinator;
@@ -12,23 +13,82 @@
 use PHPStan\Type\UnionType;
 
 /**
- * Under `reportUnsafeArrayStringKeyCasting: detect`, PHP casts a decimal-integer
- * string array key ("123") to int when iterating, so the iterable key type widens
- * from `string` to `int | non-decimal-int-string`. Shared by ArrayType and
- * ConstantArrayType so both representations agree — otherwise comparing a general
- * array (cast key) against a constant-array shape (raw key) yields a spurious
- * `Maybe`.
+ * PHP casts a decimal-integer string array key ("123") to int, so an array with
+ * a `string` key type can hand back an int. There are two places that matters,
+ * and they widen differently on purpose.
+ *
+ * {@see self::castKeyType()} widens the key type an array *has*. That type also
+ * decides how the array describes itself and what it accepts, so only
+ * `reportUnsafeArrayStringKeyCasting: detect` widens there — it opts into the
+ * resulting reports. Under `prevent` there is nothing to do, because PHPDoc
+ * string key types are narrowed to `non-decimal-int-string` when resolved.
+ *
+ * {@see self::castReadKeyType()} widens a key *taken out* of an array and handed
+ * back as a value of its own — `array_key_first()`, `array_keys()`, `key()`,
+ * `array_flip()`, … With the toggle off it widens `string` to the benevolent
+ * `(int|string)`, which stops `array_key_first([$string => null])` from looking
+ * like a certain `string` without making either branch report an error.
+ *
+ * `foreach` keys are deliberately not widened with the toggle off: the key
+ * usually goes straight back into another array (`$result[$k] = …`), and a
+ * benevolent `(int|string)` key collapses that array to `array<mixed, …>`.
+ * `detect` is the level that gets accurate `foreach` keys.
+ *
+ * Both are shared by ArrayType and ConstantArrayType so the two representations
+ * agree — otherwise comparing a general array (cast key) against a
+ * constant-array shape (raw key) yields a spurious `Maybe`.
  */
 final class UnsafeArrayStringKeyCastingTraverser implements TypeTraverserCallable
 {
 
+	private function __construct(private bool $precise)
+	{
+	}
+
 	public static function castKeyType(Type $keyType): Type
 	{
 		if (ReportUnsafeArrayStringKeyCastingToggle::getLevel() !== ReportUnsafeArrayStringKeyCastingToggle::DETECT) {
 			return $keyType;
 		}
 
-		return TypeTraverser::map($keyType, new self());
+		return TypeTraverser::map($keyType, new self(true));
+	}
+
+	public static function castReadKeyType(Type $keyType): Type
+	{
+		$level = ReportUnsafeArrayStringKeyCastingToggle::getLevel();
+		if ($level !== null) {
+			// `detect` already widened the key type the array carries, and `prevent`
+			// made sure it can't hold a decimal-integer string in the first place.
+			return self::castKeyType($keyType);
+		}
+
+		// A key type that already covers int has nothing to gain from the widening.
+		// Leaving it alone also keeps it out of a BenevolentUnionType, so what is
+		// checked against it stays as strict as it is today.
+		if ($keyType->isSuperTypeOf(new IntegerType())->yes()) {
+			return $keyType;
+		}
+
+		return TypeTraverser::map($keyType, new self(false));
+	}
+
+	/**
+	 * Adds the "there is no key" result an accessor returns for an empty array
+	 * (`null` for array_key_first(), `false` for array_search(), …).
+	 *
+	 * TypeCombinator alone would drop the benevolence of a widened key type and
+	 * start reporting on the very code the widening exists to leave alone.
+	 */
+	public static function unionWithReadKeyType(Type $keyType, Type $noKeyType): Type
+	{
+		$keyType = self::castReadKeyType($keyType);
+		$union = TypeCombinator::union($keyType, $noKeyType);
+		if ($keyType instanceof BenevolentUnionType && $union instanceof UnionType && !$union instanceof BenevolentUnionType) {
+			return new BenevolentUnionType($union->getTypes());
+		}
+
+		return $union;
 	}
 
 	/**
@@ -41,6 +101,14 @@ public function traverse(Type $type, callable $traverse): Type
 		}
 
 		if ($type->isString()->yes() && !$type->isDecimalIntegerString()->no()) {
+			if (!$this->precise) {
+				if ($type->isDecimalIntegerString()->yes()) {
+					return new IntegerType();
+				}
+
+				return new BenevolentUnionType([new IntegerType(), $type]);
+			}
+
 			return TypeCombinator::union(
 				new IntegerType(),
 				TypeCombinator::intersect($type, new AccessoryDecimalIntegerStringType(inverse: true)),

tests/PHPStan/Analyser/nsrt/bug-14245.php

@@ -104,7 +104,7 @@ function keyDifferentArray(array $arr): void {
 	$list = foo();
 	assertType('list<int>', $list);
 	$list[array_key_first($arr)] = 37;
-	assertType('non-empty-array<int|string, int>', $list);
+	assertType('non-empty-array<int>', $list);
 }
 
 function overwriteArraySearch($needle): void {

tests/PHPStan/Analyser/nsrt/php73_functions.php

@@ -54,12 +54,12 @@ public function doFoo(
 		assertType('mixed', json_decode($mixed));
 		assertType('mixed', json_decode($mixed, false, 512, JSON_THROW_ON_ERROR | JSON_NUMERIC_CHECK));
 		assertType('mixed', json_decode($mixed, false, 512, $integer | JSON_THROW_ON_ERROR | JSON_NUMERIC_CHECK));
-		assertType('int|string|null', array_key_first($mixedArray));
-		assertType('int|string|null', array_key_last($mixedArray));
+		assertType('(int|string|null)', array_key_first($mixedArray));
+		assertType('(int|string|null)', array_key_last($mixedArray));
 		assertType('(int|string)', array_key_first($nonEmptyArray));
 		assertType('(int|string)', array_key_last($nonEmptyArray));
-		assertType('string|null', array_key_first($arrayWithStringKeys));
-		assertType('string|null', array_key_last($arrayWithStringKeys));
+		assertType('(int|string|null)', array_key_first($arrayWithStringKeys));
+		assertType('(int|string|null)', array_key_last($arrayWithStringKeys));
 		assertType('null', array_key_first($emptyArray));
 		assertType('null', array_key_last($emptyArray));
 		assertType('0|1|2', array_key_first($literalArray));