Fix phpstan/phpstan#8031: Union of constant string type lost when used with generics.#5167
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Fix phpstan/phpstan#8031: Union of constant string type lost when used with generics.#5167phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
- Changed TemplateTypeHelper::generalizeInferredTemplateType() to only generalize integer types (not strings) when template bound is array-key - Previously, all scalar constant values were generalized when the bound was array-key, turning 'one'|'two' into string - Updated test expectations in bug-13144.php and native-reflection-default-values.php to reflect the more precise types - New regression test in tests/PHPStan/Analyser/nsrt/bug-8031.php Closes phpstan/phpstan#8031
| if (!$templateType->getVariance()->covariant()) { | ||
| $isArrayKey = $templateType->getBound()->describe(VerbosityLevel::precise()) === '(int|string)'; | ||
| if ($type->isScalar()->yes() && $isArrayKey) { | ||
| if ($type->isInteger()->yes() && $isArrayKey) { |
Contributor
There was a problem hiding this comment.
I dont see a reason to have a different behavior for int and string
|
|
||
| $arr = new ArrayObject(['a' => 1, 'b' => 2]); | ||
|
|
||
| assertType('ArrayObject<string, int>', $arr); // correctly inferred as `ArrayObject<string, int>` |
Contributor
There was a problem hiding this comment.
I dunno if this will create issues...
Contributor
There was a problem hiding this comment.
I am not sure either.
related issue was comment to be blocked on bidrectional narrowing:
phpstan/phpstan#8031 (comment)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
When constructing a generic class with an array containing constant string keys (e.g.,
new Collection(['one' => 1, 'two' => 2])), PHPStan was generalizing the key type from'one'|'two'tostring, losing the constant type information. This fix preserves constant string types during template type inference.Changes
src/Type/Generic/TemplateTypeHelper.php—generalizeInferredTemplateType(): changed the first condition from$type->isScalar()->yes()to$type->isInteger()->yes()so that only integer types are generalized when the template bound isarray-key. Removed the|| $isArrayKeyclause from the second condition so constant string values with scalar bounds are preserved.tests/PHPStan/Analyser/nsrt/bug-13144.php— adjusted expected type fromArrayObject<string, int>toArrayObject<'a'|'b', int>to reflect the more precise inference.tests/PHPStan/Analyser/nsrt/native-reflection-default-values.php— adjusted expected type fromArrayObject<string, int>toArrayObject<'key', int>.tests/PHPStan/Analyser/nsrt/bug-8031.php— regression test.Root cause
In
TemplateTypeHelper::generalizeInferredTemplateType(), the condition$type->isScalar()->yes() && $isArrayKeywas too broad — it matched all scalar types (including unions of constant strings) when the template bound wasarray-key, causing them to be generalized viaGeneralizePrecision::templateArgument()which turns constant strings into plainstring. The fix narrows this condition to only generalize integer types witharray-keybounds (since integer keys typically come from sequential array indices and should be generalized toint), while preserving constant string keys which are always explicitly provided.Test
Added
tests/PHPStan/Analyser/nsrt/bug-8031.phpwhich creates a genericCollection<TKey, TValue>with['one' => 1, 'two' => 2]and asserts the inferred type isCollection<'one'|'two', int>rather thanCollection<string, int>.Fixes phpstan/phpstan#8031