Issue #3619890: Raise PHPStan to level 5 and remove the dead code it finds
Raises the project's PHPStan configuration from level 3 to level 5.
Level 5 reported 160 findings. They resolved into four groups.
Dead code removed (130)
- 43
instanceofguards that can never be false, each after aloadMultiple()orloadByProperties()whose elements already are the entity type asked for. The three guards inInteractionResolverthat read$tokens[$id] ?? NULLfirst are kept: those subjects really can be NULL, and the analysis does not report them, which is what makes the distinction reliable. - 25 null-coalesce operators on keys the node, flow and variable shapes declare required. The config schema requires the same keys, so the shape and the stored data agree.
- 14 test assertions whose value already carries the asserted type.
- Two always-true
assert()calls inTaskActions::getFamily(), a deadcatchwith its unreachable tail inTenantDeletionTest, and twois_array()calls on values already known to be arrays.
Defects the run surfaced (4)
OrchestraCmFeatureAuthoringTestassertedassertNotNull()onwaitForText(), which returnsFALSEon timeout. The test passed whether or not the save landed.RolesVariableAudienceTestdeclared?inton a closure whose(int)cast could never produce NULL, defeating the?->it was written around.ApiControllerforwardedlimitandoffsetas strings where the client's declared shape asks forint.- Three kernel tests passed a method return value to
reset(), which PHP takes by reference.
Return types narrowed (23)
Six are methods a class declares for itself and that promised more than they returned. The other 17 implement a wider interface. Narrowing an implementation is covariance, so a caller holding the interface still sees the wider type, and each one records in its docblock why this implementation is narrower.
AccountRecipient is deliberately left wider. It carries @api, and docs/extending.md promises that @api signatures do not change breakingly within 1.x, 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 is still ignored (7), and why
Every entry is scoped to the files it applies to, so each rule keeps working everywhere else.
FormStateInterface::getUserInput()is documented as returning an array and returns NULL on aFormStatenothing has populated yet. Removing the coalesce errors three kernel classes with "must be of type array, null given".Sql::addWhere()documents$groupas a string while its own body maps'',0and NULL onto the default group, and core passes the integer0to it in nine places.- Three assertions the analysis folds to a constant because it 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. Each is a real regression guard at run time.
- The two
AccountRecipientsignatures above.
reportUnmatchedIgnoredErrors is left at its default, so an entry that stops matching is reported rather than sitting stale.
Verification
phpstanclean at level 5 with the project's ownphpstan.neon.phpcsclean over the whole module againstDrupalandDrupalPractice.- Kernel suite: 182 of 183 classes pass.
PaymentWorkflowSubscriberTest::testInstanceDeletionDeletesPinnedPaymentsfails identically on unmodified1.x, so it is not from this branch.