Read a merged array shape's list-ness in both key orders

src/Type/Constant/ConstantArrayType.php

@@ -1592,6 +1592,55 @@
 		return $existsInvalid ? TrinaryLogic::createMaybe() : TrinaryLogic::createYes();
 	}
 
+	/**
+	 * inferIsListFromShape() of a shape merged from $this and $otherArray. The
+	 * merged keys follow $this's order, but the arrays $otherArray describes
+	 * have its order: merging `[1 => 0, 0 => 0]` into `[0 => 1, 1 => 0]` gives
+	 * keys a list in one order and not in the other. So the shape is read in
+	 * $otherArray's order too (its keys first, then the rest), and the two
+	 * readings combine to `maybe` when they disagree.
+	 *
+	 * @param list<ConstantIntegerType|ConstantStringType> $keyTypes
+	 * @param int[] $optionalKeys
+	 */
+	private function inferMergedIsListFromShape(array $keyTypes, array $optionalKeys, self $otherArray): TrinaryLogic
+	{
+		$isList = self::inferIsListFromShape($keyTypes, $optionalKeys);
+		if ($isList->maybe()) {
+			return $isList;
+		}
+
+		$positions = [];
+		foreach ($keyTypes as $i => $keyType) {
+			$positions[$keyType->getValue()] = $i;
+		}
+
+		// merged position => key type, $otherArray's keys first
+		$reordered = [];
+		foreach ($otherArray->keyTypes as $keyType) {
+			$i = $positions[$keyType->getValue()] ?? null;
+			if ($i === null) {
+				continue;
+			}
+			$reordered[$i] = $keyTypes[$i];
+		}
+		$reordered += $keyTypes;
+		if (array_keys($reordered) === array_keys($keyTypes)) {
+			return $isList;
+		}
+
+		$optional = array_flip($optionalKeys);
+		$reorderedOptionalKeys = [];
+		foreach (array_keys($reordered) as $position => $i) {
+			if (!array_key_exists($i, $optional)) {
+				continue;
+			}
+			$reorderedOptionalKeys[] = $position;
+		}
+
+		return TrinaryLogic::extremeIdentity($isList, self::inferIsListFromShape(array_values($reordered), $reorderedOptionalKeys));
+	}
+
 	/**
 	 * When we're unsetting something not on the array, it will be untouched,
 	 * So the nextAutoIndexes won't change, and the array might still be a list even with PHPStan definition.
@@ -3271,8 +3320,8 @@
 		// where two list inputs guarantee `yes`.
 		$naiveIsList = $this->isList->and($otherArray->isList);
 		$mergedIsSealed = $mergedUnsealedKey instanceof NeverType && $mergedUnsealedKey->isExplicit();
-		$isList = $mergedIsSealed
-			? $naiveIsList->or(self::inferIsListFromShape($keyTypes, $optionalKeys))
+		$isList = $mergedIsSealed && !$naiveIsList->yes()
+			? $naiveIsList->or($this->inferMergedIsListFromShape($keyTypes, $optionalKeys, $otherArray))
 			: $naiveIsList;
 
 		return $this->recreate(
@@ -3314,8 +3363,8 @@
 		$naiveIsList = $this->isList->and($otherArray->isList);
 		$mergedIsSealed = $this->unsealed === null
 			|| ($this->unsealed[0] instanceof NeverType && $this->unsealed[0]->isExplicit());
-		$isList = $mergedIsSealed
-			? $naiveIsList->or(self::inferIsListFromShape($this->keyTypes, $optionalKeys))
+		$isList = $mergedIsSealed && !$naiveIsList->yes()
+			? $naiveIsList->or($this->inferMergedIsListFromShape($this->keyTypes, $optionalKeys, $otherArray))
 			: $naiveIsList;
 
 		return $this->recreate($this->keyTypes, $valueTypes, $nextAutoIndexes, $optionalKeys, $isList, $this->unsealed);

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

@@ -0,0 +1,57 @@
+<?php declare(strict_types = 1);
+
+namespace Bug15679;
+
+use function PHPStan\Testing\assertType;
+
+function differingAtOneKey(bool $b): void
+{
+	$x = $b ? [1 => 0, 0 => 0] : [0 => 1, 1 => 0];
+	assertType('array{0|1, 0}', $x);
+	assertType('bool', array_is_list($x));
+	if (array_is_list($x)) {
+		assertType('array{0|1, 0}', $x);
+	} else {
+		assertType('array{0|1, 0}', $x);
+	}
+
+	$y = $b ? [1 => 'a', 0 => 'b'] : [0 => 'c', 1 => 'a'];
+	assertType("array{'b'|'c', 'a'}", $y);
+	assertType('bool', array_is_list($y));
+}
+
+function listFirst(bool $b): void
+{
+	$x = $b ? [0 => 1, 1 => 0] : [1 => 0, 0 => 0];
+	assertType('array{1: 0, 0: 0|1}', $x);
+	assertType('bool', array_is_list($x));
+}
+
+function sameValues(bool $b): void
+{
+	$x = $b ? [1 => 0, 0 => 0] : [0 => 0, 1 => 0];
+	assertType('array{0, 0}', $x);
+	assertType('bool', array_is_list($x));
+
+	$y = $b ? [0 => 0, 1 => 0] : [1 => 0, 0 => 0];
+	assertType('array{1: 0, 0: 0}', $y);
+	assertType('bool', array_is_list($y));
+}
+
+function differingAtTwoKeys(bool $b): void
+{
+	$x = $b ? [1 => 'a', 0 => 'b'] : [0 => 'c', 1 => 'd'];
+	assertType("array{'c', 'd'}|array{1: 'a', 0: 'b'}", $x);
+	assertType('bool', array_is_list($x));
+}
+
+function sameOrder(bool $b): void
+{
+	$x = $b ? [0 => 0, 1 => 0] : [0 => 1, 1 => 0];
+	assertType('array{0|1, 0}', $x);
+	assertType('true', array_is_list($x));
+
+	$y = $b ? [1 => 0, 0 => 0] : [1 => 0, 0 => 1];
+	assertType('array{1: 0, 0: 0|1}', $y);
+	assertType('false', array_is_list($y));
+}

tests/PHPStan/Type/Constant/ConstantArrayTypeTest.php

@@ -1755,6 +1755,35 @@ public static function dataMergeWithRecomputesListnessOfSealedShape(): iterable
 			[[0, new IntegerType(), false], [5, new StringType(), true]],
 			TrinaryLogic::createYes(),
 		];
+
+		// The merged keys follow $this's order, but the other side's arrays have
+		// theirs: a list merged with the same keys in another order is maybe a
+		// list, whichever side comes first.
+		foreach ([true, false] as $bleedingEdge) {
+			$prefix = $bleedingEdge ? '' : 'legacy: ';
+			yield $prefix . 'list merged with its keys reordered is maybe a list' => [
+				$bleedingEdge,
+				[[0, new IntegerType(), false], [1, new IntegerType(), false]],
+				[[1, new IntegerType(), false], [0, new StringType(), false]],
+				TrinaryLogic::createMaybe(),
+			];
+
+			yield $prefix . 'reordered keys merged with a list are maybe a list' => [
+				$bleedingEdge,
+				[[1, new IntegerType(), false], [0, new StringType(), false]],
+				[[0, new IntegerType(), false], [1, new IntegerType(), false]],
+				TrinaryLogic::createMaybe(),
+			];
+		}
+
+		// The other side's extra key comes before the shared one there, so its
+		// arrays are no list, unlike the merged shape read in $this's order.
+		yield 'list merged with a key before the shared one is maybe a list' => [
+			true,
+			[[0, new IntegerType(), false]],
+			[[1, new StringType(), true], [0, new IntegerType(), false]],
+			TrinaryLogic::createMaybe(),
+		];
 	}
 
 	/**

turbo-ext/src/ConstantArrayType.cpp

@@ -33,6 +33,7 @@
 #include "generated/ConstantArrayType.h"
 
 #include <unordered_map>
+#include <vector>
 
 namespace slots = ptdecl::ConstantArrayType::slot;
 namespace sigs = ptdecl::ConstantArrayType::sig;
@@ -2922,6 +2923,84 @@ class ConstantArrayType
 		return existsInvalid ? PT_TRI_MAYBE : PT_TRI_YES;
 	}
 
+	/* inferIsListFromShape() of a shape merged from $this and the other
+	 * array. The merged keys follow $this's order, but the arrays the other
+	 * describes have its order: merging `[1 => 0, 0 => 0]` into
+	 * `[0 => 1, 1 => 0]` gives keys a list in one order and not in the
+	 * other. So the shape is read in the other's order too (its keys first,
+	 * then the rest), and the two readings combine to `maybe` when they
+	 * disagree; -1 = pending exception */
+	[[nodiscard]] static zend_long inferMergedIsListFromShape(zval *keyTypes, zval *optionalKeys, zend_object *other)
+	{
+		zend_long isList = inferIsListFromShape(keyTypes, optionalKeys);
+		if (isList < 0 || isList == PT_TRI_MAYBE) return isList;
+
+		zval *otherK = slotOf(other, slots::keyTypes, "keyTypes");
+		if (UNEXPECTED(otherK == NULL)) return -1;
+
+		uint32_t count = zend_hash_num_elements(Z_ARRVAL_P(keyTypes));
+		zv::ScratchTable positions(count);
+		std::vector<zend_ulong> keyOrder;
+		keyOrder.reserve(count);
+		for (zv::ArrayEntry entry : zv::ArrRef(keyTypes)) {
+			zv::Val keyValueZv = keyValue(entry.value().deref().raw());
+			if (UNEXPECTED(keyValueZv.isUndef())) return -1;
+			zval position;
+			ZVAL_LONG(&position, (zend_long) entry.indexKey());
+			if (UNEXPECTED(!mapSet(positions.table(), keyValueZv.raw(), &position))) return -1;
+			keyOrder.push_back(entry.indexKey());
+		}
+
+		/* merged positions, the other's keys first ($reordered += $keyTypes) */
+		zv::ScratchTable reordered(count);
+		for (zv::ArrayEntry entry : zv::ArrRef(otherK)) {
+			zv::Val keyValueZv = keyValue(entry.value().deref().raw());
+			if (UNEXPECTED(keyValueZv.isUndef())) return -1;
+			zval *position = mapFind(positions.table(), keyValueZv.raw());
+			if (position == NULL) continue;
+			zend_hash_index_add_empty_element(reordered.table(), (zend_ulong) Z_LVAL_P(position));
+		}
+		for (zend_ulong i : keyOrder) {
+			zend_hash_index_add_empty_element(reordered.table(), i);
+		}
+
+		bool sameOrder = true;
+		size_t p = 0;
+		for (zv::ArrayEntry entry : zv::TableRef(reordered.table())) {
+			if (entry.indexKey() != keyOrder[p++]) {
+				sameOrder = false;
+				break;
+			}
+		}
+		if (sameOrder) return isList;
+
+		zv::ScratchTable optional((uint32_t) zend_hash_num_elements(Z_ARRVAL_P(optionalKeys)));
+		for (zv::ArrayEntry entry : zv::ArrRef(optionalKeys)) {
+			zval *optionalKey = entry.value().deref().raw();
+			if (Z_TYPE_P(optionalKey) == IS_LONG) {
+				zend_hash_index_add_empty_element(optional.table(), (zend_ulong) Z_LVAL_P(optionalKey));
+			}
+		}
+
+		zv::Arr reorderedKeyTypes = zv::Arr::create(count);
+		zv::Arr reorderedOptionalKeys = zv::Arr::create(8);
+		zend_long position = 0;
+		for (zv::ArrayEntry entry : zv::TableRef(reordered.table())) {
+			zend_ulong i = entry.indexKey();
+			if (zend_hash_index_exists(optional.table(), i)) {
+				reorderedOptionalKeys.push(zv::Val::integer(position));
+			}
+			reorderedKeyTypes.push(zv::Ref(zend_hash_index_find(Z_ARRVAL_P(keyTypes), i)));
+			position++;
+		}
+
+		zend_long otherOrderIsList = inferIsListFromShape(reorderedKeyTypes.raw(), reorderedOptionalKeys.raw());
+		if (UNEXPECTED(otherOrderIsList < 0)) return -1;
+
+		/* TrinaryLogic::extremeIdentity() */
+		return isList == otherOrderIsList ? isList : PT_TRI_MAYBE;
+	}
+
 	/* When we're unsetting something not on the array, it will be untouched,
 	 * so the nextAutoIndexes won't change, and the array might still be a
 	 * list even with PHPStan definition; -1 = pending exception */
