Dead code still gets maintained

Code whose result nobody reads still gets maintained. In PHPUnit, that was a few lines in TestRunner::run() that had no effect after April 2023. They shipped in more than 300 releases of PHPUnit 10, 11, 12 and 13. In that time I reformatted them, moved them to a new file, adapted them to a renamed import and hardened them for a stricter level of static analysis. When I deleted them on 6 October 2026, they still cost me two merge conflicts.

PHPStan 2.3 found them on the day it was released, with a new check for assignments whose value nobody reads. I keep that check switched on, even though every update can bring new findings. It only points at the end of a dead chain, though. What else was dead, I only found when I read the code and its history.

First the value died, then the variable

The lines once had two jobs. The older one dates from January 2011. Back then, an invalid @covers annotation aborted the entire test run, and issue #63 asked for an error on the affected test instead. From version 3.6 onwards, PHPUnit therefore caught the exception from the code coverage library and made it the test's exception, unless the test had already raised one of its own. In 2018 this became the line $e = $e ?? $cce;.

The younger job dates from June 2016. Since PHPUnit 5.5, a failed assert() in the code under test counts as a test failure and no longer as an error. For that, PHP's AssertionError had to become PHPUnit's AssertionFailedError, because TestResult::addFailure() accepted nothing else. Since the assert() is in the code under test and not in the test, I appended the file and line to the message. Without them, the message would not have told you where the failed assert() was. Up to PHPUnit 9, the following code used the value of $e:

if ($error) {
    $this->addError($test, $e, $time);
} elseif ($failure) {
    $this->addFailure($test, $e, $time);
} elseif ($warning) {
    $this->addWarning($test, $e, $time);
}

In PHPUnit 10, test results are no longer collected in a TestResult object but derived from events. TestCase::runBare() catches an AssertionError itself, reports the original exception with the Test\Failed event and rethrows it. So the catch block in TestRunner::run() is still reached. It still has to mark the test as failed and count the assert() as an assertion. But nobody sees the message it builds any more. Since 7 August 2022, the end-to-end test for this case has expected the original message followed by the location from the stack trace.

On 8 August 2022 I deleted Framework\TestResult. With the class went addError() and addFailure(), the last places that needed the value of $e. What remained was code that only wanted to know whether $e was set at all. “The exception to report” had become “an exception occurred”. In that role, $e = $e ?? $cce; still had an effect: after an exception from the code coverage library, it made sure $e was set. The assignment of new AssertionFailedError(...) had no effect any more, because $e was already set in that catch block.

On 22 April 2023, for #5340, I removed an early return. Because of it, not all events were emitted for a test that failed or errored. With it went the last place that checked $e after the try/catch. From PHPUnit 10.1.2 onwards, both assignments to $e were dead. Nothing in the code marked these changes. The variable was still called $e, and the lines that wrote to it looked the same as ever.

The last reader took a signal with it

That $e had come to mean nothing more than “an exception occurred” had a consequence that surfaced sooner than the dead code. Before #5340, run() returned as soon as $e was set. A test that had stopped with an error therefore never reached the check for whether it had performed any assertion at all. After #5340 it did reach that check, and the check did not look at $error. From PHPUnit 10.1.2 to 10.2.4, a test that errored before its first assertion was therefore also reported as risky.

This was already visible in the commit for #5340. It changed the expectations of 16 existing end-to-end tests, and in one of them Risky: 1 became Risky: 4. That is the kind of change to existing tests that I described as a reason to pause in “Untouched tests are half the proof”. The bug was reported in July 2023 as #5443. I fixed it the same day with a !$error && in the condition, and the fix shipped in PHPUnit 10.2.5.

When PHPStan 2.3 analyses the code as it was in PHPUnit 10.1.2, it reports not only $e = $e ?? $cce; but also the assignment $error = true in the code coverage block as never read. That dead assignment is a symptom of the bug, not the bug itself. It says: after this line, nobody looks at $error any more. That is what #5443 was about.

A live class protects dead code

