Skip to content

Keep generic types on concrete collection classes - #56

Merged
delacry merged 1 commit into
0.2.xfrom
fix/concrete-collection-types
Oct 29, 2026
Merged

delacry merged 1 commit into
0.2.xfrom
fix/concrete-collection-types

Conversation

@delacry

@delacry delacry commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

Fixes #49.

A value typed as a concrete class (ImmutableHashSet, ImmutableArrayList, or your own class using the logic traits) got worse PHPStan types than the same value typed as its interface. Some trait docblocks didn't match the interfaces, and our type tests only went through the interfaces, so nobody noticed.

/** @param ImmutableHashSet<int> $set */
$set->chunked(2);
// before: ImmutableList
// now:    ImmutableList<ListInterface<int>>

$set->add(4);
// before: ImmutableSet<int|NE>  (unresolved template)
// now:    ImmutableSet<int>

$set->groupBy(fn (int $x): bool => $x > 1, fn (int $x): bool => $x % 2 === 0);
// before: ImmutableMap<bool, ImmutableSet<int>>  (wrong, runtime gives lists of bools)
// now:    ImmutableMap<bool, ImmutableList<bool>>

What changed:

  • chunked(), windowed(), zip() and zipWithNext() in CollectionLogic declare their generic return types. The Collection interface stays as it is: ImmutableList<ListInterface<E>> fits its ListInterface<ListInterface<E>>, so PHPDocs that spell out the result keep working.
  • groupBy() in the traits uses the same conditional return type as the interfaces.
  • add(), addFirst() and addAll() on immutable lists and sets widen the type as documented, instead of leaking NE.

One thing can break, described in UPGRADE-0.2.md: a class built on the base traits that rebuilds itself and validates its elements can now receive a foreign element from add(). Declare @method self add(YourType $element) to keep it strict. The extending guide and test fixtures show this.

The new tests/Type/Collection/ConcreteClassTypeTest.php fails on 0.2.x and passes here.

@codecov

codecov Bot commented Oct 10, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@delacry

delacry commented Oct 13, 2026 •

Copy link
Copy Markdown
Member Author

I realized that my previous fix went too far, as I narrowed few return types to ImmutableList in the Collection..

So I force-pushed smaller fix, Collection keeps ListInterface as the return type of chunked(), windowed(), zip() and zipWithNext().. only CollectionLogic declares the deneric types now, so a concrete class gets ImmutableList<ListInterface<E>> for chunks

@nikophil if you already tried the branch - chunks on concrete classes are now typed ImmutableList<ListInterface<int>> instead of ImmutableList<ImmutableList<int>>, and ->first()->first() is still int

#57 is rebased on top, with no changes

A value typed as a concrete class (ImmutableHashSet, ImmutableArrayList,
or a class built on the logic traits) resolves its methods from the
traits, where a docblock that diverges from the interface went
unnoticed: every type test reached the library through the interfaces.

chunked(), windowed(), zip() and zipWithNext() returned a bare
ImmutableList. Collection declared ListInterface<...>, which does not
fit the traits' native ImmutableList, so PHPStan dropped it with its
generics. CollectionLogic now declares them itself. chunked() and
windowed() narrow only the outer list, to ImmutableList<ListInterface<E>>:
the element type is invariant, so ImmutableList<ImmutableList<E>> would
not fit the interface's ListInterface<ListInterface<E>>. Collection stays
unchanged, and a PHPDoc that spells out the result keeps working.

groupBy() in ListLogic, SetLogic and CollectionLogic dropped the
interfaces' conditional return type, so with a value transform the
groups were typed as the original elements while they hold the
transformed values in an ImmutableList.

add(), addFirst() and addAll() on immutable lists and sets overrode the
interface's @PARAM NE with E, which left NE in the inherited return
type unresolved and rejected the documented widening. They now widen as
documented. A class that rebuilds itself through newCollectionOf() and
validates its elements keeps them strict with @method; the extending
and upgrade guides show how, and the extending fixtures do it.

Fixes #49
@delacry
delacry force-pushed the fix/concrete-collection-types branch from 1c1d888 to 2131a40 Compare October 26, 2026 23:47
@delacry
delacry merged commit e1c3f6d into 0.2.x Oct 29, 2026
6 checks passed
@delacry
delacry deleted the fix/concrete-collection-types branch October 29, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant