Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Closed
thirsch opened this issue Oct 3, 2023 · 3 comments · Fixed by #1015
Closed

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

thirsch opened this issue Oct 3, 2023 · 3 comments · Fixed by #1015
Labels
type/bug

Comments

@thirsch
Copy link
Contributor

thirsch commented Oct 3, 2023

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
Copy link
Contributor Author

thirsch commented Oct 3, 2023 •
edited

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
Copy link
Owner

@dvdoug @Slamdunk What do you think?

@Slamdunk
Copy link
Contributor

Slamdunk commented Oct 4, 2023

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

@thirsch thirsch changed the title Incorrect statement count in clover coverage report for constructor property promotion Incorrect statement count in coverage report for constructor property promotion Oct 4, 2023
@Slamdunk Slamdunk mentioned this issue Oct 4, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type/bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix new in initializers Slamdunk/php-code-coverage
3 participants
@sebastianbergmann @Slamdunk @thirsch