In August 2022, I deleted dead code right next to these lines twice. On 12 August, catch (Warning $e) { $warning = true; } went, together with the class Framework\Warning. On 23 August, $e = new ExceptionWrapper($e); went, together with the class Framework\ExceptionWrapper. That assignment was a dead store of exactly the same kind as $e = new AssertionFailedError(...): an object built for a consumer that no longer existed. It was only a few lines further down.

I deleted both because I deleted their classes. Whoever deletes a class has to touch every place that uses it, or static analysis reports an error. AssertionFailedError, on the other hand, is alive to this day: every failed assertion in PHPUnit throws it or one of its subclasses. Nothing forced me to look at the place where an instance of it was built and immediately forgotten. Dead code that only uses live classes does not stand out when you clean up.

Lines without effect still need maintenance

Between 22 April 2023 and 6 October 2026, the lines had no effect. They were touched anyway:

The reformatting came from an extended trailing comma rule in PHP-CS-Fixer's configuration that changed 358 files in one merge. The move was part of the groundwork for swappable implementations of process isolation. The rename comes from a commit with the message “Delete unused code”. It deleted the unused class Framework\CodeCoverageException, which freed its name for the alias of the exception from php-code-coverage. It left the dead code directly below where it was. Finally, in April 2026, I hardened $e->getTrace()[0] because PHPStan at level 7 reported a possibly non-existent array offset. So I protected code against a failure case although nobody used its result.

I had a vague suspicion every time I passed these lines. I never followed it up. TestRunner::run() is the method every test runs through, and I did not want to change anything there without a good reason. A suspicion was not enough for that. So each of these changes stuck to its own purpose, and none of them asked whether the lines were still needed at all.

Even the deletion still cost work. I committed it to 11.5 and merged it from there into 12.5, 13.4 and main. The merges into 12.5 and 13.4 ran into conflicts because the dead lines no longer looked the way they did in 11.5: in 12.5 because of the renamed alias, in 13.4 because of the hardening from April. I had to resolve both conflicts by hand.

PHPStan reports only the end of a chain

PHPStan 2.3.0 was released on 6 October 2026. At 08:49 I updated the PHAR on 11.5 from 2.2.16 to 2.3.0. PHPUnit has included bleeding edge since November 2024, that is, the rules and stricter checks that PHPStan only switches on for everyone with its next major version. Markus Staab contributed that at the time. With 2.2.16, PHPStan reported no errors on 11.5; with 2.3.0 it reported six. Without bleeding edge it would have been only one.

The other five come from the new detection of unused variables in PHPStan 2.3. It walks the statements of a function body backwards and checks, for every assignment, whether any path reads its value before it is overwritten or the function ends. Compilers have used this liveness analysis for decades to remove unnecessary assignments. There, an assignment whose value is not read on any path is called a dead store.

PHP's optimiser eliminates dead code as well, as I showed in “How PHP optimises bytecode”. It never got anywhere near these lines. TestRunner::run() contains try and catch, and since PHP 7.1 the optimiser has skipped every pass that builds on SSA form for functions with exception handlers; the source code has a “TODO” at that point. But even without try, it would have left the assignments in place: it does not let an object that might have a destructor die earlier than the code says, and it calls a constructor in any case. On the command line, the optimiser only runs at all if somebody switches on opcache.enable_cli. So PHP executed the dead code on every failed assert(): it built a new exception object with its own stack trace and threw it away immediately.

In TestRunner::run(), PHPStan reported only $e = $e ?? $cce;, with the message Value assigned to variable $e is never read. It did not report the assignment of new AssertionFailedError(...), because its value is read: by $e ?? $cce. PHPStan recognises that this read itself only flows into a dead value, but deliberately reports only the end of the chain. A comment in PHPStan's source code justifies this by saying that this one finding already points at the whole chain.

If you delete only the reported line, a second run reports the assignment of new AssertionFailedError(...). If you delete that too, PHPStan reports nothing more. $frame is still read, after all, by assert(isset($frame['file'])) and assert(isset($frame['line'])), and a function call counts as a use. I wrote these two lines in May 2022 with the commit message “Be paranoid”, because Psalm had complained about a possibly non-existent array offset. Assertions that I wrote in 2022 for one static analysis tool hid the rest of the dead code from another one in 2026.

