Narrow tagged unions based on count() with array-size#3302
Narrow tagged unions based on count() with array-size#3302ondrejmirtes merged 9 commits intophpstan:1.11.xfrom
Conversation
|
This pull request has been marked as ready for review. |
src/Analyser/TypeSpecifier.php
Outdated
There was a problem hiding this comment.
Just got an idea on how this can be improved. Will look into the PR again later
There was a problem hiding this comment.
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
{|
Also the same logic could essentially be executed for 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 |
|
I'm going to open a separate issue about it. |
|
Thank you. |
|
Here. phpstan/phpstan#11480 |
|
awesome, thanks |
|
As long as phpstan does not distinguish sealed shapes and unsealed shapes, this change is totally unsafe because phpstan would allow |
|
@stof please report a new issue with a code snippet reproducing the problem you are talking about |
|
There are many issues about current array unsealed-ness. PHPStan isn't very consistent about it. |
|
@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. |
as discussed in phpstan/phpstan#11473 I put the logic arround the list to constant-array logic