sebastianbergmann / php-code-coverage

Library that provides collection, processing, and rendering functionality for PHP code coverage information.
BSD 3-Clause "New" or "Revised" License
8.76k stars 370 forks source link

Incorrect statement count in coverage report for constructor property promotion #1014

Closed thirsch closed 9 months ago

thirsch commented 9 months ago
Q A
php-code-coverage version 10.1.6
PHP version 8.2.8
Driver not relevant
Installation Method Composer
Usage Method PHPUnit
PHPUnit version (if used) 10.3.5

If using constructor property promotion in a single line while defining a new instance of an object, the clover report does not contain a line of type stmt but counts it as a statement in the metrics. On the other hand, every method is mentioned on it's own but I'm not able to distinguish a constructor with an kind of inline statement from a regular method without.

I hope the following two examples make it more clear. I've written both of them as unittests in php-code-coverage, but I'm not sure what the correct solution might be? Should there be a second line for the same num with type stmt? Is that even allowed in the clover format?

Example 1 (single line)

<?php

class CoveredClassWithConstructorPropertyPromotion
{
    public function __construct(private DateTimeInterface $dateTime = new DateTime()) {

    }
}

Generated clover.xml:

<?xml version="1.0" encoding="UTF-8"?>
<coverage generated="%i">
  <project timestamp="%i">
    <file name="%s%esource_with_class_and_constructor_property_promotion.php">
      <class name="CoveredClassWithConstructorPropertyPromotion" namespace="global">
        <metrics complexity="1" methods="1" coveredmethods="1" conditionals="0" coveredconditionals="0" statements="2" coveredstatements="2" elements="3" coveredelements="3"/>
      </class>
      <line num="5" type="method" name="__construct" visibility="public" complexity="1" crap="1" count="1"/>
      <line num="8" type="stmt" count="1"/>
      <metrics loc="10" ncloc="10" classes="1" methods="1" coveredmethods="1" conditionals="0" coveredconditionals="0" statements="2" coveredstatements="2" elements="3" coveredelements="3"/>
    </file>
    <metrics files="1" loc="10" ncloc="10" classes="1" methods="1" coveredmethods="1" conditionals="0" coveredconditionals="0" statements="2" coveredstatements="2" elements="3" coveredelements="3"/>
  </project>
</coverage>

Please note the attributes "statements" and "coveredstatements" both being 2, but only one line of type stmt.

Example 2 (with linebreak)

<?php

class CoveredClassWithConstructorPropertyPromotion
{
    public function __construct(
        private DateTimeInterface $dateTime = new DateTime()
    ) {

    }
}

Generated clover.xml:

<?xml version="1.0" encoding="UTF-8"?>
<coverage generated="%i">
  <project timestamp="%i">
    <file name="%s%esource_with_class_and_constructor_property_promotion.php">
      <class name="CoveredClassWithConstructorPropertyPromotion" namespace="global">
        <metrics complexity="1" methods="1" coveredmethods="1" conditionals="0" coveredconditionals="0" statements="2" coveredstatements="2" elements="3" coveredelements="3"/>
      </class>
      <line num="5" type="method" name="__construct" visibility="public" complexity="1" crap="1" count="1"/>
      <line num="6" type="stmt" count="1"/>
      <line num="9" type="stmt" count="1"/>
      <metrics loc="11" ncloc="11" classes="1" methods="1" coveredmethods="1" conditionals="0" coveredconditionals="0" statements="2" coveredstatements="2" elements="3" coveredelements="3"/>
    </file>
    <metrics files="1" loc="11" ncloc="11" classes="1" methods="1" coveredmethods="1" conditionals="0" coveredconditionals="0" statements="2" coveredstatements="2" elements="3" coveredelements="3"/>
  </project>
</coverage>
thirsch commented 9 months ago

I've just played a bit further with it. The second example is wrong, if using real coverage. As I was using RawCodeCoverageData::fromXdebugWithoutPathCoverage() and marked line 6 as covered in the tests, it generated the given output of the second example.

It doesn't count the promoted property at all, if in multi-line mode. That's true for PCOV and Xdebug. But for single line, it makes no difference to use PCOV or Xdebug for coverage.

So the correct fix seems to be not counting the line as a statement?

The same behavior is true for the html reporting. If using a single line, it is counted as a statement, if using multi line, not.

sebastianbergmann commented 9 months ago

@dvdoug @Slamdunk What do you think?

Slamdunk commented 9 months ago

I can confirm the bug:

image

So the correct fix seems to be not counting the line as a statement?

Exactly: the constructor countent is already handled correctly, there should be no CC line for ctor arguments in both cases.

I'd mark this as a low priority bug though: no false-positive nor false-negative possible, so I'll propose a fix once I'm done with my current job tasks, hopefully in a week