Follow-up to #3619890: Raise PHPStan to level 5 and remove the dead code it finds, which raised phpstan to level 5 and added the ignores that made that possible. One of them looks retirable.

The ignore covers two test classes and explains itself as "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". That reading is right about the cause: phpstan remembers the type of a call expression and carries it forward, so a second reading of the same expression is judged against what the first assertion proved, not against what the mutation in between did.

It does not need an ignore, though. Reading each call into a variable named for the moment it belongs to gives phpstan a fresh expression to type, and says at the call site which side of the mutation is being asserted:

$sweep->fireNow($token);
$after_first = $events;
$this->assertCount(1, $after_first, 'The timeout fired once.');

$sweep->fireNow($this->findParkedToken($instance_id));
$after_second = $events;
$this->assertCount(1, $after_second, 'The timeout did not fire a second time.');

Every read has to be named, including the first: it is the earliest assertion that pins the expression, so leaving that one inline keeps the later ones folded.

Measured

With the method.alreadyNarrowedType block removed from phpstan.neon, the two classes report 2 findings. With three reads named in WorkflowVariablesUiTest (lines 68, 85 and 112 on 1.x) and two in NotifyTimeoutTest (174 and 180), both classes report none, and the block can go. Checked against 1c7d4eb4 with the rest of phpstan.neon unchanged.

Not proposed here

The other ignores in that file look right and are left alone. The getUserInput() coalesce stands on a core docblock that is wrong where removing the guard breaks three kernel classes at run time, which no annotation fixes honestly. AccountRecipient carries @api, so its wider signature is a promise rather than an oversight. RemoteOrchestraClientTest reads always-true on one lane and always-false on the next-major lane, which no single edit can satisfy.

Tests were not run for this: the change is to which variable an assertion reads, and the assertions keep their arguments.

AI-Generated: Yes (Claude Code found this while raising phpstan in another project, reproduced it here and drafted this summary. I reviewed it before posting.)

Issue fork orchestra-3619912

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

mably created an issue. See original summary.

mably’s picture

Status: Active » Needs review

  • mably committed c4593aac on 1.x
    task: #3619912 Retire the method.alreadyNarrowedType ignore by naming...
mably’s picture

Status: Needs review » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.