Skip to content

Resolve every constant entity type ID in getStorage() return type - #1045

Merged
mglaman merged 2 commits into
mainfrom
audit/2b-getstorage-union
Sep 9, 2026
Merged

Resolve every constant entity type ID in getStorage() return type#1045
mglaman merged 2 commits into
mainfrom
audit/2b-getstorage-union

Conversation

@mglaman

@mglaman mglaman commented Sep 8, 2026

Copy link
Copy Markdown
Owner

Split out of #1027. Type inference improvement, no configuration change.

What changed

getStorage() now resolves every constant string in its argument, so getStorage($cond ? 'node' : 'user') infers NodeStorage|UserStorage instead of silently using the first string. Chained calls on that union return every branch: ->load(1) is Node|User|null, ->loadMultiple() is array<int, Node|User>, and ->create([]) is Node|User. Narrowing with instanceof on a member still works.

Three pieces make the union sound:

  • EntityStorageType compares by entity type ID. isSuperTypeOf(), equals(), and the cache-level description treat two storages with different entity type IDs as distinct even when they share a class. Without this, NodeStorage|SqlContentEntityStorage collapsed to the base class and the node branch was lost. User-facing descriptions are unchanged.
  • EntityStorageDynamicReturnTypeExtension resolves each union member on its own instead of asking resolveFromStorage() for the first registered match.
  • An unknown entity type ID falls back to the declared return type. getStorage($cond ? 'node' : 'not_an_entity_type') is EntityStorageInterface. Before, an unknown ID produced an interface type tagged with the ID, which served no consumer.

The Concat/MethodCall AST sniffing is gone; Scope::getType() already folds constant expressions. The extension declares ?Type and returns null to fall back, and no longer computes the declared return type eagerly on every call.

Known display quirk

Two config entity types that both use ConfigEntityStorage describe as ConfigEntityStorage#1|ConfigEntityStorage#2. The #N suffix is PHPStan's own rendering for union members whose names collide. They are different storages, and the chained result (Block|ConfigEntityUsingDefaultStorage) reads normally.

Testing

Fixture cases cover: the union on getStorage() itself, chained load(), loadMultiple(), and create() in both operand orders, two content storages sharing SqlContentEntityStorage, two config storages sharing ConfigEntityStorage, a content and config storage together (array<int|string, Block|Node>), a known plus unknown ID, a fully dynamic ID, a first-class callable, and instanceof narrowing. Full suite, self-analysis, and phpcs are green.

🤖 Generated with Claude Code

mglaman and others added 2 commits September 9, 2026 10:19
Return null to fall back to the declared return type instead of eagerly
computing it via ParametersAcceptorSelector, resolve every constant
string in a union instead of only the first, and drop the Concat and
MethodCall AST sniffing that the constant-strings check already
subsumes. Covers getStorage($cond ? 'node' : 'user') with a proper
union type now.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
EntityStorageType now compares by entity type ID in isSuperTypeOf(),
equals(), and the cache-level description, so a union such as
NodeStorage|SqlContentEntityStorage no longer collapses to the base
class. EntityStorageDynamicReturnTypeExtension resolves each union
member on its own, so load(), loadMultiple(), and create() on a union
storage return every branch instead of the first registered match.

An unknown entity type ID anywhere in the union falls back to the
declared return type instead of tagging the interface with an ID.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@mglaman
mglaman force-pushed the audit/2b-getstorage-union branch from b024323 to c5277e7 Compare September 9, 2026 15:24
@mglaman
mglaman marked this pull request as ready for review September 9, 2026 15:25
@mglaman
mglaman merged commit 535453a into main Sep 9, 2026
19 of 20 checks passed
@mglaman
mglaman deleted the audit/2b-getstorage-union branch September 9, 2026 15:38
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