PHPStan does not report the variables in the three catch clauses either, even though nothing reads them afterwards. Dropping such a variable changes the syntax, because since PHP 8.0 a catch clause can do without one. PHPStan deliberately leaves that decision to a human.

I found out with git blame and git log, not with PHPStan, that the lines had been dead since Framework\TestResult was deleted. After that, I also deleted $frame with its two assertions, the import of assert() and the three catch variables:

 } catch (AssertionFailedError $e) {
     $failure = true;
 
     if ($e instanceof IncompleteTestError) {
         $incomplete = true;
     } elseif ($e instanceof SkippedTest) {
         $skipped = true;
     }
-} catch (AssertionError $e) {
+} catch (AssertionError) {
     $test->addToAssertionCount(1);
 
     $failure = true;
-    $frame   = $e->getTrace()[0];
-
-    assert(isset($frame['file']));
-    assert(isset($frame['line']));
-
-    $e = new AssertionFailedError(
-        sprintf(
-            '%s in %s:%s',
-            $e->getMessage(),
-            $frame['file'],
-            $frame['line'],
-        ),
-    );
-} catch (Throwable $e) {
+} catch (Throwable) {
     $error = true;
 }
 
 // ...
 
-    } catch (OriginalCodeCoverageException $cce) {
+    } catch (OriginalCodeCoverageException) {
         $error = true;
-
-        $e = $e ?? $cce;
     }

What remains has an effect: $failure = true, $error = true and the additional assertion that is counted for a failed assert(). On 11.5, the commit removes 19 lines and adds 3. The deletion first shipped on 12 October 2026 in PHPUnit 12.5.39 and PHPUnit 13.4.2.

The other five findings from the update were smaller. Two foreach loops in TestCase restored error and exception handlers and iterated over values they never read (foreach.valueOverwritten). Now two for loops call restore_error_handler() and restore_exception_handler() as many times as there are active handlers. Twice, the code assigned a value that the variable already had (assign.redundant). And once, isset() checked a variable that always exists and is never null (isset.variable). At 09:01, twelve minutes after the update, five commits had fixed all six findings.

Code coverage took the dead code for tested

Code coverage would not have helped. The end-to-end test that I wrote in 2016 together with the conversion of AssertionError into AssertionFailedError still exists. It executes every executable line of the catch block, the dead assignment included. In the code coverage report for this one test, the block is entirely green.

Since August 2022, however, the test has checked a message that PHPUnit produces somewhere else. Line coverage measures whether a line was executed, not whether its result ends up anywhere. A test that executes code without checking its result makes that code look tested. I described in more detail what code coverage measures and what it does not in “Path Coverage or Mutation Testing?”.

I keep the check switched on

PHPUnit had a comparable check once before. From 2019 to 2024 I analysed PHPUnit with Psalm, and in July 2020 I switched on its findUnusedVariablesAndParams option. In November 2020 I switched it off again, in a commit with the message “Tweak”. I no longer remember why. With this option, Psalm 6.20 reports both dead assignments in the code before the deletion in a single run.

Checks for unused variables come at a price. They also report assignments that are intentional. With bleeding edge, every PHPStan update can bring new findings that have to be fixed or added to the baseline before the next green build. How many there are depends on the codebase; for PHPUnit, there were six. For an assignment you want to keep on purpose, there is a way out: PHPStan ignores variables whose name starts with an underscore.

I keep the check switched on anyway. Its findings only show where the reading starts, though. PHPStan pointed at two assignments. How long they had been dead and which other lines belonged to them, I had to find out myself. Whether this pays off in your project depends on how many findings an update brings and whether somebody has the time to follow them up on the same day. For PHPUnit, it was twelve minutes on a Tuesday morning against three and a half years in which I maintained lines without effect, and against a regression that a check for dead stores would have pointed to.