Keep generic types on concrete collection classes

UPGRADE-0.2.md

@@ -100,6 +100,26 @@ Custom stores must add:
 
 Element stores extending `AbstractElementStore` already get `sortBy()` from it.
 
+## Immutable mutations of the base logic traits widen
+
+`add()`, `addFirst()` and `addAll()` of `ImmutableListLogic` and `ImmutableSetLogic` now widen the element type
+the way `ImmutableList` and `ImmutableSet` declare, also on a value typed as your own class built on them. In 0.1
+PHPStan rejected a foreign element there. A class that rebuilds itself in `newCollectionOf()` and validates its
+elements now gets such an element at runtime instead. Declare the mutations you call with your element type to
+keep them strict, or use a `SelfPreserving*Logic` trait, whose mutations are strict:
+
+```php
+/**
+ * @implements ImmutableSet<OrderItem>
+ * @method self add(OrderItem $element)
+ */
+class OrderItemCollection implements ImmutableSet
+{
+    use ImmutableSetLogic;
+    // ...
+}
+```
+
 ## Behaviour changes
 
 These do not break code written against the 0.1 documentation, but can be observed:
@@ -110,3 +130,8 @@ These do not break code written against the 0.1 documentation, but can be observ
   (a subclass of `UnsupportedOperationException`) with the original exception as its previous one.
 - Every exception the library throws implements the new `Noctud\Collection\Exception\NoctudCollectionException`,
   so a single `catch` covers them all. Existing `catch` blocks keep working.
+- On a value typed as a concrete class (`ImmutableHashSet`, `ImmutableArrayList`, your own class using the
+  logic traits), PHPStan now types several methods the way the interfaces declare them. In 0.1 such a value lost
+  the element type of `chunked()`, `windowed()`, `zip()` and `zipWithNext()`, typed `groupBy()` with a value
+  transform as groups of the original elements, and leaked an unresolved template from `add()`, `addFirst()`
+  and `addAll()` on immutable lists and sets (see above for classes that rebuild themselves).

docs/collection/extending.md

@@ -103,6 +103,7 @@ If you'd rather keep the base trait, declare `@method` overrides on the class an
  * @method self filter(Closure(OrderItem, int): bool $predicate)
  * @method self sorted()
  * @method array{self, self} partition(Closure(OrderItem, int): bool $predicate)
+ * @method self add(OrderItem $element)
  */
 class OrderItemCollection implements ImmutableSet
 {
@@ -120,6 +121,10 @@ class OrderItemCollection implements ImmutableSet
 
 Add `@method` lines only for methods that genuinely keep the same shape. This is more verbose but useful when you only need a few methods narrowed, or when you can't add a constructor accepting an `iterable`.
 
+::: warning Declare the mutations you use
+The base trait's immutable mutations widen like the interface does — `add(NE): ImmutableSet<E|NE>` — and rebuild through `newCollectionOf()`, so a foreign element would reach your constructor. Declare each mutation you call with your element type, as `add()` above: PHPStan then rejects a foreign element, and the result is typed as your class.
+:::
+
 ### What is and isn't narrowed
 
 Self-preservation applies only where the result is still a collection of the same element type:

src/CollectionLogic.php

@@ -13,6 +13,7 @@
 use LogicException;
 use Noctud\Collection\Exception\IndexOutOfBoundsException;
 use Noctud\Collection\List\ImmutableList;
+use Noctud\Collection\List\ListInterface;
 use Noctud\Collection\List\MutableList;
 use Noctud\Collection\Map\ImmutableMap;
 use Noctud\Collection\Sequence\Sequence;
@@ -519,7 +520,10 @@ public function jsonSerialize(): array
 		return $this->toArray();
 	}
 
-	/** {@inheritDoc} */
+	/**
+	 * {@inheritDoc}
+	 * @return ImmutableList<ListInterface<E>>
+	 */
 	#[NoDiscard]
 	public function chunked(int $size): ImmutableList
 	{
@@ -539,6 +543,7 @@ public function chunked(int $size): ImmutableList
 	 *
 	 * @param int $size
 	 * @param int $step
+	 * @return ImmutableList<ListInterface<E>>
 	 */
 	#[NoDiscard]
 	public function windowed(int $size, int $step = 1, bool $partialWindows = false): ImmutableList
@@ -551,14 +556,22 @@ public function windowed(int $size, int $step = 1, bool $partialWindows = false)
 		})());
 	}
 
