Raise PHPStan to level 5 and remove the dead code it finds

The project phpstan.neon runs at level: 3 and is clean there. That level was reached in #3618612: Raise PHPStan to level 3 with an entity mapping, because the analysis runs at level 0 and lets a wrong type through, which added the file together with the entity_mapping.neon that makes it affordable. The next two levels are where the analysis stops checking types and starts checking code: level 4 reports conditions and branches that can never do anything, and level 5 checks the type of every argument passed to a function or a method.

Measured on this module

Against Drupal 11.3.16 with phpstan 2.2.9 and phpstan-drupal 2.1.2, on 1.x at 82b155a1, keeping the existing entity mapping and the existing new static ignore, over the whole module:

• level 3: 0 findings, which is what the job reports today.
• level 4: 152 findings.
• level 5: 160 findings, the same 152 plus 8.

Level 5 is a strict superset of level 4 here, so there is no reason to stop at 4. 101 files carry at least one finding; 121 of the 160 are in production code and 39 in tests. Both phpstan jobs read the same phpstan.neon, so the next-major lane moves with the main one.

What the 160 are

• 43 instanceof.alwaysTrue
• 34 return.unusedType
• 16 method.alreadyNarrowedType
• 15 nullCoalesce.offset
• 11 identical.alwaysFalse
• 7 nullCoalesce.expr
• 7 method.impossibleType
• 6 notIdentical.alwaysTrue
• 5 argument.type, level 5 only
• 5 function.alreadyNarrowedType
• 4 nullCoalesce.property
• 3 argument.byRef, level 5 only
• 2 booleanOr.alwaysTrue
• 1 catch.neverThrown
• 1 deadCode.unreachable

The ones that are defects

• OrchestraViewsTenantScopeHooks passes the integer 0 as the $group argument of Sql::addWhere(), which core declares as a string.
• ApiController hands listInstances() an array whose shape does not match the one OrchestraClientInterface declares.
• The easy_email and mail notification tests build an array of RecipientInterface where OrchestraNotificationEvent asks for Recipient. Either the constructor or the tests are wrong about which one the event carries.
• Three kernel tests call reset() on a method return value, which PHP takes by reference.
• TaskActions::getFamily() asserts that a token is NULL or a TokenInterface, and that a handler is NULL or a PendingActionHandlerInterface. Both getters already promise exactly that, so both asserts are always true and both are dead weight.
• 10 of the 34 return.unusedType name a non-null type the method never produces, among them respond() declaring an array in EntityInteraction and in CommentInteraction, and three methods declaring a Response they never return. The other 24 are nullable returns, and most of those are not defects; see below.
• 16 test assertions assert a type the value already has, and 7 assert one the value cannot have. Both kinds prove nothing about the code under test, and the 7 are worth reading closely before they are changed.

The ones that need a decision first

Not every finding is a defect. Three groups are the analyzer reading Drupal more narrowly than Drupal behaves, and each wants a decision here rather than an ignore.

• phpstan-drupal types an entity reference target_id as a non-nullable string, so every getter that reads one looks like it can never return NULL. That single quirk accounts for at least 19 findings: the 7 getters Incident::getInstanceId() and getTokenId(), Token::getInstanceId(), getParentId() and getForkId(), Variable::getTokenId() and WorkItem::getInstanceId(), plus 12 of the 17 always-true and always-false comparisons. Their ?int is right and an empty reference really does give NULL at runtime, so narrowing those return types would introduce the very bug phpstan thinks it found. The fix is to ask the field item whether it is empty. That count comes from matching the source line at each finding, so read it as a floor rather than an exact figure.
• 42 of the 43 instanceof.alwaysTrue are in production code, and they hold only because entity_mapping.neon names the concrete class behind each entity type id. Dropping the guard is usually right once the NULL from a failed load is handled on its own, but that is a judgement per call site and not a sweep.
• The remaining 17 nullable returns are interface contracts rather than typos. PendingActionHandlerInterface::getActionUrl() declares ?Url and documents the NULL as the case where the node exposes no page to act on it; OperationActionHandler simply always has a page. Narrowing each implementation is legal and says the wrong thing about the interface, so this is a call per interface.
• TenantDeletionTest catches a LogicException that phpstan says is never thrown, and then reports the rest of the method unreachable, because $this->fail() never returns and core documents no throws on EntityInterface::delete(). The try, fail, catch shape is core's own, so this is about how the test is written.

What was done

Fixed in !414. Of the 160, 130 were dead code and are removed, 4 were defects and are fixed, 23 were return types wider than the implementation behind them and are narrowed, and 7 remain ignored with the reason recorded beside each entry in phpstan.neon.

The four defects

• OrchestraCmFeatureAuthoringTest asserted assertNotNull() on waitForText(), which returns FALSE on timeout, so that test passed whether or not the save it was waiting for ever landed.
• RolesVariableAudienceTest declared ?int on a closure whose (int) cast could never produce NULL, defeating the ?-> it was written around.
• ApiController forwarded limit and offset as strings where the client's declared shape asks for int.
• Three kernel tests passed a method return value to reset(), which PHP takes by reference.

The return types, and why one class kept its wider one

Six of the 23 are methods a class declares for itself. The other 17 implement a wider interface, which the rule reports only because those classes are final: it asks whether a subclass could return something else, and a final class has none. Narrowing an implementation is covariance, so a caller holding the interface still sees the wider type, and each narrowed method records in its docblock why this implementation is narrower.

AccountRecipient deliberately keeps its wider signatures. It carries @api, and docs/extending.md promises that @api signatures do not change breakingly within the 1.x line, so a narrowing that costs nothing to write now would be a breaking change to undo. RecipientInterface keeps getAccount() and label() nullable and the concrete class follows it.

What stays ignored

Seven findings are the analysis being wrong about code that is right, so each is ignored with its reason and scoped to the files it applies to, leaving the rule working everywhere else. FormStateInterface::getUserInput() is documented as returning an array and returns NULL on a FormState nothing has populated yet; removing the coalesce errors three kernel classes with "must be of type array, null given". Sql::addWhere() documents its group argument as a string while its own body maps the empty string, 0 and NULL onto the default group, and core passes the integer 0 to it in nine places. Three assertions are folded to a constant because the analysis cannot model what changes between them: a service resolved from the container, config read back after a form submit, and an event collector a dispatch appends to. The last two are the AccountRecipient signatures above.

Verified: phpstan clean at level 5 with the project's own phpstan.neon, phpcs clean over the whole module, and 182 of the 183 kernel classes pass. The one failure, PaymentWorkflowSubscriberTest::testInstanceDeletionDeletesPinnedPayments, fails identically on unmodified 1.x and is not from this branch.

AI-Generated: Yes (Claude Code was used to help draft this issue summary and to write the change on the merge request. I reviewed the work and ran phpstan, phpcs and the kernel suite myself before posting it.)