@@ -5175,8 +5254,8 @@ class ConstantArrayType
 		bool mergedIsSealed;
 		if (UNEXPECTED(!isExplicitNever(mergedUnsealedKey.raw(), mergedIsSealed))) return zv::Val();
 		zend_long isListValue = naiveIsList;
-		if (mergedIsSealed) {
-			zend_long inferred = inferIsListFromShape(keyTypesNew.raw(), optionalKeysUnique.raw());
+		if (mergedIsSealed && naiveIsList != PT_TRI_YES) {
+			zend_long inferred = inferMergedIsListFromShape(keyTypesNew.raw(), optionalKeysUnique.raw(), other);
 			if (UNEXPECTED(inferred < 0)) return zv::Val();
 			isListValue = naiveIsList | inferred;
 		}
@@ -5251,8 +5330,8 @@ class ConstantArrayType
 			if (UNEXPECTED(unsealedKey == NULL || !isExplicitNever(unsealedKey, mergedIsSealed))) return zv::Val();
 		}
 		zend_long isListValue = naiveIsList;
-		if (mergedIsSealed) {
-			zend_long inferred = inferIsListFromShape(k, optionalKeysUnique.raw());
+		if (mergedIsSealed && naiveIsList != PT_TRI_YES) {
+			zend_long inferred = inferMergedIsListFromShape(k, optionalKeysUnique.raw(), other);
 			if (UNEXPECTED(inferred < 0)) return zv::Val();
 			isListValue = naiveIsList | inferred;
 		}