Skip to content

Let immutable map puts widen on concrete classes - #57

Open
delacry wants to merge 1 commit into
fix/concrete-collection-typesfrom
fix/immutable-map-widening
Open

delacry wants to merge 1 commit into
fix/concrete-collection-typesfrom
fix/immutable-map-widening

Conversation

@delacry

@delacry delacry commented Oct 10, 2026

Copy link
Copy Markdown
Member

Stacked on #56, so merge that one first. After it's merged, I'll rebase this onto 0.2.x and retarget it.

This fixes the same problem as #56, but for maps. On a concrete map class, some trait docblocks didn't match the map interfaces:

/** @param ImmutableHashMap<string, int> $map */
$map->put($id, $flag);  // int $id, bool $flag
// before: PHPStan error
// now:    ImmutableMap<int|string, bool|int>, like the interface

$map->containsKey($id);
// before: PHPStan error
// now:    fine, every map interface accepts any key

$map->toArray();
// before: array<int>
// now:    array<string, int>

It also fixes a runtime bug: intMapOf([1 => 'a'])->putIfAbsent('b', 'x') threw InvalidKeyTypeException, while put() falls back to a hash map. putIfAbsent() on immutable IntMap and StringMap now falls back the same way.

Docs:

  • The FAQ, the map guide and the class docblocks said every foreign key throws. Only the mutable IntMap and StringMap throw.
  • The extending guide shows how a map class that rebuilds itself can keep put() strict with @method.
  • UPGRADE-0.2.md is updated.

The new tests/Type/Map/ConcreteClassTypeTest.php and the putIfAbsent_rejects_* tests fail without this change.

@delacry
delacry added this pull request to stack #58 October 10, 2026 12:10
@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!

A value typed as a concrete map (ImmutableHashMap, MutableHashMap, or a
class using the map logic traits) resolves its methods from the traits,
whose docblocks diverged from the map interfaces.

put(), putFirst(), putIfAbsent(), putAll() and putAllPairs() overrode
the ImmutableMap interface's widening NK/NV with the map's own K/V, so
PHPStan rejected a put the interface allows. They now inherit the
interface docblock. A class that rebuilds itself through newMapOf() and
validates its entries keeps them strict with @method; the extending and
upgrade guides show how, and the extending fixture does it.

containsKey(), containsValue() and remove() narrowed the key or value
to K or V, while every map interface accepts any; they inherit the
interfaces now too.

putIfAbsent() on an immutable IntMap or StringMap threw
InvalidKeyTypeException for a key of another type, while put() falls
back to a hash map. Both classes now override it like put() and its
siblings. The FAQ, the map guide and their class docblocks said every
foreign key throws, which only the mutable variants do.

toArray() was typed array<array-key, V> on every map, mutable ones
included, losing the key type the interface keeps. Inheriting the
interface's conditional is not enough: with a still generic K, as on
ImmutableHashMap, PHPStan falls back to a bare array, so the trait
repeats it.
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