-	/** {@inheritDoc} */
+	/**
+	 * {@inheritDoc}
+	 * @template U
+	 * @param iterable<U> $other
+	 * @return ImmutableList<array{E, U}>
+	 */
 	#[NoDiscard]
 	public function zip(iterable $other): ImmutableList
 	{
 		return $this->newListOf(new ZipOperation($this->store)->with($other));
 	}
 
-	/** {@inheritDoc} */
+	/**
+	 * {@inheritDoc}
+	 * @return ImmutableList<array{E, E}>
+	 */
 	#[NoDiscard]
 	public function zipWithNext(): ImmutableList
 	{
@@ -588,7 +601,7 @@ public function partition(Closure $predicate): array
 	 * @template V
 	 * @param Closure(E, int):K $keySelector
 	 * @param (Closure(E, int):V)|null $valueTransform
-	 * @return ImmutableMap<K, ImmutableCollection<E>>
+	 * @return ($valueTransform is null ? ImmutableMap<K, ImmutableCollection<E>> : ImmutableMap<K, ImmutableList<V>>)
 	 */
 	#[NoDiscard]
 	public function groupBy(Closure $keySelector, ?Closure $valueTransform = null): ImmutableMap

src/List/ImmutableListLogic.php

@@ -28,32 +28,23 @@ trait ImmutableListLogic
 
 	// --- Mutation (returns new) ---
 
-	/**
-	 * {@inheritDoc}
-	 * @param E $element
-	 */
+	/** {@inheritDoc} */
 	public function add(mixed $element): ImmutableList
 	{
 		$store = clone $this->store;
 		$store->add($element);
 		return $this->newCollectionOf($store); // @phpstan-ignore return.type
 	}
 
-	/**
-	 * {@inheritDoc}
-	 * @param E $element
-	 */
+	/** {@inheritDoc} */
 	public function addFirst(mixed $element): ImmutableList
 	{
 		$store = clone $this->store;
 		$store->addFirst($element);
 		return $this->newCollectionOf($store); // @phpstan-ignore return.type
 	}
 
-	/**
-	 * {@inheritDoc}
-	 * @param iterable<E> $elements
-	 */
+	/** {@inheritDoc} */
 	public function addAll(iterable $elements): ImmutableList
 	{
 		$store = clone $this->store;

src/List/ListLogic.php

@@ -479,7 +479,7 @@ public function shuffled(): ImmutableList
 	 * @template GV
 	 * @param Closure(E, int):GK $keySelector
 	 * @param (Closure(E, int):GV)|null $valueTransform
-	 * @return ImmutableMap<GK, ImmutableList<E>>
+	 * @return ($valueTransform is null ? ImmutableMap<GK, ImmutableList<E>> : ImmutableMap<GK, ImmutableList<GV>>)
 	 * @phpstan-ignore-next-line method.childReturnType
 	 */
 	#[NoDiscard]

src/Set/ImmutableSetLogic.php

@@ -28,32 +28,23 @@ trait ImmutableSetLogic
 
 	// --- Mutation (returns new) ---
 
-	/**
-	 * {@inheritDoc}
-	 * @param E $element
-	 */
+	/** {@inheritDoc} */
 	public function add(mixed $element): ImmutableSet
 	{
 		$store = clone $this->store;
 		$store->add($element);
 		return $this->newCollectionOf($store); // @phpstan-ignore return.type
 	}
 
-	/**
-	 * {@inheritDoc}
-	 * @param E $element
-	 */
+	/** {@inheritDoc} */
 	public function addFirst(mixed $element): ImmutableSet
 	{
 		$store = clone $this->store;
 		$store->addFirst($element);
 		return $this->newCollectionOf($store); // @phpstan-ignore return.type
 	}
 
-	/**
-	 * {@inheritDoc}
-	 * @param iterable<E> $elements
-	 */
+	/** {@inheritDoc} */
 	public function addAll(iterable $elements): ImmutableSet
 	{
 		$store = clone $this->store;

src/Set/SetLogic.php

@@ -11,6 +11,7 @@
 
 use Closure;
 use Noctud\Collection\CollectionLogic;
+use Noctud\Collection\List\ImmutableList;
 use Noctud\Collection\Operation\DistinctOperation;
 use Noctud\Collection\Map\ImmutableMap;
 use Noctud\Collection\Operation\DropOperation;
@@ -335,7 +336,7 @@ public function shuffled(): ImmutableSet
 	 * @template GV
 	 * @param Closure(E, int):GK $keySelector
 	 * @param (Closure(E, int):GV)|null $valueTransform
-	 * @return ImmutableMap<GK, ImmutableSet<E>>
+	 * @return ($valueTransform is null ? ImmutableMap<GK, ImmutableSet<E>> : ImmutableMap<GK, ImmutableList<GV>>)
 	 * @phpstan-ignore-next-line method.childReturnType
 	 */
 	#[NoDiscard]

tests/Collection/List/Extending/ManualLineItems.php

@@ -19,7 +19,8 @@
 
 /**
  * Extension style #2 (List): base ImmutableListLogic + class-level `@method self`
- * overrides + a newCollectionOf override.
+ * overrides + a newCollectionOf override. add() is declared too: the base trait's
+ * add() widens, which the constructor rejects.
  *
  * The constructor enforces an element invariant: it proves that transforms
  * never go through the hand-written newCollectionOf() with transformed elements.
@@ -28,6 +29,7 @@
  * @phpstan-consistent-constructor
  * @method self filter(Closure(SwappableItem, int): bool $predicate)
  * @method self sorted()
+ * @method self add(SwappableItem $element)
  */
 class ManualLineItems implements ImmutableList
 {

tests/Collection/Set/Extending/ManualItemCollection.php

@@ -18,7 +18,8 @@
 /**
  * Extension style #2: the base ImmutableSetLogic with hand-written class-level
  * `@method self` overrides + a newCollectionOf override. Param types use the
- * concrete element type, so element/closure checking is fully preserved.
+ * concrete element type, so element/closure checking is fully preserved. add()
+ * is declared too: the base trait's add() widens, which the constructor rejects.
  *
  * The constructor enforces an element invariant: it proves that transforms
  * never go through the hand-written newCollectionOf() with transformed elements.
@@ -28,6 +29,7 @@
  * @method self filter(Closure(SwappableItem, int): bool $predicate)
  * @method self sorted()
  * @method array{self, self} partition(Closure(SwappableItem, int): bool $predicate)
+ * @method self add(SwappableItem $element)
  */
 class ManualItemCollection implements ImmutableSet
 {

tests/Type/Collection/ConcreteClassTypeTest.php

@@ -0,0 +1,100 @@
+<?php
+
+/**
+ * This file is part of the Noctud Collection.
+ * Copyright (c) Noctud.dev
+ */
+
+declare(strict_types=1);
+
+namespace Noctud\Collection\Tests\Type\Collection;
+
+use Noctud\Collection\List\ArrayList\ImmutableArrayList;
+use Noctud\Collection\List\ArrayList\MutableArrayList;
+use Noctud\Collection\Set\HashSet\ImmutableHashSet;
+use Noctud\Collection\Set\HashSet\MutableHashSet;
+use Noctud\Collection\Tests\Collection\List\Extending\LineItems;
+use Noctud\Collection\Tests\Collection\List\Extending\ManualLineItems;
+use Noctud\Collection\Tests\Collection\Set\Extending\ManualItemCollection;
+use Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem;
+use Noctud\Collection\Tests\Collection\Set\Extending\TraitItemCollection;
+use function PHPStan\Testing\assertType;
+
+// The other type tests reach the library through its interfaces (listOf(), setOf(), @var).
+// A value typed as a concrete class resolves its methods from the logic traits instead, so
+// any trait docblock that diverges from the interface shows up only here: an inherited
+// @return that does not fit a narrower native type is dropped along with its generics (#49),
+// and a @param override can unbind a method template the inherited @return still uses.
+
+$immutableList = new ImmutableArrayList([1, 2, 3]);
+$mutableList = new MutableArrayList([1, 2, 3]);
+$immutableSet = new ImmutableHashSet([1, 2, 3]);
+$mutableSet = new MutableHashSet([1, 2, 3]);
+$lineItems = new LineItems([new SwappableItem(1)]);
+$items = new TraitItemCollection([new SwappableItem(1)]);
+$manualLineItems = new ManualLineItems([new SwappableItem(1)]);
+$manualItems = new ManualItemCollection([new SwappableItem(1)]);
+
+// chunked() and windowed() keep the element type of the chunks.
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $immutableList->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $mutableList->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $immutableSet->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $mutableSet->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem>>', $lineItems->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem>>', $items->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem>>', $manualLineItems->chunked(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem>>', $manualItems->chunked(2));
+
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $immutableList->windowed(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $mutableList->windowed(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $immutableSet->windowed(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<int>>', $mutableSet->windowed(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem>>', $lineItems->windowed(2));
+assertType('Noctud\Collection\List\ImmutableList<Noctud\Collection\List\ListInterface<Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem>>', $items->windowed(2));
+
+// ...so the elements of a chunk stay usable.
+assertType('int', $immutableSet->chunked(2)->first()->first());
+
+// zip() and zipWithNext() keep the pair shape.
+assertType('Noctud\Collection\List\ImmutableList<array{int, string}>', $immutableList->zip(['a', 'b']));
+assertType('Noctud\Collection\List\ImmutableList<array{int, string}>', $mutableList->zip(['a', 'b']));
+assertType('Noctud\Collection\List\ImmutableList<array{int, string}>', $immutableSet->zip(['a', 'b']));
+assertType('Noctud\Collection\List\ImmutableList<array{int, string}>', $mutableSet->zip(['a', 'b']));
+assertType('Noctud\Collection\List\ImmutableList<array{Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem, string}>', $lineItems->zip(['a', 'b']));
+assertType('Noctud\Collection\List\ImmutableList<array{Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem, string}>', $items->zip(['a', 'b']));
+
+assertType('Noctud\Collection\List\ImmutableList<array{int, int}>', $immutableList->zipWithNext());
+assertType('Noctud\Collection\List\ImmutableList<array{int, int}>', $mutableList->zipWithNext());
+assertType('Noctud\Collection\List\ImmutableList<array{int, int}>', $immutableSet->zipWithNext());
+assertType('Noctud\Collection\List\ImmutableList<array{int, int}>', $mutableSet->zipWithNext());
+assertType('Noctud\Collection\List\ImmutableList<array{Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem, Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem}>', $lineItems->zipWithNext());
+assertType('Noctud\Collection\List\ImmutableList<array{Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem, Noctud\Collection\Tests\Collection\Set\Extending\SwappableItem}>', $items->zipWithNext());
+
+// Immutable additions keep the element type, without leaking the interface's NE template...
+assertType('Noctud\Collection\List\ImmutableList<int>', $immutableList->add(4));
+assertType('Noctud\Collection\List\ImmutableList<int>', $immutableList->addFirst(0));
+assertType('Noctud\Collection\List\ImmutableList<int>', $immutableList->addAll([4, 5]));
+assertType('Noctud\Collection\Set\ImmutableSet<int>', $immutableSet->add(4));
+assertType('Noctud\Collection\Set\ImmutableSet<int>', $immutableSet->addFirst(0));
+assertType('Noctud\Collection\Set\ImmutableSet<int>', $immutableSet->addAll([4, 5]));
+assertType('int', $immutableList->add(4)->first());
+
+// ...and widen it like the interfaces do.
+assertType('Noctud\Collection\List\ImmutableList<int|string>', $immutableList->add('a'));
+assertType('Noctud\Collection\Set\ImmutableSet<int|string>', $immutableSet->add('a'));
+
+// A class built on the base trait that rebuilds itself declares add() with @method, which
+// keeps a foreign element out and the result typed as the class.
+assertType('Noctud\Collection\Tests\Collection\List\Extending\ManualLineItems', $manualLineItems->add(new SwappableItem(2)));
+assertType('Noctud\Collection\Tests\Collection\Set\Extending\ManualItemCollection', $manualItems->add(new SwappableItem(2)));
+
+// groupBy() without a value transform keeps the collection kind of the source...
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\List\ImmutableList<int>>', $immutableList->groupBy(fn (int $x): bool => $x > 1));
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\Set\ImmutableSet<int>>', $immutableSet->groupBy(fn (int $x): bool => $x > 1));
+
+// ...and with one, every group is a list of the transformed values, as the interfaces declare.
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\List\ImmutableList<bool>>', $immutableList->groupBy(fn (int $x): bool => $x > 1, fn (int $x): bool => $x % 2 === 0));
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\List\ImmutableList<bool>>', $mutableList->groupBy(fn (int $x): bool => $x > 1, fn (int $x): bool => $x % 2 === 0));
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\List\ImmutableList<bool>>', $immutableSet->groupBy(fn (int $x): bool => $x > 1, fn (int $x): bool => $x % 2 === 0));
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\List\ImmutableList<bool>>', $mutableSet->groupBy(fn (int $x): bool => $x > 1, fn (int $x): bool => $x % 2 === 0));
+assertType('Noctud\Collection\Map\ImmutableMap<bool, Noctud\Collection\List\ImmutableList<int>>', $manualItems->groupBy(fn (SwappableItem $i): bool => $i->swapped, fn (SwappableItem $i): int => $i->id));