Extract Type::sortArray() for sort / rsort / usort#5613
Merged
ondrejmirtes merged 2 commits into2.1.xfrom May 7, 2026
Merged
Conversation
Replaces `FuncCallHandler::getArraySortPreserveListFunctionType()`'s
hand-rolled `TypeTraverser::map` callback with a polymorphic `Type`
method. The call site shrinks to `$type->sortArray()`.
The semantics — values reordered, array re-indexed as a list — are
similar to `shuffleArray()` but differ in two ways that ruled out
direct reuse:
- Empty arrays stay empty (`array{}` does not degrade to
`list<never>`). The check moves into `ArrayTypeTrait::sortArray()`
via `isIterableAtLeastOnce()->no()`, so per-leaf decisions also
preserve emptiness inside unions (precision improvement vs. the
closure-captured outer flag in the original).
- Non-array leaves pass through unchanged (rather than
`ErrorType` like `shuffleArray()`), because `sort($x)` where `$x`
is a `mixed` / template / non-array variable should not erase the
variable's type — the arg-type rule is responsible for the
diagnostic.
Implementation:
- `ArrayTypeTrait::sortArray()`: shared body for `ArrayType` and
`ConstantArrayType`. Returns the input unchanged if
`isIterableAtLeastOnce()->no()`; otherwise builds
`IntersectionType[ArrayType<int<0,max>, valueType>,
AccessoryArrayListType]`, intersected with `NonEmptyArrayType`
when the leaf's own `isIterableAtLeastOnce()` is `Yes`.
- `NonArrayTypeTrait`, `MaybeArrayTypeTrait`, `MixedType`,
`StrictMixedType`, `StaticType`, `NeverType`, and the array
accessory types (`AccessoryArrayListType`, `NonEmptyArrayType`,
`OversizedArrayType`, `HasOffsetType`, `HasOffsetValueType`):
return `$this`.
- `UnionType` / `IntersectionType`: distribute via `unionTypes` /
`intersectTypes`.
- `LateResolvableTypeTrait`: delegates to `resolve()`.
Pure refactor: full test suite + phpstan + cs pass.
Follow-up cleanup across #5611, #5612, #5613: - `FuncCallHandler::getArrayWalkResultType()` and `getArraySortDoNotPreserveListFunctionType()` were single-statement wrappers around `Type::mapValueType()` / `Type::makeListMaybe()`. Both private, both fully replaced by their polymorphic call; inline at the two/one call sites and delete the helpers. - `InstanceofHandler` and `TypeSpecifier` (the `instanceof <expr>` branch) used a throwaway `$classType = $scope->getType(...)` only to immediately overwrite it with `$result->type`. Drop the intermediate; chain the call. Pure simplification: full test suite + phpstan + cs pass.
Contributor
|
I'm not fully sure but
Take a look at https://phpstan.org/r/cacb0738-d70f-4c8b-83b5-931c989fd4b0 @ondrejmirtes Edit: illustrated with |
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
2 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
Same pattern as #5611 / #5612: extract a hand-rolled
TypeTraverser::mapcallback into a polymorphicTypemethod. The call site (FuncCallHandler::getArraySortPreserveListFunctionType) shrinks to\$type->sortArray().The semantics — values reordered, array re-indexed as a list — are similar to the existing
shuffleArray(), but two differences prevent direct reuse:array{}does not degrade tolist<never>likeshuffleArray()does. The check moves intoArrayTypeTrait::sortArray()viaisIterableAtLeastOnce()->no(), so per-leaf decisions also preserve emptiness inside unions (incidental precision improvement over the closure-captured outer flag in the original).sort(\$x)where\$x: mixed(or a template, or a non-array variable) should not erase the variable's type — the arg-type rule is responsible for the diagnostic.shuffleArray()returnsErrorTypefor non-arrays, which would propagate the wrong way throughprocessVirtualAssign.Implementation
ArrayTypeTrait::sortArray(): shared body forArrayTypeandConstantArrayType(their original behavior was identical, so consolidating into the trait drops the per-class duplication).NonArrayTypeTrait,MaybeArrayTypeTrait,MixedType,StrictMixedType,StaticType,NeverType, and the five array accessory types:return \$this.UnionType/IntersectionType: distribute.LateResolvableTypeTrait: delegate toresolve().Net diff: +93/-27 across 16 files.
Test plan