Code coverage changed between 9.2.17 and 9.2.18

[kukulich]

phpunit/php-code-coverage 9.2.17

phpunit/php-code-coverage 9.2.18

The problem is that infection/infection now think the line is not covered and reports mutants.

With phpunit/php-code-coverage 9.2.17: 10 mutants were not covered by tests
With phpunit/php-code-coverage 9.2.18: 68 mutants were not covered by tests

29) /home/runner/work/BetterReflection/BetterReflection/src/Reflection/StringCast/ReflectionAttributeStringCast.php:27    [M] Concat

--- Original
+++ New
@@ @@
     {
         $arguments = $attributeReflection->getArguments();
         $argumentsFormat = $arguments !== [] ? " {\n  - Arguments [%d] {%s\n  }\n}" : '';
-        return sprintf('Attribute [ %s ]' . $argumentsFormat . "\n", $attributeReflection->getName(), count($arguments), self::argumentsToString($arguments));
+        return sprintf($argumentsFormat . 'Attribute [ %s ]' . "\n", $attributeReflection->getName(), count($arguments), self::argumentsToString($arguments));
     }

[Slamdunk]

There's a lot to talk about here.

First: IMHO infection shouldn't generate mutants for lines that php-code-coverage doesn't consider executable.
Maybe this should be "resolved" by using --only-covered infection's flag, I'm not really sure, but that's none of php-code-coverage business

Second: is 'Attribute [ %s ]' . $argumentsFormat . "\n" and executable line?
I've done some research, and yes it should be because $argumentsFormat could undergo a typecasting during Concat that may generate errors (like a __toString() class), so indeed we should take this into account

Third: does a fix for the second point resolve the issue completely? No.
Take into consideration this example:

/* 1 */    $var
/* 2 */        =
/* 3 */            'a'
/* 4 */            .
/* 5 */            'b'
/* 6 */    ;

The only real executable line is 1 according to xDebug and PCOV, but Infection\Mutator\Operator\Concat doesn't care and Infection will ALWAYS report 'a' . 'b' to 'b' . 'a' mutation as uncovered.
And this would also apply to a plethora of Mutators where code heavily relies on multilines.

Conclusion: a fix is needed to count variables as executable lines during Concat operations, but use --only-covered with Infection otherwise we can't help further

[sebastianbergmann]

IMHO infection shouldn't generate mutants for lines that php-code-coverage doesn't consider executable

Agreed.

[theofidry]

IIRC the reason why infection does it by default is a mix of historical reasons (humbug was doing so as well) and because the goal is to "make sure your tests are good". Since you cannot assert that your code is covered, allowing to mutate non covered codes allows to check that (since you can make infection fail if you don't meet the desired MSI score).

Also there is an argument to be made that some mutants could be killed by other things than tests, such as phpstan/psalm type violation or an obscure black box bash script. Although I think we should be able to manage those scenarios regardless of the flag at some point.

[Slamdunk]

Posted a bug on Infection: infection/infection#1750

Meanwhile, I'll do my best to have meaningful CC here in the upcoming days

[sebastianbergmann]

Meanwhile, I'll do my best to have meaningful CC here in the upcoming days

Thank you so much for your work on this!

[mvorisek]

see mvorisek#2 (comment)

for example:

u(
$a,
$b . 'x',
);

xdebug outputs (/w opcache) only line 1 as executable (covered/uncovered) if $a and $b is known ($a/b = const expr)

if line 2 and line 3 should be covered, we need to analyse if variable is comming from const expr or not.

Title should be changed to something like "Impl. local variable static analysis and add coverage line for non-const variable". We can then prune also definitively non-reachable if branches.

[kukulich]

Fixed in #964