Narrow tagged unions based on count() with array-size

[staabm]

as discussed in phpstan/phpstan#11473 I put the logic arround the list to constant-array logic

[phpstan-bot]

This pull request has been marked as ready for review.

[staabm]

Just got an idea on how this can be improved. Will look into the PR again later

[staabm]

hmm I can't get the idea pass all tests..

diff --git a/src/Analyser/TypeSpecifier.php b/src/Analyser/TypeSpecifier.php
index 8ae29d00e..b95074a2d 100644
--- a/src/Analyser/TypeSpecifier.php
+++ b/src/Analyser/TypeSpecifier.php
@@ -1013,15 +1013,18 @@ class TypeSpecifier
 					}
 
 					$arraySize = $innerType->getArraySize();
-					if (!$arraySize instanceof ConstantIntegerType) {
+					if (
+						$arraySize instanceof ConstantIntegerType
+						|| $arraySize instanceof IntegerRangeType
+					) {
+						if (!$arraySize->isSuperTypeOf($constantType)->yes()) {
+							continue;
+						}
+					} else {
 						$result = null;
 						break;
 					}
 
-					if (!$constantType->isSuperTypeOf($arraySize)->yes()) {
-						continue;
-					}
-
 					$result[] = $innerType;
 				}
 
diff --git a/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php b/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php
index 5ccb14560..c6664b605 100644
--- a/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php
+++ b/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php
@@ -79,6 +79,45 @@ class HelloWorld
 		assertType("array{array<int>, numeric-string}|array{string, '', non-empty-string}", $arr);
 	}
 
+	/** @param array{0: int, 1: int, 2: int, 3?: int}|array{string, ''} $arr */
+	public function optKeysInConstArray(array $arr): void
+	{
+		if (count($arr) === 1) {
+			assertType("*NEVER*", $arr);
+		} else {
+			assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+		}
+		assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+
+		if (count($arr) === 2) {
+			assertType("array{string, ''}", $arr);
+		} else {
+			assertType("array{0: int, 1: int, 2: int, 3?: int}", $arr);
+		}
+		assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+
+		if (count($arr) === 3) {
+			assertType("array{0: int, 1: int, 2: int, 3?: int}", $arr);
+		} else {
+			assertType("array{string, ''}", $arr);
+		}
+		assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+
+		if (count($arr) === 4) {
+			assertType("array{0: int, 1: int, 2: int, 3?: int}", $arr);
+		} else {
+			assertType("array{string, ''}", $arr);
+		}
+		assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+
+		if (count($arr) === 5) {
+			assertType("*NEVER*", $arr);
+		} else {
+			assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+		}
+		assertType("array{0: int, 1: int, 2: int, 3?: int}|array{string, ''}", $arr);
+	}
+
 	/** @param array{string, '', non-empty-string}|array<int> $arr */
 	public function mixedArrays(array $arr): void
 	{

[ondrejmirtes]

I've simplified and fixed the logic: 8d16a7c

Also I issued a bugfix that made it possible: ed6bc0b

[ondrejmirtes]

Also the same logic could essentially be executed for if (count($x) > $constantInt) { because the same logic holds even if $arraySize is IntegerRangeType. I've added these tests but one of the asserts fails:

diff --git a/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php b/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php
index 8ecf3438e..fa2daa399 100644
--- a/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php
+++ b/tests/PHPStan/Analyser/nsrt/narrow-tagged-union.php
@@ -107,5 +107,41 @@ class HelloWorld
 		}
 		assertType("array{}|array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
 	}
+
+	public function arrayGreatherThan(): void
+	{
+		$x = [];
+		if (rand(0,1)) {
+			$x[] = 'ab';
+		}
+		if (rand(0,1)) {
+			$x[] = 'xy';
+		}
+
+		if (count($x) > 0) {
+			assertType("array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
+		} else {
+			assertType("array{}", $x);
+		}
+		assertType("array{}|array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
+	}
+
+	public function arrayGreatherThan2(): void
+	{
+		$x = [];
+		if (rand(0,1)) {
+			$x[] = 'ab';
+		}
+		if (rand(0,1)) {
+			$x[] = 'xy';
+		}
+
+		if (count($x) > 1) {
+			assertType("array{0: 'ab', 1?: 'xy'}", $x);
+		} else {
+			assertType("array{}|array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
+		}
+		assertType("array{}|array{'xy'}|array{0: 'ab', 1?: 'xy'}", $x);
+	}
 }
 

We should be able to reuse the same logic for } elseif ($expr instanceof Node\Expr\BinaryOp\Smaller || $expr instanceof Node\Expr\BinaryOp\SmallerOrEqual) { branch in TypeSpecifier.

[ondrejmirtes]

I'm going to open a separate issue about it.

[ondrejmirtes]

Thank you.

[ondrejmirtes]

Here. phpstan/phpstan#11480

[staabm]

awesome, thanks

[stof]

As long as phpstan does not distinguish sealed shapes and unsealed shapes, this change is totally unsafe because phpstan would allow array('a', 'b', 'c') to satisfy a type array{string, string} (as it accepts arrays that have additional values unspecified in the shape)

[staabm]

@stof please report a new issue with a code snippet reproducing the problem you are talking about

[ondrejmirtes]

There are many issues about current array unsealed-ness. PHPStan isn't very consistent about it.

[stof]

@staabm here you go: phpstan/phpstan#11494

@ondrejmirtes I would suggest avoiding to add narrowings that requires sealedness to be valid until sealedness is actually implemented.