CodeCoverage depends on autoload order

[Slamdunk]

Q	A
php-code-coverage version	9.2.10
PHP version	8.1.2
Driver	Tested on both PCOV & Xdebug
Xdebug version	3.1.2
Installation Method	Composer
Usage Method	PHPUnit
PHPUnit version	9.5.13

Hi, currently CodeCoverage depends on code execution order.
I created a self contained repo that shows this behavior in https://github.com/Slamdunk/phpunit_codecoverage_bug#readme

Basically, given the following code:

/* 1 */ class A
/* 2 */ {
/* 3 */     public function inArray(): bool
/* 4 */     {
/* 5 */         return \in_array(1, [
/* 6 */             AType::OPT,
/* 7 */         ], true);
/* 8 */     }
/* 9 */ }

1. lines 6 and 7 are not considered LOC if AType was already loaded and parsed before the A is used in the test
2. otherwise if AType is unknown to PHP during inArray call, lines 6 and 7 are considered LOC.

This messes CodeCoverage merging in context like ParaTest where test execution is isolated but not deterministic.

Merging two CC reports coming from different tests run in different processes can result in something like this:

Which doesn't make much sense

I don't know if this library can do much about this, maybe @krakjoe and @derickr could help

[dvdoug]

Can you try changing processUncoveredFiles="true" to processUncoveredFiles="false" and see if that helps?

(not sure if it will in this example thinking it through, but it has helped in some other cases of inconsistencies)

[dvdoug]

Noting that it's class constants involved here which form part of the class definition, I'm thinking that for Xdebug this might relate to/be another example of https://bugs.xdebug.org/view.php?id=1798 which affects extends and implements?

[sebastianbergmann]

CC @krakjoe @derickr

[Slamdunk]

Can you try changing processUncoveredFiles="true" to processUncoveredFiles="false" and see if that helps?

This has no effect on the underlying issue

Noting that it's class constants involved here which form part of the class definition, I'm thinking that for Xdebug this might relate to/be another example of https://bugs.xdebug.org/view.php?id=1798 which affects extends and implements?

That's likely a similar issue: the solution I proposed in #892 should also cover your #775 indeed

[Slamdunk]

With the merge of #892, IIRC, the todo list now is:

1. Unify ParsingCoveredFileAnalyser and ParsingUncoveredFileAnalyser
2. Add memory level cache, both to speed things and and address Windows filesystem issues

[sebastianbergmann]

Agreed. Do you want to work on this, Filippo? Otherwise I'll try to work on this over the weekend.

[Slamdunk]

As soon as I'm able to code something, I'll open a PR on draft: if you don't see any it means I haven't started it yet so you can take that chance

[sebastianbergmann]

Cheers!

[sebastianbergmann]

@Slamdunk I just pushed 5698195 to the refactor-static-analysis branch. It should work, but one test currently fails.

[Slamdunk]

The failing diff relates to:

php-code-coverage/tests/_files/source_without_namespace.php

Lines 12 to 18 in 6882cdb

	function &foo($bar)
	{
	$baz = function () {};
	$a = true ? true : false;
	$b = "{$a}";
	$c = "${b}";
	}

And this:

php-code-coverage/src/StaticAnalysis/IgnoredLinesFindingVisitor.php

Lines 92 to 94 in 8c95e20

	if ($node instanceof ClassMethod || $node instanceof Function_) {
	return NodeTraverser::DONT_TRAVERSE_CHILDREN;
	}

Blocks all visitors, including ExecutableLinesFindingVisitor, to analyze that function's content, even thought it shouldn't.

I'm trying some fixes

[sebastianbergmann]

Ah, of course. Did not think about the usage of NodeTraverser::DONT_TRAVERSE_CHILDREN here. We probably can no longer use this optimization anymore.

[sebastianbergmann]

Should be taken care of in 115a62b.