Preserve constant array when setting a union of constant scalar keys #4520

ondrejmirtes · 2025-11-03T18:13:52Z

No description provided.

ondrejmirtes · 2025-11-03T18:19:57Z

/cc @VincentLanglet This change solves a few issues, including your own: https://github.com/phpstan/phpstan-src/actions/runs/19044800760

Unfortunately it introduces also some problems which I can't dig into right now. Could you please debug why some existing tests are failing and some errors reappear?

I think some of it might be related to #4372 but not sure.

ondrejmirtes · 2025-11-03T18:31:46Z

Also /cc @staabm because you might have touched this more recently 😊

staabm · 2025-11-03T20:29:24Z

src/Type/Constant/ConstantArrayType.php

+		if ($offsetType !== null) {
+			$constantScalars = $offsetType->getConstantScalarTypes();
+			$constantScalarsCount = count($constantScalars);
+			if ($constantScalarsCount > 1 && $constantScalarsCount < ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT) {
+				$arrays = [];
+				foreach ($constantScalars as $constantScalar) {
+					$builder = ConstantArrayTypeBuilder::createFromConstantArray($this);
+					$builder->setOffsetValueType($constantScalar, $valueType);
+					$arrays[] = $builder->getArray();
+				}
+
+				return TypeCombinator::union($this, ...$arrays);
+			}
+		}


I wonder, whether this code should be moved into the impl of ConstantArrayTypeBuilder->setOffsetValueType()?

It should be handled there but this code should not be copied, but adapted (it's not so easy to do it there). I just wanted to try a quick PoC first.

staabm · 2025-11-03T20:41:13Z

src/Type/Constant/ConstantArrayType.php

 		return new ErrorType(); // undefined offset
 	}

 	public function setOffsetValueType(?Type $offsetType, Type $valueType, bool $unionValues = true): Type


looking at this unionValues parameter, makes me feel we nearly everytime ignore it.

I wonder whether this bool represents the same 2 use-cases as setOffsetValueType() vs. setExistingOffsetValueType().

like unionValues=true is the same as setOffsetValueType()
and unionValues=false is the same as setExistingOffsetValueType()

The unionValues parameter is some hack I came up with, there's no deeper thought in it and it's likely the proper solution is different.

The unionValues parameter is some hack I came up with,

What is the purpose of this hack ?

I did some research yesterday. it was introduced with 5d64483

VincentLanglet · 2025-11-03T23:45:03Z

src/Type/Constant/ConstantArrayType.php

+					$arrays[] = $builder->getArray();
+				}
+
+				return TypeCombinator::union($this, ...$arrays);


I might be tired but why

return TypeCombinator::union($this, ...$arrays);

and not

Suggested change

return TypeCombinator::union($this, ...$arrays);

return TypeCombinator::union(...$arrays);

?

I was also tired, it makes sense without the $this 😊 Updated that.

staabm · 2025-11-04T07:12:48Z

src/Type/Constant/ConstantArrayType.php

+		if ($offsetType !== null) {
+			$constantScalars = $offsetType->getConstantScalarTypes();
+			$constantScalarsCount = count($constantScalars);
+			if ($constantScalarsCount > 1 && $constantScalarsCount < ConstantArrayTypeBuilder::ARRAY_COUNT_LIMIT) {


Do we need similar handling for integer ranges with less than array-limit elements?

Yes, that would make sense!

VincentLanglet · 2025-11-05T20:07:01Z

Looking at failure

1) PHPStan\Rules\Classes\InstantiationRuleTest::testBug7594
Failed asserting that two strings are identical.
--- Expected
+++ Actual
@@ @@
-'
+'55: Parameter #1 $boolAbilities of class Bug7594\HelloWorld constructor expects array<0|1|2|3|4|5|6|7|8|9|10|11|12|13|14|15|16|17, bool>, array<int<0, max>, bool> given.
 '

=> Related to for loop, which generalize the result computed after the second loop.

phpstan-src/src/Analyser/NodeScopeResolver.php

Lines 1239 to 1241 in 770b08c

    
           if ($count >= self::GENERALIZE_AFTER_ITERATION) { 
        
           	$bodyScope = $prevScope->generalizeWith($bodyScope); 
        
           }

And looking at all the other issues, I think it's also the fact that for loop generalize result creating less precise ones, except:

4) PHPStan\Analyser\NodeScopeResolverTest::testFile with data set "tests\PHPStan\Analyser\nsrt\constant-array-type-set.php" ('D:\a\phpstan-src\phpstan-src\...et.php')
Failed assertions in D:\a\phpstan-src\phpstan-src\tests\PHPStan\Analyser\nsrt\constant-array-type-set.php:

Line 24:
Expected: array{bool, bool, bool}
Actual:   array{false, false, true}|array{false, true, false}|array{true, false, false}

Line 36:
Expected: array{0: bool, 1: bool, 2: bool, 3?: true}
Actual:   array{false, false, false, true}|array{false, false, true}|array{false, true, false}|array{true, false, false}

Line 42:
Expected: array{bool, bool, false}
Actual:   array{false, true, false}|array{true, false, false}

Line 104:
Expected: array{bool, bool, false}
Actual:   array{false, true, false}|array{true, false, false}

Seems expected to me

staabm reviewed Nov 3, 2025

View reviewed changes

VincentLanglet reviewed Nov 3, 2025

View reviewed changes

staabm reviewed Nov 4, 2025

View reviewed changes

Preserve constant array when setting a union of constant scalar keys

50eec8c

ondrejmirtes force-pushed the constant-array-set branch from 7ab4b71 to 50eec8c Compare November 4, 2025 07:55

paul-norman mentioned this pull request Nov 4, 2025

Infer array types when keys are initialised / manipulated via loops phpstan/phpstan#13759

Open

	return TypeCombinator::union($this, ...$arrays);
	return TypeCombinator::union(...$arrays);

Preserve constant array when setting a union of constant scalar keys #4520

Are you sure you want to change the base?

Preserve constant array when setting a union of constant scalar keys #4520

Conversation

ondrejmirtes commented Nov 3, 2025

Uh oh!

ondrejmirtes commented Nov 3, 2025

Uh oh!

ondrejmirtes commented Nov 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

staabm Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VincentLanglet commented Nov 5, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

staabm Nov 3, 2025 •

edited

Loading