Preserve array type through dependent offset assignment with template keys#5625
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Preserve array type through dependent offset assignment with template keys#5625phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
… keys When assigning `$arr[$key] = $val` where `$key` is a template key-of<T> and `$val` is T[$key] (an OffsetAccessType), the assignment is type- preserving by definition. Previously PHPStan would eagerly resolve the OffsetAccessType, losing the dependent relationship between key and value, then widen all value types to the union of all possible values. Detect this pattern in AssignHandler by examining the raw (unresolved) expression type: if the assigned value's raw type is an OffsetAccessType whose offset matches the dim's template parameter and whose accessed type is a supertype of the array being assigned to, skip the widening step and preserve the original array type. Also compare template parameters by identity (name + scope) rather than strict equals, since the same template K may appear as TemplateKeyOfType in the OffsetAccessType but as TemplateStringType in the resolved dim. Closes phpstan/phpstan#7380
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
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
Fixes false positives when assigning to an array with a dependent template key and value type, such as
$attr[$key] = $valwhere$key: K of key-of<Attrs>and$val: Attrs[K].Previously, PHPStan eagerly resolved
Attrs[K](anOffsetAccessType) into the union of all possible value types (string|5|6|7|bool), then widened each key's value type with this union, producingnon-empty-array<'bar'|'baz'|'foo'|K, 5|6|7|bool|string>instead of preserving the originalarray{foo?: string, bar?: 5|6|7, baz?: bool}. This caused a false positive on the subsequent$this->setAttributes($attr)call.Changes
src/Analyser/ExprHandler/AssignHandler.php: Before callingproduceArrayDimFetchAssignValueToWrite(), detect when the assignment follows a dependent offset access pattern by inspecting the raw (unresolved) expression type from the scope. When the raw value type is anOffsetAccessTypewhose offset matches the dim's template parameter and whose accessed type is a supertype of the target array, skip the widening and preserve the original array type. Uses template identity comparison (name + scope) rather than strictequals()to handle cases where the same templateKappears asTemplateKeyOfTypevsTemplateStringType.src/Type/OffsetAccessType.php: AddedgetAccessedType()andgetAccessedOffset()public getters to enable inspecting the components of anOffsetAccessType.tests/PHPStan/Analyser/nsrt/bug-7380.php: New regression test covering both the concrete type alias case (@phpstan-type Attrs) and the generic class case (@template T).tests/PHPStan/Analyser/AnalyserIntegrationTest.php: UpdatedtestBug7094()to remove the false positive assertion at line 29 (previously expected 6 errors, now correctly expects 5).Closes phpstan/phpstan#7380