Skip to content

Match coverage to the methods it belongs to - #8

Open
dillon7f wants to merge 8 commits into
mainfrom
dillon7f/5/coverage-matching
Open

dillon7f wants to merge 8 commits into
mainfrom
dillon7f/5/coverage-matching

Conversation

@dillon7f

Copy link
Copy Markdown

This addresses some edge cases at join between coverage and complexity analysis. Coverage was being dropped for most methods the C# compiler rewrites. Also addresses solutions with more than one test project, especially in the case where one project references methods from another project.

dillonharless18 and others added 8 commits September 9, 2026 16:09
The C# compiler rewrites async and iterator bodies into a generated state
machine type named "Outer/<Method>d__N", and Coverlet reports coverage
against MoveNext on that type. NormalizeClassName only converted the nested
type separator, so the resulting key could never match the method in the
source and the left-outer-join default of 0.0 applied instead.

Because CRAP cubes the uncovered fraction, that turned fully covered async
methods into the worst-scoring methods in a report. On a ~1,700 method
ASP.NET Core project it accounted for the bulk of 50 false positives out of
60 methods flagged over threshold; after this change that project drops from
60 flagged methods to 18, and crap load from 525 to 160.

Recognise the state machine pattern and attribute MoveNext back to the
owning method. The generated type carries no parameter list, so the
recovered key is emitted without a signature and resolves through the
existing name-only fallback pass.

Generic methods are still unmatched: the Roslyn side normalizes
GetPracticeRights<TWorkflowStatus> to GetPracticeRights<>, while the state
machine spells it <GetPracticeRights>d__9`1. That is handled next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A generic method's state machine carries the method's arity as a backtick
suffix on the generated type ("<Get>d__9`1"), while the Roslyn side spells
the type parameters out and normalizes them to angle brackets ("Get<>").
The recovered owner key used the bare method name, so the two never met and
generic async methods stayed at the 0.0 default.

Carry the arity through the existing NormalizeBacktickGenerics conversion so
both sides reduce to the same canonical form.

Non-generic async methods were already handled by the previous commit. Plain
generic methods remain unmatched for a different reason: they have no state
machine, and Coverlet writes no arity on the method name at all, so the
arity cannot be recovered from the coverage entry. That needs a
generic-erased fallback in the matcher and is handled separately.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A non-async generic method has no state machine, so it appears in Cobertura
as an ordinary method, and Coverlet writes no arity on the method name at
all: "ApplyFilterRules", where the Roslyn side normalizes the type
parameters to "ApplyFilterRules<>". The arity is therefore unrecoverable
from the coverage entry, and neither the exact-key pass nor the name-only
pass could bridge the difference.

Add a third fallback keyed on the name with the method's generic arity
erased from both sides. The declaring type keeps its arity, which both
sides do supply. The pass carries the same single-candidate guard as the
name-only fallback, so it still declines to guess between overloads, and
its matches are excluded from the orphan count.

Completes the compiler-rewritten method work: on the same ~1,700 method
project this takes the report from 17 flagged methods to 13, and crap load
from 151 to 113. Against the original v0.1.1 behaviour that is 60 flagged
methods down to 13, and crap load 525 down to 113.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A solution with several test projects emits one coverage file each, and
--coverage accepts them all. The entries are flattened into a single list,
so the same method legitimately appears more than once. The exact-key pass
took exactMatches[0], which made the reported coverage depend on the order
the files were passed on the command line.

Take the maximum instead. A method covered by one test project is covered,
regardless of how many other projects loaded the assembly without
exercising it. Averaging would let an indifferent test project dilute a
real result, which is the failure mode this is meant to prevent.

This addresses only the exact-key pass. The name-only fallback still drops
duplicated methods to zero via its single-candidate guard, which is the
larger effect and is handled next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Matching async and iterator methods means dropping the signature, because a
generated state machine does not carry one. That makes overloads collide on
a single key, and the name-only pass could only respond by refusing every
candidate, so async overloads scored zero however well tested they were.

The generated type does keep the source positions of the method it was
rewritten from, and Cobertura records them. Carry the first line through on
CoberturaMethodCoverage, then pair colliding source methods to coverage
entries in line order.

Pairing applies only when the two sides line up exactly: equal counts, line
information on both sides, and matching filenames. Anything less certain
falls through to the existing default, so an uncertain method is reported as
uncovered rather than being handed another overload's result. Hiding an
untested overload is the worse failure.

The same resolver merges entries that differ only in which coverage file
they came from, which is the exact-key fix extended to this pass. A method
covered by one test project is covered regardless of how many other
projects loaded the assembly without exercising it.

Verified against JobPostCreator.ExecuteAsync, whose two overloads at lines
29 and 43 now report 1.0 and 0.7142 from the state machines starting at
lines 31 and 45. That project has nine overload groups whose entries carry
different coverage.

The arity-erased pass still carries the old guard and is handled next.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The arity-erased pass still carried the single-candidate guard the
name-only pass has dropped, so a generic method arriving from more than one
coverage file fell through to the 0.0 default.

Route it through the shared resolver, with its own sibling grouping. The
erased key collapses Apply<T> and Apply onto one entry, so reusing the
name-only grouping would pair a method against the wrong sibling list and
silently hand it another method's coverage.

Completes the multiple-coverage-file work. On the reference project all
three orderings of two coverage files now agree at 10 flagged methods and
a crap load of 91, where v0.1.1 reported 60, 101 and 164 depending on which
files were passed and in what order.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
"Let crap4dotnet run tests and generate coverage automatically" reads as
though the tool measures coverage itself. It does not: --run-tests shells
out to dotnet test --collect:"XPlat Code Coverage" and reads the Cobertura
XML that Coverlet writes, which is why a test project without a
coverlet.collector reference silently contributes no coverage at all.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
the existing nuget.config restricts sources by enabling package source mapping. This disallows the --add-source command. We really just want to ensure the code in the PR builds and installs, we point it to a different config during ci to get around the conflict.
@dillon7f
dillon7f marked this pull request as ready for review September 14, 2026 14:52

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants