Raise phpstan from level 3 to level 5

phpstan.neon says level 5. It reported 408 findings against the branch point and reports none now, with no baseline, no new ignoreErrors, and the one that was already there (new static(), which is how Drupal builds a plugin or a form) untouched.

Why level 5 and not 4

Level 5 is where an argument is checked against the type its signature declares, and that is where the defects were. Levels 4 and 5 arrive together, so the always-true reports came with it; the guards they named are gone rather than silenced.

What it caught

A guard that did not establish what the next line needed. HoldEngine::mutateUnderLock() checked its reloaded row against BookingInterface and then called save() on it. BookingAsk satisfies that check and has no save(): the engine keeps the two apart on purpose, an ask being a booking asked for and never written. The same shape appeared a second time in the hold itself, under a comment that already said the storage makes an entity.

One key under three names. UnitAssignmentMove carried its chosen partition as part in the docblock, area where it was written, and partition where it was read. Nothing had ever written or read part. It is not a live bug, because the provider sets the two to the same value and the reader falls back to it, and no test can see the difference; it is the next provider answering with an area other than the partition it was handed whose value would be dropped.

A counter that keeps no number. LockAnchors::getTakenByScope() declared array<string, int> while returning the multi-slot reader's array<string, int|null>. That one word made a correct NULL check read as dead code in the audit, and three assertions read as impossible.

Field rules are a list. Two interfaces declared getFieldRules() as keyed by field rule id. The config schema is a sequence and every entry names its own field, so an assertion on the real shape read as impossible.

The always-true bulk, and why the flag was not used

286 of the 408 would have gone away with treatPhpDocTypesAsCertain: false, which stops phpstan trusting an annotation when deciding a check is redundant. It was not used. Each finding was judged instead, and the answer was almost always that the guard could not fire: a value straight from a storage load, whose class the storage builds; a service out of the container, where asserting the class services.yml names is asserting that the container works; a parameter PHP had already type-checked. Several were worse than useless, skipping a row on a condition that cannot be true and turning an impossible state into a quiet wrong answer.

The deletions were verified rather than argued: with 83 guards gone at once phpstan reported no error it had not reported before, so none of them was carrying anything.

getTranslationFromContext() is worth naming because I twice argued for keeping its guards, on the strength of core's docblock saying EntityInterface|null. Reading the method settles it the other way: it opens with $translation = $entity and the only value it ever assigns instead is getTranslation(), which core declares as returning static and throwing on an unknown langcode. The docblock is stale.

A written booking says so

The largest single strand: 60-odd signatures described lists of held, releasing and looked-up bookings as BookingInterface, the type an ask also answers. Every one of those lists holds rows that exist. Nothing outside yoyaku implements these interfaces, none carries @api, and the parameters narrowed are docblocks on array parameters, which PHP does not enforce; widening again stays available, which is the direction that keeps callers working.

Tests

51 assertions could not fail, each naming a type PHP had already enforced at the call. Six were the whole subject of their test, and deleting them showed those tests had never checked what they claimed: five scenarios now count the units their order holds, and a seam test that asserted an interface relationship the compiler settles is gone. Where a test read the same call twice with a mutation in between, phpstan carried the first reading forward; each read is named for its moment now, which fixes the report and says which side of the change is being asserted.

Two things CI caught that I had got wrong

Deleting an assertion deleted a wait. assertNotNull() around waitForTheText() really was vacuous, since that answers a bool, but the call is a wait and the next line then read the control before the refusal had landed. The call stands on its own now.

A config entity's id is a string. The shared reader introduced here answers an int, and applying it to a resource type made every type nought. Only two config entity types exist in the module and one site touched either; the reader now says which half of the entity world it is for.

drupal/domain

phpstan read two guards in DomainTenantResolver differently here and on CI, because require-dev asked for ^3.0 || ^4.0 and composer locked the stable 3.0.1, where getActiveDomain() cannot answer NULL. orchestra has never seen this: its require-dev already names the dev branches. yoyaku says the same thing now. A stability flag alone does not do it, since @dev permits a dev version without preferring one.

Not in scope

The entity mapping, and the new static() ignore, which is a Drupal convention rather than something this project can fix.

AI-Generated: Yes (Claude Code took the measurements, made these changes and drafted this summary. I reviewed them before posting.)