Never-executed `match` default arm reported as covered when the `match` is the operand of `return`

[sebastianbergmann]

When a multi-line match expression whose default arm throws is used as the operand of return, the entire never-executed default arm is reported as covered. The root cause is a line-attribution defect in the PHP compiler (php/php-src#22727): the RETURN / VERIFY_RETURN_TYPE opcodes of return match (...) are stamped with the line of the last leaf of the last arm instead of the line of the return statement. php-code-coverage's branch-based executable-lines mapping then propagates that single spurious line hit to every line of the arm.

How to reproduce

src/unescape.php:

<?php declare(strict_types=1);

function unescape(string $character): string
{
    return match ($character) {
        'n'     => "\n",
        default => throw new InvalidArgumentException(
            'Unknown escape sequence: ' .
            $character,
        ),
    };
}

repro.php (with composer require phpunit/php-code-coverage):

<?php declare(strict_types=1);
require 'vendor/autoload.php';

use SebastianBergmann\CodeCoverage\CodeCoverage;
use SebastianBergmann\CodeCoverage\Driver\Selector;
use SebastianBergmann\CodeCoverage\Filter;

$file = __DIR__ . '/src/unescape.php';

require $file;

$filter = new Filter;
$filter->includeFile($file);

$coverage = new CodeCoverage(
    (new Selector)->forLineCoverage($filter),
    $filter,
);

$coverage->start('test');
unescape('n');
$coverage->stop();

$lines = $coverage->getData()->lineCoverage()[$file];

ksort($lines);

foreach ($lines as $line => $tests) {
    printf(
        "%2d: %s\n",
        $line,
        $tests === null ? 'not executable' : ($tests === [] ? 'NOT covered' : 'covered'),
    );
}

Run with XDEBUG_MODE=coverage php repro.php.

unescape('n') executes only the 'n' arm on line 6; the default arm (lines 7–10) is never executed.

Actual result

 6: covered
 7: covered
 8: covered
 9: covered
10: covered

The entire never-executed default arm (lines 7–10) is reported as covered. The HTML report paints it green accordingly (and, confusingly, correctly paints non-default sibling arms of larger match expressions red; see "Real-world example" below).

Expected result

 6: covered
 7: NOT covered
 8: NOT covered
 9: NOT covered
10: NOT covered

Analysis

Two mechanisms combine:

1. PHP attributes the RETURN opcode to a line inside the default arm

The raw Xdebug data for the same run contains a single spurious line:

 6:  1      'n' arm — correct
 7: -1      default arm start — correct (not executed)
 9:  1      <-- WRONG: last operand of the concatenation inside the never-executed default arm
12: -2

The phpdbg opcode dump shows why. The VERIFY_RETURN_TYPE / RETURN opcodes of return match (...) are stamped with CG(zend_lineno) as it stands after the whole match expression has been compiled: the line of the last leaf of the last arm, line 9:

L0006 0006 JMP 0016            ; the executed 'n' arm jumps to the shared epilogue
...
L0009 0016 VERIFY_RETURN_TYPE T2   ; should be L0005 (the line of the return statement)
L0009 0017 RETURN T2               ; should be L0005

Every arm's JMP targets these shared opcodes, so executing any arm executes opcodes attributed to line 9. Xdebug (and PCOV) faithfully report what the compiler attributed; this part is a php-src issue, filed as php/php-src#22727. It is not specific to match (a multi-line ternary as return operand shows the same artifact) and is unaffected by OPcache optimization. It does not occur when the match result is assigned to a variable that is then returned.

2. php-code-coverage propagates the spurious hit across the whole arm

ExecutableLinesFindingVisitor::enterMatch() maps every line of an arm's body ($arm->body->getStartLine() … $arm->body->getEndLine()) to one branch, so lines 7–10 form a single branch. RawCodeCoverageData::markExecutableLineByBranch() then copies the status of any raw-covered line to all lines of its branch. This propagation exists for a good reason, for a genuinely executed multi-line arm only the lines that carry opcodes get raw hits, but it also amplifies the one spurious hit on line 9 into "lines 7–10 covered".

The sibling arms are separate branches and correctly remain uncovered, which is why real-world reports look inconsistent: some uncovered arms red, the uncovered default arm green.

Real-world example

In a parser class with

private function unescape(string $character): string
{
    return match ($character) {                          // line 144
        'n'     => "\n",                                 // covered (executed)
        't'     => "\t",                                 // red (not executed) — correct
        '"'     => '"',                                  // red (not executed) — correct
        '\\'    => '\\',                                 // red (not executed) — correct
        default => throw new ParserException(            // lines 149-154: GREEN — wrong,
            sprintf(                                     // never executed
                'Unbekannte Escape-Sequenz: \\%s',
                $character,
            ),
        ),
    };
}

and a test suite that never feeds an unknown escape sequence, the HTML report shows lines 149–154 as covered while 146–148 are correctly shown as not covered. The raw Xdebug data contains exactly one spurious executed line (152, the $character, argument line) which markExecutableLineByBranch() expands to the whole arm.

Discussion

The root fix belongs in php-src: stamp the RETURN / VERIFY_RETURN_TYPE opcodes with the return statement's own line. Once that lands, the raw data no longer contains a hit inside the arm and the report is correct without any change here.

Whether php-code-coverage can or should defend against it in the meantime is debatable: after markExecutableLineByBranch() the spurious signal is indistinguishable from a genuine one, and a heuristic such as "only propagate from an arm's first line" would be unsound in the other direction (an arm's first line is not guaranteed to carry an opcode). At minimum this issue documents the behavior and links the upstream report.

Workaround for affected code: assign the match result to a variable and return that ($result = match (...) {...}; return $result;) — the spurious line hit disappears entirely.

[Caligone]

Thanks for the detailed analysis. The enterMatch() / markExecutableLineByBranch() mechanism you describe also manifests in the opposite direction, and that variant breaks 100%-coverage gates rather than just displaying a wrong green.

Reproducer (php-code-coverage 14.2.3, PHPUnit 13.2.4, PHP 8.5.8, Xdebug 3.5.0):

enum T { case A; case B; }

function assigned(T $t): string
{
    $clause = match ($t) {
        T::A => <<<SQL
            NAMED
        SQL,
        default => <<<SQL
            DEFAULT
        SQL,
    };
    return $clause;
}

function simple(T $t): string
{
    $clause = match ($t) {
        T::A => 'NAMED',
        default => 'DEFAULT',
    };
    return $clause;
}

Driving only the default arm (assigned(T::B); simple(T::B);):

 7: covered        |     $clause = match ($t) {
 8: covered        |         T::A => <<<SQL      ← never executed, reported covered
10: covered        |         SQL,
11: NOT covered    |         default => <<<SQL   ← ONLY executed arm, reported NOT covered
13: NOT covered    |         SQL,
15: covered        |     return $clause;
21: covered        |     $clause = match ($t) {
22: covered        |         T::A => 'NAMED',    ← never executed, reported covered
23: covered        |         default => 'DEFAULT',
25: covered        |     return $clause;

Two observations that may be relevant to the scoping discussion in your last section:

1. The documented workaround does not help here. assigned() already assigns the match to a variable instead of returning it, and the misattribution still occurs. The trigger appears to be the multi-line arm body (heredoc), not return match specifically simple(), whose arms are single-line, is unaffected.

2. The false-negative direction has a harder failure mode. A wrongly-green arm is a silent gap; a wrongly-red one fails a CI gate on code that is genuinely covered, with no in-code workaround available. In our case a PHPUnit 12.5 → 13.2 bump (php-code-coverage 12.5 → 14.2) dropped a 100%-covered suite to 6683/6690 methods, with zero production-code changes every lost line is a default => <<<SQL arm in SQL builders that the tests do execute.

This looks like the same family as #1154 / #1155 (fixed in 14.1.7 via branchOperatorLines), with the heredoc arm variant surviving that fix.

Happy to open a separate issue if you'd prefer to track the false-negative direction independently.

[sebastianbergmann]

As far as I am concerned, this is primarily a bug in PHP that is tracked in php/php-src#18985 (more information in php/php-src#22727).

@iluuu1994 is working on a solution for that bug in PHP.