Do not assign ErrorType back to array when assign-op result is an error#5476
Open
phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Open
Do not assign ErrorType back to array when assign-op result is an error#5476phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
ErrorType back to array when assign-op result is an error#5476phpstan-bot wants to merge 1 commit intophpstan:2.1.xfrom
Conversation
…rror - When an assign-op like `+=` on an array element produces ErrorType (e.g., `(bool|float|int|string) + int`), the ErrorType was assigned back to the array via `setExistingOffsetValueType`/`setOffsetValueType`. Since ErrorType extends MixedType, `TypeCombinator::union` absorbed the original value type, corrupting the array type to MixedType for ALL elements. - This caused subsequent assign-ops on different keys of the same array to see MixedType instead of the original type, suppressing error reports. - Fix: in `AssignHandler::processAssignVar()`, when the assign-op result is ErrorType, preserve the original element type by traversing the offset chain to recover it. - Applied fix in three locations: ArrayDimFetch branch, ExistingArrayDimFetch branch, and Variable branch. - Covers all assign-op types: `+=`, `-=`, `*=`, `/=`, `%=`, `.=`, `<<=`, `>>=`, `&=`, `|=`, `^=`, `**=`.
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
When multiple assign-ops (e.g.,
+=) are performed on different keys of the same array, only the first operation was flagged as an error. Subsequent operations on sibling keys were silently missed because the first assign-op'sErrorTyperesult corrupted the array's value type.Changes
Modified
src/Analyser/ExprHandler/AssignHandler.phpto detect when an assign-op producesErrorTypeand preserve the original element type instead of assigningErrorTypeback:Added rule test
tests/PHPStan/Rules/Operators/data/bug-10349.phpcovering:+=in a foreach loop (issue 1-A from the report)+=in a foreach loop (issue 1-B)+=outside a loop+=-=,*=,/=,%=,.=,<<=Added NSRT test
tests/PHPStan/Analyser/nsrt/bug-10349.phpverifying the array element type is preserved after an error-producing assign-opRoot cause
ErrorTypeextendsMixedType. When an assign-op like(bool|float|int|string) += intproducesErrorTypeas its result type, thisErrorTypewas assigned back to the array element viasetOffsetValueType/setExistingOffsetValueType. Inside these methods, the new value type is unioned with the existing value type viaTypeCombinator::union. SinceErrorType(as aMixedType) absorbs all other types in a union, the array's entire value type becameMixedType. All subsequent accesses to any key of that array then returnedMixedTypeinstead of the original declared type, suppressing further error reports.The fix preserves the original element type when the assign-op result is
ErrorType, since at runtime a failed operation (TypeError) would leave the variable unchanged.Analogous cases probed
$obj->prop += $x): Not vulnerable — properties don't use nested offset type unwinding, so ErrorType doesn't propagate to sibling properties.Test
+=,-=,*=,/=,%=,.=,<<=) report errors on every array key, not just the firstFixes phpstan/phpstan#10349