From a2901aa5d58c5f7b5aedb747b99b45c2db595093 Mon Sep 17 00:00:00 2001 From: "ondrej@mirtes.cz" Date: Thu, 5 Nov 2026 21:24:19 +0000 Subject: [PATCH] Keep a union benevolent when its members absorbed benevolent members TypeCombinator::union() kept the result benevolent only when every member described exactly like a benevolent input member. Merging (int|int<1, max>|string) with 0 produces int, which matches none of them, so the union lost its benevolence - e.g. an array key after a switch narrowed it with loose comparisons on PHP 7. Amp-Thread-ID: https://ampcode.com/threads/T-01a1ad8d-7160-7699-b079-02097b61a80b --- src/Type/TypeCombinator.php | 21 ++++++++++----- .../Analyser/LooseConstComparisonPhp7Test.php | 1 + .../PHPStan/Analyser/data/bug-15667-php7.php | 26 +++++++++++++++++++ .../PHPStan/Analyser/nsrt/array-functions.php | 8 +++--- tests/PHPStan/Type/TypeCombinatorTest.php | 16 ++++++++++++ turbo-ext/src/TypeCombinator.cpp | 19 +++++++++++--- 6 files changed, 78 insertions(+), 13 deletions(-) create mode 100644 tests/PHPStan/Analyser/data/bug-15667-php7.php diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php index 275725bdc7a..f42af28f3f2 100644 --- a/src/Type/TypeCombinator.php +++ b/src/Type/TypeCombinator.php @@ -636,16 +636,25 @@ public static function doUnion(Type ...$types): Type } if ($benevolentTypes !== []) { - $tempTypes = $types; - foreach ($tempTypes as $i => $type) { - if (!isset($benevolentTypes[$type->describe(VerbosityLevel::value())])) { - break; + $allBenevolent = true; + foreach ($types as $type) { + if (isset($benevolentTypes[$type->describe(VerbosityLevel::value())])) { + continue; + } + + // a member that absorbed a benevolent member stays benevolent, + // like int absorbing int<1, max> from (int<1, max>|string) and 0 + foreach ($benevolentTypes as $benevolentType) { + if ($type->isSuperTypeOf($benevolentType)->yes()) { + continue 2; + } } - unset($tempTypes[$i]); + $allBenevolent = false; + break; } - if ($tempTypes === []) { + if ($allBenevolent) { return new BenevolentUnionType(array_values($types), true); } } diff --git a/tests/PHPStan/Analyser/LooseConstComparisonPhp7Test.php b/tests/PHPStan/Analyser/LooseConstComparisonPhp7Test.php index dd17d275393..6e3df603fa6 100644 --- a/tests/PHPStan/Analyser/LooseConstComparisonPhp7Test.php +++ b/tests/PHPStan/Analyser/LooseConstComparisonPhp7Test.php @@ -15,6 +15,7 @@ public static function dataFileAsserts(): iterable yield from self::gatherAssertTypes(__DIR__ . '/data/loose-const-comparison-php7.php'); yield from self::gatherAssertTypes(__DIR__ . '/data/bug-11732-php7.php'); yield from self::gatherAssertTypes(__DIR__ . '/data/loose-equality-narrowing-php7.php'); + yield from self::gatherAssertTypes(__DIR__ . '/data/bug-15667-php7.php'); } /** diff --git a/tests/PHPStan/Analyser/data/bug-15667-php7.php b/tests/PHPStan/Analyser/data/bug-15667-php7.php new file mode 100644 index 00000000000..62e9415a1f0 --- /dev/null +++ b/tests/PHPStan/Analyser/data/bug-15667-php7.php @@ -0,0 +1,26 @@ + $fields */ + public function doFoo(array $fields): void + { + foreach ($fields as $key => $fieldset) { + assertType('(int|string)', $key); + switch ($key) { + case 'shipping': + break; + case 'billing': + break; + } + assertType('(int|string)', $key); + $this->takesString($key); + } + } +} diff --git a/tests/PHPStan/Analyser/nsrt/array-functions.php b/tests/PHPStan/Analyser/nsrt/array-functions.php index b5a640c8cef..175a8ab6620 100644 --- a/tests/PHPStan/Analyser/nsrt/array-functions.php +++ b/tests/PHPStan/Analyser/nsrt/array-functions.php @@ -354,16 +354,16 @@ assertType('1|2', array_rand([1 => 1, 2 => "2"])); assertType("'a'|'b'", array_rand(["a" => 1, "b" => "2"])); assertType("2|'a'", array_rand(["a" => 1, 2 => "b"])); -assertType('int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed])); +assertType('(int|string)', array_rand([1 => 1, 2 => "b", $mixed => $mixed])); assertType('1|2', array_rand([1 => 1, 2 => "b"], 1)); assertType("'a'|'b'", array_rand(["a" => 1, "b" => "b"], 1)); assertType("2|'a'", array_rand(["a" => 1, 2 => "b"], 1)); -assertType('int|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], 1)); +assertType('(int|string)', array_rand([1 => 1, 2 => "b", $mixed => $mixed], 1)); assertType('non-empty-list<1|2>', array_rand([1 => 1, 2 => "b"], 2)); assertType("non-empty-list<'a'|'b'>", array_rand(["a" => 1, "b" => "b"], 2)); assertType("non-empty-list<2|'a'>", array_rand(["a" => 1, 2 => "b"], 2)); -assertType('non-empty-list', array_rand([1 => 1, 2 => "2", $mixed => $mixed], 2)); +assertType('non-empty-list<(int|string)>', array_rand([1 => 1, 2 => "2", $mixed => $mixed], 2)); assertType('1|2|non-empty-list<1|2>', array_rand([1 => 1, 2 => "b"], $mixed)); assertType("'a'|'b'|non-empty-list<'a'|'b'>", array_rand(["a" => 1, "b" => "b"], $mixed)); assertType("2|'a'|non-empty-list<2|'a'>", array_rand(["a" => 1, 2 => "b"], $mixed)); -assertType('int|non-empty-list|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], $mixed)); +assertType('int|non-empty-list<(int|string)>|string', array_rand([1 => 1, 2 => "b", $mixed => $mixed], $mixed)); diff --git a/tests/PHPStan/Type/TypeCombinatorTest.php b/tests/PHPStan/Type/TypeCombinatorTest.php index 2102af4f173..ed8ce2665da 100644 --- a/tests/PHPStan/Type/TypeCombinatorTest.php +++ b/tests/PHPStan/Type/TypeCombinatorTest.php @@ -845,6 +845,22 @@ public static function dataUnion(): iterable BenevolentUnionType::class, '(int|string)', ], + [ + [ + new BenevolentUnionType([IntegerRangeType::fromInterval(null, -1), IntegerRangeType::fromInterval(1, null), new StringType()]), + new ConstantIntegerType(0), + ], + BenevolentUnionType::class, + '(int|string)', + ], + [ + [ + new BenevolentUnionType([IntegerRangeType::fromInterval(1, null), new StringType()]), + new FloatType(), + ], + UnionType::class, + 'float|int<1, max>|string', + ], [ [ new UnionType([new IntegerType(), new StringType(), new FloatType()]), diff --git a/turbo-ext/src/TypeCombinator.cpp b/turbo-ext/src/TypeCombinator.cpp index 5d1778fad3e..0f5f69d4ce0 100644 --- a/turbo-ext/src/TypeCombinator.cpp +++ b/turbo-ext/src/TypeCombinator.cpp @@ -1289,10 +1289,23 @@ class TypeCombinator zv::Val description = describe(type.raw(), valueLevel); zend_string *key = describedKey(description); if (UNEXPECTED(key == NULL)) return zv::Val(); - if (zend_symtable_find(benevolentTypes.table(), key) == NULL) { - allBenevolent = false; - break; + if (zend_symtable_find(benevolentTypes.table(), key) != NULL) continue; + + /* a member that absorbed a benevolent member stays benevolent, + * like int absorbing int<1, max> from (int<1, max>|string) and 0 */ + bool absorbed = false; + for (zv::ArrayEntry entry : zv::ArrRef(benevolentTypes.raw())) { + int superType = isSuperTypeYes(type.raw(), entry.value().raw()); + if (UNEXPECTED(superType < 0)) return zv::Val(); + if (superType == 1) { + absorbed = true; + break; + } } + if (absorbed) continue; + + allBenevolent = false; + break; } if (allBenevolent) return benevolentUnionType(listOf(types), true);