Preserve TemplateType in ArrayType and IntersectionType array-mutating methods

[phpstan-bot]

Summary

Follow-up to PRs #5694 and #5696 which fixed shuffleArray() losing template types. This PR systematically audits all other Type::*array methods and applies the same pattern to every method that was losing generic/template types.

Changes

src/Type/ArrayType.php

• intersectKeyArray(): new self(...) → $this->withTypes(...) so TemplateArrayType preserves template identity
• sliceArray(): same fix in the integer-key reindexing branch
• spliceArray(): same fix for the spliced array construction
• mapValueType(): new ArrayType(...) → $this->withTypes(...)
• mapKeyType(): same
• changeKeyCaseArray(): same
• filterArrayRemovingFalsey(): same

src/Type/IntersectionType.php

• popArray(), shiftArray(), reverseArray(), sliceArray(), spliceArray(), intersectKeyArray(), mapValueType(), mapKeyType(), changeKeyCaseArray(), filterArrayRemovingFalsey(): skip calling the array method on TemplateType members (return the template unchanged), matching the pattern from the shuffleArray() fix
• getValuesArray(): preserve TemplateType conditionally when the intersection is already a list (since array_values() on a list is a no-op)

src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php

• When no callback is provided and the type is purely an array, call filterArrayRemovingFalsey() directly on the original type instead of decomposing via getArrays(). This preserves the intersection context (and thus template types).

Methods probed and found to be already correct

