Make Merger::merge() accept any kind of iterable

[Jean85]

This adds a refactor to the Merger class, allowing async merge of serialized coverage results (edit: by leveraging generators), for Paraunit (and similar tools) .

This would help me with facile-it/paraunit#390:

• allows to merge incoming coverage results just-in-time, and not in bulk at the end of tests execution
• it has memory advantages, because it allows me to immediately erase the serialized coverage file as soon as it's merged
• it doesn't change the logic or structure of the method, just switches from array (list) to \Generator
• I maintained backward compatibility without code duplication, just by converting the array into a generator internally

[github-actions]

API Surface Changes

If any of the additions below are not intended as public API, mark them with @internal in the docblock.

Modified API Surface

Methods

• SebastianBergmann\CodeCoverage\Serialization\Merger::merge

  - public function merge(array $paths, bool $requireMatchingGitInformation = true, bool $requireMatchingPhpVersion = true, bool $requireMatchingCodeCoverageDriver = true): array
  + public function merge(iterable $paths, bool $requireMatchingGitInformation = true, bool $requireMatchingPhpVersion = true, bool $requireMatchingCodeCoverageDriver = true): array

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.83%. Comparing base (7c1344e) to head (8f2d548).
✅ All tests successful. No failed tests found.

Additional details and impacted files

@@             Coverage Diff              @@
##               main    #1186      +/-   ##
============================================
+ Coverage     82.80%   82.83%   +0.03%     
- Complexity     1570     1574       +4     
============================================
  Files           113      113              
  Lines          5315     5326      +11     
============================================
+ Hits           4401     4412      +11     
  Misses          914      914              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[sebastianbergmann]

Is the asyncMerge() method supposed to be public?

Nevermind; sorry for the noise.

[Jean85]

Yes, it's definitely intended :D I would like to use it in Paraunit.

I've added a dedicated test for the new method.

[sebastianbergmann]

A couple of notes:

• I think the name asyncMerge() is misleading. There is nothing asynchronous here; generators are lazy/streaming, and the merge still runs synchronously in a sequential loop. Since this becomes a public method covered by the backward-compatibility promise, I would rather not ship a name that will confuse people for the lifetime of the API. Something like mergeStream() / mergeIterable() / mergeLazy() describes what it actually does.

• The body only needs to iterate, so requiring Generator specifically is more restrictive than necessary. It rejects plain Iterator/IteratorAggregate implementations a caller might already have. If the method takes iterable<non-empty-string> and the body uses a foreach with a "first item" flag rather than manual valid()/current()/next() calls, then merge() can delegate directly (an array is iterable), and the createGeneratorFor() helper disappears. Right now it only exists to wrap an array into a generator so it can be iterated back out as a generator, which is a round-trip we can avoid.

• Please also cover the distinct behavior of the new entry point with tests. At minimum an empty generator/iterable producing EmptyPathListException (the !valid() branch), and ideally one case asserting merged content through the new method.

[Jean85]

@sebastianbergmann your comments made me think that I could try a different approach: 157f66b

Since iterable includes array, I could just widen the method signature, without adding a new method. This means also that all previous tests apply, and I just left the one that explicitly tests using a generator.

Only drawback, I couldn't find a clean way to do the same as array_slice with a generic iterable, so I had to be somewhat "creative" 😅

Let me know if this is acceptable to you.

[sebastianbergmann]

Not sure what you mean by "creative", but can we get rid of the ??= operators? Thank you.

[Jean85]

Not sure what you mean by "creative", but can we get rid of the ??= operators? Thank you.

That's exactly what I meant by "creative" 😅 PHPStan complains about "possibly unassigned/null variables" because it considers the case where the iterable is empty and the loop doesn't run; that's caught by the EmptyPathListException check, but PHPStan doesn't know about that.

I've found in 8f2d548 a different approach to solve this, but I had to use a couple of asserts to silence some PHPStan issues; I did it because I couldn't reproduce the issue to report it so I expect it to be solved in future PHPStan releases.

Let me know if now it's ok for you.