• popArray(), shiftArray(), reverseArray() on ArrayType: already return $this, so TemplateArrayType naturally preserves template
• makeListMaybe(), makeAllArrayKeysOptional() on both ArrayType and IntersectionType: already return $this
• flipArray(), fillKeysArray(): correctly drop template identity (fundamentally new array structure)
• getKeysArray(), getValuesArray() on ArrayType: correctly drop template (create new derived arrays)
• setExistingOffsetValueType(), unsetOffset() on ArrayType: correctly drop template (explicit mutations that may violate T's contract)

Root cause

Two parallel patterns caused the same bug:

1. ArrayType level: Methods constructing new self(...) or new ArrayType(...) instead of $this->withTypes(...) lose the template identity when $this is a TemplateArrayType, because TemplateArrayType overrides withTypes() to preserve template metadata.

2. IntersectionType level: When intersectTypes() calls an array method on each intersection member, TemplateType members (backed by TemplateMixedType which extends MixedType) return a plain ArrayType, losing the template. The fix skips calling the array method on TemplateType members, preserving them as-is — the actual array transformation is applied to the concrete array type members of the intersection.

Test

• Added tests/PHPStan/Analyser/nsrt/bug-14633.php with regression tests covering:
  • IntersectionType template preservation for popArray, shiftArray, reverseArray, sliceArray, getValuesArray, filterArrayRemovingFalsey (both list and non-list variants)
  • ArrayType template preservation for filterArrayRemovingFalsey, intersectKeyArray, sliceArray

Fixes phpstan/phpstan#14633

[staabm]

extract and re-use a intersectTypesPreserveTemplateType method

[phpstan-bot]

Extracted a intersectTypesPreserveTemplateType() private helper method that wraps the callback with the TemplateType instanceof check, then replaced all 10 unconditional occurrences of that pattern with one-liner calls to the new helper. The 2 conditional cases (getValuesArray and shuffleArray which check $isList first) remain using intersectTypes directly since they have different logic. Tests and static analysis both pass cleanly.

[staabm]

why do we need this only for lists?

[phpstan-bot]

Both review comments are addressed:

1. "Extract and re-use a intersectTypesPreserveTemplateType method" — Already pushed as commit cf2f90d24. The repeated if ($type instanceof TemplateType) { return $type; } closures in 10 methods are now routed through a single intersectTypesPreserveTemplateType() helper.

2. "Why do we need this only for lists?" (getValuesArray()) — The list-only guard is correct and intentional. array_values() re-indexes keys to sequential integers (0, 1, 2, ...). For a list, this is a no-op — the keys are already sequential integers — so preserving the template type T is safe. For a non-list like T & array<string, int>, array_values() fundamentally changes the key structure (string keys become integer keys), making the result incompatible with T. The same reasoning applies to shuffleArray(), which also has the $isList guard. The other methods that use intersectTypesPreserveTemplateType unconditionally (popArray, shiftArray, reverseArray, etc.) only remove or reorder elements without changing key types, so T remains valid.

[staabm]

add tests expecting these methods only preserve their template types on lists

[phpstan-bot]

Pushed. Added 4 new test cases in bug-14633.php that verify the list-only guard on getValuesArray() and shuffleArray():

• arrayValuesOnNonList: array_values() on T & array<string, int> returns list<int> (T dropped — keys change from string to int)
• shuffleOnList: shuffle() on T & list<int> preserves T (keys are already sequential integers)
• shuffleOnNonList: shuffle() on T & array<string, int> returns list<int> (T dropped — keys change)
• The existing arrayValuesOnList test already covered the list-preserving case

All tests (12100) and make phpstan pass cleanly.

[VincentLanglet]

Is there existing test for the toBenevolentUnion behavior strategy in ArrayFilterFunctionReturnTypeHelper which is not in some other returnTypeExtension ?

Cause we end up different behavior for array_filter than for other function see

public function startDay(array|object $task): string
	{
		dumpType(array_chunk($task, 1));
		dumpType(array_filter($task));
	}

[phpstan-bot]

All 12102 tests pass and make phpstan reports no errors.

The reviewer's comment is a question about pre-existing behavior, not a request for a code change. Here's my analysis:

The toBenevolentUnion in ArrayFilterFunctionReturnTypeHelper is pre-existing (not introduced by this PR). It was there before to handle union types like array|object — when array_filter is called on such a type, BenevolentUnionType::unionTypes() skips the ErrorType from the ObjectType member and returns a useful filtered array type. In contrast, array_chunk doesn't use toBenevolentUnion, so it returns *ERROR* for the same input.

This PR's change doesn't affect the array|object case. The new $originalArrayArgType->isArray()->yes() shortcut path (line 84) only triggers when the type is purely an array. For array|object, isArray()->yes() returns false, so execution falls through to the existing getArrays() decomposition path which continues to use $arrayArgType (the benevolent-unioned version).

There are no specific tests for toBenevolentUnion behavior with non-array union types in ArrayFilterFunctionReturnTypeHelper. The inconsistency between array_filter and array_chunk for array|object input is a valid observation but pre-dates this PR and should be investigated separately if it's considered a bug.

No code changes were needed for this review comment since it was a question about pre-existing behavior. Tests and static analysis both pass cleanly.

[staabm]

do more methods exist in ArrayType which use new self() but should use $this->withTypes() instead?

[phpstan-bot]

All 12102 tests pass and static analysis is clean.

I audited all remaining new self() usages in ArrayType.php. Here's the complete inventory of what remains and why each is correct:

Line	Method	Why new self() is correct
93	withTypes()	The base implementation itself — new self() is the definition
194	generalizeValues()	Only called from ConstantArrayType override; never reaches TemplateArrayType
204	getKeysArray()	Creates a fundamentally new array (keys become values) — T no longer applies
209	getValuesArray()	Creates a fundamentally new array (re-indexed list) — T no longer applies
433, 440	setExistingOffsetValueType()	Explicit value mutation that may widen types beyond T's bound
459	unsetOffset()	Element deletion may violate T's structural constraints
482	flipArray()	Keys and values swap — completely different array structure

There are also two new ArrayType() calls in fillKeysArray() (lines 474, 477) — these create an entirely new array using the original array's values as keys, so template preservation doesn't apply.

No additional changes needed. All methods that should use $this->withTypes() already do after the earlier commits in this PR.

[VincentLanglet]

I'm not a big fan of the

TypeUtils::toBenevolentUnion($originalArrayArgType);

logic which ends up having to do

if ($originalArrayArgType->isArray()->yes()) {
				return $originalArrayArgType->filterArrayRemovingFalsey();
			}
			return TypeCombinator::union(
				...array_map([$this, 'removeFalsey'], $arrayArgType->getArrays()),
			);

while calling filterArrayRemovingFalsey should be enough.

This seems to be a fix for phpstan/phpstan#10189
but similar issue exists with other function like
https://phpstan.org/r/72402489-a8e7-4b49-95b9-f32d11dcbbe8

I feel like we should have a consistent behavior and wonder:

• If it's worth to handle maybeArray in such dynamicReturnTypeExtension
• Or if we should just:
  • either letting ErrorType
  • or just add an early return if the param is not array->yes().

Maybe instead of the if ($arrayArgType instanceof MixedType) { check ?

WDYT @staabm ?

[staabm]

I would be fine with reverting the change in src/Type/Php/ArrayFilterFunctionReturnTypeHelper.php and merge only the rest (it seems to be a separate problem which deserves its own PR).

[VincentLanglet]

Revert the changes in ArrayFilterFunctionReturnTypeHelper and the related assertions

[phpstan-bot]

Reverted the ArrayFilterFunctionReturnTypeHelper changes and removed the two related array_filter intersection-type test assertions (arrayFilterOnList, arrayFilterOnArray). The filterArrayRemovingFalsey test in ArrayTypeTemplatePreservation still passes since it goes through a different code path. All 12102 tests pass and make phpstan is clean.