From cb9d2739192bf3e55e04d80f72d753c5058cb1d5 Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Wed, 9 Sep 2026 16:09:57 -0400 Subject: [PATCH 1/8] fix: attribute async and iterator coverage to the source method The C# compiler rewrites async and iterator bodies into a generated state machine type named "Outer/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 to GetPracticeRights<>, while the state machine spells it d__9`1. That is handled next. Co-Authored-By: Claude Opus 5 (1M context) --- .../Matching/CoberturaMethodParser.cs | 36 ++++++++++++++++++- .../Matching/MethodCoverageMatcherTests.cs | 20 +++++++++++ 2 files changed, 55 insertions(+), 1 deletion(-) diff --git a/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs b/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs index eb4df5b..9603a20 100644 --- a/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs +++ b/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs @@ -1,3 +1,4 @@ +using System.Text.RegularExpressions; using Crap4DotNet.Core.Coverage; namespace Crap4DotNet.Core.Matching; @@ -7,7 +8,7 @@ namespace Crap4DotNet.Core.Matching; /// Converts CLR type names to C# keywords, backtick generics to angle brackets, /// nested type separators, and accessor/operator naming conventions. /// -public static class CoberturaMethodParser +public static partial class CoberturaMethodParser { private static readonly Dictionary ClrToCSharpTypes = new(StringComparer.Ordinal) { @@ -64,12 +65,45 @@ public static class CoberturaMethodParser /// public static string ToCanonicalKey(CoberturaMethodCoverage coverage) { + if (TryGetStateMachineOwnerKey(coverage.ClassName, coverage.MethodName, out var ownerKey)) + return ownerKey; + var className = NormalizeClassName(coverage.ClassName); var methodName = NormalizeMethodName(coverage.MethodName, className); var signature = NormalizeSignature(coverage.Signature); return $"{className}.{methodName}{signature}"; } + /// + /// Async and iterator bodies are compiled into a generated state machine type named + /// "Outer/<MethodName>d__N", and coverage is reported against MoveNext on that + /// type rather than against the method in the source. Recover the owning method so + /// the entry can be matched to it. + /// + /// + /// The generated type carries no parameter information, so the key is emitted + /// without a signature and resolves through the name-only fallback pass. + /// + private static bool TryGetStateMachineOwnerKey(string className, string methodName, out string key) + { + key = string.Empty; + + // Only MoveNext holds the rewritten body; the rest of the type is plumbing. + if (!string.Equals(methodName, "MoveNext", StringComparison.Ordinal)) + return false; + + var match = StateMachineClassRegex().Match(className); + if (!match.Success) + return false; + + var owner = NormalizeClassName(match.Groups["owner"].Value); + key = $"{owner}.{match.Groups["method"].Value}"; + return true; + } + + [GeneratedRegex(@"^(?.+)/<(?[^>]+)>d__\d+(?:`\d+)?$")] + private static partial Regex StateMachineClassRegex(); + private static string NormalizeClassName(string className) { // Nested type separator: / → . diff --git a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs index 1479db4..fb625fb 100644 --- a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs +++ b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs @@ -390,4 +390,24 @@ public void ResultsPreserveComplexityOrder() result.Methods.Select(m => m.Coverage) .Should().Equal(0.3, 0.1, 0.2); } + + [Fact] + public void AsyncMethod_CoverageOnCompilerGeneratedStateMachine_IsAttributedToSourceMethod() + { + // The C# compiler rewrites an async body into a state machine class named + // "d__N", so Coverlet reports the coverage against MoveNext on + // that generated type rather than against the method the developer wrote. + var complexity = new[] { MakeComplexity("UpdateAsync", signature: "(int)") }; + var coverage = new[] + { + MakeCoverage( + "MoveNext", + className: "MyApp.Service/d__5", + coverage: 1.0) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Single().Coverage.Should().Be(1.0); + } } From 633be7f76b48a1d7ff254b336f6e39f15ab5d09d Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Thu, 10 Sep 2026 13:20:36 -0400 Subject: [PATCH 2/8] fix: match generic async methods by state machine arity A generic method's state machine carries the method's arity as a backtick suffix on the generated type ("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) --- .../Matching/CoberturaMethodParser.cs | 13 ++++++++++-- .../Matching/MethodCoverageMatcherTests.cs | 20 +++++++++++++++++++ 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs b/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs index 9603a20..d729b55 100644 --- a/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs +++ b/src/Crap4DotNet.Core/Matching/CoberturaMethodParser.cs @@ -97,11 +97,20 @@ private static bool TryGetStateMachineOwnerKey(string className, string methodNa return false; var owner = NormalizeClassName(match.Groups["owner"].Value); - key = $"{owner}.{match.Groups["method"].Value}"; + var method = match.Groups["method"].Value; + + // A generic method's state machine carries the method's arity as a backtick + // suffix, where the source side spells the type parameters out. Both sides + // reduce to the same angle-bracket form. + var arity = match.Groups["arity"].Value; + if (arity.Length > 0) + method += MethodKeyHelper.NormalizeBacktickGenerics(arity); + + key = $"{owner}.{method}"; return true; } - [GeneratedRegex(@"^(?.+)/<(?[^>]+)>d__\d+(?:`\d+)?$")] + [GeneratedRegex(@"^(?.+)/<(?[^>]+)>d__\d+(?`\d+)?$")] private static partial Regex StateMachineClassRegex(); private static string NormalizeClassName(string className) diff --git a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs index fb625fb..c1195a7 100644 --- a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs +++ b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs @@ -410,4 +410,24 @@ public void AsyncMethod_CoverageOnCompilerGeneratedStateMachine_IsAttributedToSo result.Methods.Single().Coverage.Should().Be(1.0); } + + [Fact] + public void GenericAsyncMethod_StateMachineArity_IsAttributedToSourceMethod() + { + // A generic async method's state machine carries the method's generic arity as + // a backtick suffix ("d__9`1"), while the source side spells the type + // parameters out ("Get"). Both have to reach the same canonical form. + var complexity = new[] { MakeComplexity("Get", signature: "(int)") }; + var coverage = new[] + { + MakeCoverage( + "MoveNext", + className: "MyApp.Service/d__9`1", + coverage: 1.0) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Single().Coverage.Should().Be(1.0); + } } From be1b3aa06fe7df45dce04747d4869cb278179d59 Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Thu, 10 Sep 2026 13:22:30 -0400 Subject: [PATCH 3/8] fix: match generic methods whose coverage entry carries no arity 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) --- .../Matching/MethodCoverageMatcher.cs | 26 +++++++++++++++++++ .../Matching/MethodIdentityNormalizer.cs | 20 ++++++++++++++ .../Matching/MethodCoverageMatcherTests.cs | 14 ++++++++++ 3 files changed, 60 insertions(+) diff --git a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs index 0238395..477fadd 100644 --- a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs +++ b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs @@ -17,6 +17,7 @@ public static MatchResult Match( // Build coverage lookups by normalized key var fullKeyLookup = new Dictionary>(StringComparer.Ordinal); var nameKeyLookup = new Dictionary>(StringComparer.Ordinal); + var erasedKeyLookup = new Dictionary>(StringComparer.Ordinal); foreach (var entry in coverageEntries) { @@ -25,10 +26,17 @@ public static MatchResult Match( var nameKey = MethodKeyHelper.GetNameOnlyKey(fullKey); AddToLookup(nameKeyLookup, nameKey, entry); + + var erasedKey = MethodKeyHelper.EraseMethodGenericArity(nameKey); + if (!string.Equals(erasedKey, nameKey, StringComparison.Ordinal)) + AddToLookup(erasedKeyLookup, erasedKey, entry); + else + AddToLookup(erasedKeyLookup, nameKey, entry); } var matchedFullKeys = new HashSet(StringComparer.Ordinal); var matchedNameKeys = new HashSet(StringComparer.Ordinal); + var matchedErasedKeys = new HashSet(StringComparer.Ordinal); var methods = new List(); var unmatchedNames = new List(); var warnings = new List(); @@ -62,6 +70,20 @@ public static MatchResult Match( continue; } + // Pass 3: Generic methods carry no arity on the Cobertura side, so fall + // back to a key with the method's arity erased from both sides. + var erasedKey = MethodKeyHelper.EraseMethodGenericArity(nameKey); + if (erasedKeyLookup.TryGetValue(erasedKey, out var erasedMatches) && erasedMatches.Count == 1) + { + matchedErasedKeys.Add(erasedKey); + methods.Add(new MatchedMethod + { + Complexity = complexity, + Coverage = erasedMatches[0].Coverage + }); + continue; + } + // No match found → default to 0.0 unmatchedNames.Add(complexity.Identity.FullName); methods.Add(new MatchedMethod @@ -85,6 +107,10 @@ public static MatchResult Match( if (matchedNameKeys.Contains(nameKey)) continue; + // Check if matched by the generic-erased fallback + if (matchedErasedKeys.Contains(MethodKeyHelper.EraseMethodGenericArity(nameKey))) + continue; + orphanedCount += kvp.Value.Count; orphanedNames.AddRange( kvp.Value.Select(c => $"{c.ClassName}.{c.MethodName}")); diff --git a/src/Crap4DotNet.Core/Matching/MethodIdentityNormalizer.cs b/src/Crap4DotNet.Core/Matching/MethodIdentityNormalizer.cs index 8123776..3194964 100644 --- a/src/Crap4DotNet.Core/Matching/MethodIdentityNormalizer.cs +++ b/src/Crap4DotNet.Core/Matching/MethodIdentityNormalizer.cs @@ -74,6 +74,26 @@ public static List SplitTypeList(string typeList) return result; } + /// + /// Remove the generic arity marker from the method-name portion of a name-only key. + /// MyApp.Cache<>.Get<> -> MyApp.Cache<>.Get + /// + /// + /// Coverlet writes no arity on a generic method's name, so the arity cannot be + /// recovered from the coverage side. Erasing it from both sides gives them a + /// common key. The declaring type keeps its arity, which both sides do supply. + /// + public static string EraseMethodGenericArity(string nameOnlyKey) + { + var lastDot = nameOnlyKey.LastIndexOf('.'); + if (lastDot < 0) + return nameOnlyKey; + + var methodName = nameOnlyKey[(lastDot + 1)..]; + var angle = methodName.IndexOf('<'); + return angle < 0 ? nameOnlyKey : string.Concat(nameOnlyKey.AsSpan(0, lastDot + 1), methodName.AsSpan(0, angle)); + } + /// /// Convert CLR backtick generic arity notation to angle bracket notation. /// Cache`1 → Cache<>, Dictionary`2 → Dictionary<,> diff --git a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs index c1195a7..7b95775 100644 --- a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs +++ b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs @@ -428,6 +428,20 @@ public void GenericAsyncMethod_StateMachineArity_IsAttributedToSourceMethod() var result = MethodCoverageMatcher.Match(complexity, coverage); + result.Methods.Single().Coverage.Should().Be(1.0); + } + [Fact] + public void GenericMethod_CoverageEntryCarriesNoArity_IsStillMatched() + { + // A non-async generic method has no state machine, and Coverlet writes no + // arity on the method name, so the entry is a bare "Apply". The source side + // normalizes type parameters to "Apply<>". The arity cannot be recovered from + // the coverage entry, so matching has to erase it from the source key instead. + var complexity = new[] { MakeComplexity("Apply", signature: "(int)") }; + var coverage = new[] { MakeCoverage("Apply", signature: "(System.Int32)", coverage: 1.0) }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + result.Methods.Single().Coverage.Should().Be(1.0); } } From fa6af6041a8e76d586bde17fa957aa4ed9332699 Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Thu, 10 Sep 2026 13:38:21 -0400 Subject: [PATCH 4/8] fix: merge duplicate coverage entries instead of taking the first 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) --- .../Matching/MethodCoverageMatcher.cs | 5 ++++- .../Matching/MethodCoverageMatcherTests.cs | 18 ++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs index 477fadd..4a6a73d 100644 --- a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs +++ b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs @@ -51,8 +51,11 @@ public static MatchResult Match( matchedFullKeys.Add(fullKey); methods.Add(new MatchedMethod { + // One method can appear in several coverage files. Covered by any + // of them means covered, and picking the first would make the + // result depend on the order the files were passed in. Complexity = complexity, - Coverage = exactMatches[0].Coverage + Coverage = exactMatches.Max(m => m.Coverage) }); continue; } diff --git a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs index 7b95775..874399f 100644 --- a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs +++ b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs @@ -444,4 +444,22 @@ public void GenericMethod_CoverageEntryCarriesNoArity_IsStillMatched() result.Methods.Single().Coverage.Should().Be(1.0); } + + [Fact] + public void SameMethodInTwoCoverageFiles_TakesTheHigherCoverage() + { + // A solution with several test projects emits one coverage file each, and a + // project that merely references the assembly under analysis reports its + // classes at zero. Which file was passed first must not decide the answer. + var complexity = new[] { MakeComplexity("Run") }; + var coverage = new[] + { + MakeCoverage("Run", coverage: 0.0), + MakeCoverage("Run", coverage: 1.0) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Single().Coverage.Should().Be(1.0); + } } From a93edf05a76c1f392db16362c0444da3ad3620e1 Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Thu, 10 Sep 2026 14:31:19 -0400 Subject: [PATCH 5/8] fix: tell overloads apart by source position when the signature is gone 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) --- .../Coverage/CoberturaCoverageReader.cs | 20 ++++ .../Coverage/CoberturaMethodCoverage.cs | 8 ++ .../Matching/MethodCoverageMatcher.cs | 104 +++++++++++++++++- .../Matching/MethodCoverageMatcherTests.cs | 86 ++++++++++++++- 4 files changed, 212 insertions(+), 6 deletions(-) diff --git a/src/Crap4DotNet.Core/Coverage/CoberturaCoverageReader.cs b/src/Crap4DotNet.Core/Coverage/CoberturaCoverageReader.cs index b1939c0..9b15b8a 100644 --- a/src/Crap4DotNet.Core/Coverage/CoberturaCoverageReader.cs +++ b/src/Crap4DotNet.Core/Coverage/CoberturaCoverageReader.cs @@ -48,6 +48,7 @@ private static List ReadFromDocument(XDocument doc) MethodName = methodName, Signature = signature, FileName = filename, + StartLine = FirstLineNumber(method) ?? FirstLineNumber(cls), Coverage = coverage }); } @@ -79,6 +80,25 @@ private static double SelectCoverage(XElement method) return ParseDouble(branchRateAttr.Value); } + /// + /// Lowest source line number recorded under an element, if any. + /// + private static int? FirstLineNumber(XElement element) + { + int? lowest = null; + foreach (var line in element.Descendants("line")) + { + var attr = line.Attribute("number"); + if (attr is null) + continue; + if (int.TryParse(attr.Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var number) + && (lowest is null || number < lowest)) + lowest = number; + } + + return lowest; + } + private static double ParseDouble(string value) => double.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out var result) ? result diff --git a/src/Crap4DotNet.Core/Coverage/CoberturaMethodCoverage.cs b/src/Crap4DotNet.Core/Coverage/CoberturaMethodCoverage.cs index a9c7691..71123b5 100644 --- a/src/Crap4DotNet.Core/Coverage/CoberturaMethodCoverage.cs +++ b/src/Crap4DotNet.Core/Coverage/CoberturaMethodCoverage.cs @@ -11,5 +11,13 @@ public sealed record CoberturaMethodCoverage public required string MethodName { get; init; } public required string Signature { get; init; } public string? FileName { get; init; } + + /// + /// First source line this entry describes, where the report provides one. + /// Compiler-generated types keep the positions of the method they were rewritten + /// from, which is what makes overloads separable once the signature is gone. + /// + public int? StartLine { get; init; } + public required double Coverage { get; init; } } diff --git a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs index 4a6a73d..89f6129 100644 --- a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs +++ b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs @@ -34,6 +34,22 @@ public static MatchResult Match( AddToLookup(erasedKeyLookup, nameKey, entry); } + // Source methods that share a name-only key are the overloads a signature-less + // coverage key cannot tell apart; keep them together so they can be paired by + // source position when that happens. + var sourceGroups = new Dictionary>(StringComparer.Ordinal); + foreach (var complexity in complexityResults) + { + var key = MethodKeyHelper.GetNameOnlyKey(RoslynMethodParser.ToCanonicalKey(complexity.Identity)); + if (!sourceGroups.TryGetValue(key, out var group)) + { + group = []; + sourceGroups[key] = group; + } + + group.Add(complexity); + } + var matchedFullKeys = new HashSet(StringComparer.Ordinal); var matchedNameKeys = new HashSet(StringComparer.Ordinal); var matchedErasedKeys = new HashSet(StringComparer.Ordinal); @@ -62,13 +78,14 @@ public static MatchResult Match( // Pass 2: Fallback to name-only key (without signature) var nameKey = MethodKeyHelper.GetNameOnlyKey(fullKey); - if (nameKeyLookup.TryGetValue(nameKey, out var nameMatches) && nameMatches.Count == 1) + if (nameKeyLookup.TryGetValue(nameKey, out var nameMatches) + && TryResolveForMethod(nameMatches, complexity, sourceGroups[nameKey], out var nameCoverage)) { matchedNameKeys.Add(nameKey); methods.Add(new MatchedMethod { Complexity = complexity, - Coverage = nameMatches[0].Coverage + Coverage = nameCoverage }); continue; } @@ -167,6 +184,89 @@ public static MatchResult Match( }; } + /// + /// Pick the coverage entry belonging to one specific source method from candidates + /// that share a name-only key. + /// + /// + /// Candidates collide for two unrelated reasons. The same method is reported once + /// per coverage file, because every file describes the whole assembly it loaded; + /// those entries merge to the highest coverage observed. Separately, overloads + /// collide whenever a signature-less key is in play, which is unavoidable for + /// async and iterator methods. Those are told apart by source position, since a + /// generated state machine keeps the positions of the method it was rewritten + /// from. Pairing is positional and only applies when every overload has exactly + /// one entry; anything less certain returns false so the caller declines rather + /// than attributing one overload's coverage to another. + /// + private static bool TryResolveForMethod( + List candidates, + MethodComplexityResult complexity, + List siblings, + out double coverage) + { + coverage = 0.0; + + if (candidates.Count == 0) + return false; + + // Collapse the same entry arriving from several coverage files. + var distinct = candidates + .GroupBy(c => (c.ClassName, c.MethodName, c.Signature, c.StartLine)) + .Select(g => new + { + g.First().FileName, + g.First().StartLine, + Coverage = g.Max(c => c.Coverage) + }) + .ToList(); + + if (distinct.Count == 1 && siblings.Count == 1) + { + coverage = distinct[0].Coverage; + return true; + } + + // More than one real method behind the key: pair by source position, and only + // when the two sides line up exactly. + if (distinct.Count != siblings.Count) + return false; + + if (distinct.Exists(d => d.StartLine is null) + || siblings.Exists(sibling => sibling.Identity.LineNumber is null)) + return false; + + if (!distinct.TrueForAll(d => SameFile(complexity.Identity.FilePath, d.FileName))) + return false; + + var orderedEntries = distinct.OrderBy(d => d.StartLine).ToList(); + var orderedSiblings = siblings.OrderBy(sibling => sibling.Identity.LineNumber).ToList(); + + var index = orderedSiblings.FindIndex(sibling => sibling.Identity == complexity.Identity); + if (index < 0) + return false; + + coverage = orderedEntries[index].Coverage; + return true; + } + + /// + /// Whether a Roslyn file path and a Cobertura filename name the same file. The + /// report path is relative to the project, the Roslyn path is absolute. + /// + private static bool SameFile(string? sourcePath, string? coverageFile) + { + if (string.IsNullOrEmpty(sourcePath) || string.IsNullOrEmpty(coverageFile)) + return false; + + var source = sourcePath.Replace('\\', '/'); + var report = coverageFile.Replace('\\', '/'); + + return source.EndsWith(report, StringComparison.OrdinalIgnoreCase) + || string.Equals( + Path.GetFileName(source), Path.GetFileName(report), StringComparison.OrdinalIgnoreCase); + } + private static void AddToLookup( Dictionary> lookup, string key, diff --git a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs index 874399f..1153a0c 100644 --- a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs +++ b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs @@ -14,7 +14,9 @@ private static MethodComplexityResult MakeComplexity( string className = "Service", string ns = "MyApp", string signature = "()", - int complexity = 5) => + int complexity = 5, + string filePath = "Test.cs", + int lineNumber = 1) => new() { Identity = new MethodIdentity @@ -24,8 +26,8 @@ private static MethodComplexityResult MakeComplexity( MethodName = methodName, Signature = signature, FullName = $"{ns}.{className}.{methodName}{signature}", - FilePath = "Test.cs", - LineNumber = 1 + FilePath = filePath, + LineNumber = lineNumber }, Complexity = complexity }; @@ -34,12 +36,16 @@ private static CoberturaMethodCoverage MakeCoverage( string methodName, string className = "MyApp.Service", string signature = "()", - double coverage = 0.8) => + double coverage = 0.8, + string? fileName = null, + int? startLine = null) => new() { ClassName = className, MethodName = methodName, Signature = signature, + FileName = fileName, + StartLine = startLine, Coverage = coverage }; @@ -462,4 +468,76 @@ public void SameMethodInTwoCoverageFiles_TakesTheHigherCoverage() result.Methods.Single().Coverage.Should().Be(1.0); } + + [Fact] + public void TwoAsyncOverloads_AreNotMergedIntoASingleCoverageValue() + { + // Two async overloads compile to two state machines differing only by ordinal, + // and recovery has to drop the signature to match them at all, so both reduce + // to the same name. Nothing in the coverage file says which ordinal belongs to + // which overload, so attributing one overload's coverage to the other would + // report an untested overload as covered. Declining is the safe answer. + var complexity = new[] + { + MakeComplexity("Create", signature: "(Expense)"), + MakeComplexity("Create", signature: "(ExpenseDetails)") + }; + var coverage = new[] + { + MakeCoverage("MoveNext", className: "MyApp.Service/d__5", coverage: 0.0), + MakeCoverage("MoveNext", className: "MyApp.Service/d__6", coverage: 1.0) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Should().OnlyContain(m => m.Coverage == 0.0); + } + + [Fact] + public void TwoAsyncOverloads_AreDistinguishedBySourcePosition() + { + // The generated state machine keeps the original source positions, so the + // overload each one belongs to can be recovered from where its lines sit even + // though the signature is gone. + var complexity = new[] + { + MakeComplexity("Create", signature: "(Expense)", + filePath: "ExpenseCreator.cs", lineNumber: 26), + MakeComplexity("Create", signature: "(ExpenseDetails)", + filePath: "ExpenseCreator.cs", lineNumber: 63) + }; + var coverage = new[] + { + MakeCoverage("MoveNext", className: "MyApp.Service/d__5", + coverage: 0.83, fileName: "ExpenseCreator.cs", startLine: 27), + MakeCoverage("MoveNext", className: "MyApp.Service/d__6", + coverage: 1.0, fileName: "ExpenseCreator.cs", startLine: 64) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Single(m => m.Complexity.Identity.Signature == "(Expense)") + .Coverage.Should().Be(0.83); + result.Methods.Single(m => m.Complexity.Identity.Signature == "(ExpenseDetails)") + .Coverage.Should().Be(1.0); + } + + [Fact] + public void AsyncMethodInTwoCoverageFiles_IsStillMatchedByTheNameFallback() + { + // A recovered state machine key carries no signature, so it can only resolve + // through the name-only pass. With one coverage file per test project the same + // method arrives twice, and the pass must not mistake that for two overloads + // and drop the method to zero. + var complexity = new[] { MakeComplexity("Run", signature: "(int)") }; + var coverage = new[] + { + MakeCoverage("MoveNext", className: "MyApp.Service/d__1", coverage: 0.0), + MakeCoverage("MoveNext", className: "MyApp.Service/d__1", coverage: 1.0) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Single().Coverage.Should().Be(1.0); + } } From f56f3b0430d5db8bebf5b52c4bce99c3ae71c52f Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Thu, 10 Sep 2026 15:00:22 -0400 Subject: [PATCH 6/8] fix: apply the same duplicate and overload handling to the erased pass 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 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) --- .../Matching/MethodCoverageMatcher.cs | 31 ++++++++++++++----- .../Matching/MethodCoverageMatcherTests.cs | 17 ++++++++++ 2 files changed, 40 insertions(+), 8 deletions(-) diff --git a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs index 89f6129..7df1fb2 100644 --- a/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs +++ b/src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs @@ -38,16 +38,15 @@ public static MatchResult Match( // coverage key cannot tell apart; keep them together so they can be paired by // source position when that happens. var sourceGroups = new Dictionary>(StringComparer.Ordinal); + var erasedSourceGroups = new Dictionary>(StringComparer.Ordinal); foreach (var complexity in complexityResults) { var key = MethodKeyHelper.GetNameOnlyKey(RoslynMethodParser.ToCanonicalKey(complexity.Identity)); - if (!sourceGroups.TryGetValue(key, out var group)) - { - group = []; - sourceGroups[key] = group; - } + AddToGroup(sourceGroups, key, complexity); - group.Add(complexity); + // The erased pass collapses Apply and Apply onto one key, so it needs + // its own grouping rather than reusing the name-only one. + AddToGroup(erasedSourceGroups, MethodKeyHelper.EraseMethodGenericArity(key), complexity); } var matchedFullKeys = new HashSet(StringComparer.Ordinal); @@ -93,13 +92,15 @@ public static MatchResult Match( // Pass 3: Generic methods carry no arity on the Cobertura side, so fall // back to a key with the method's arity erased from both sides. var erasedKey = MethodKeyHelper.EraseMethodGenericArity(nameKey); - if (erasedKeyLookup.TryGetValue(erasedKey, out var erasedMatches) && erasedMatches.Count == 1) + if (erasedKeyLookup.TryGetValue(erasedKey, out var erasedMatches) + && TryResolveForMethod( + erasedMatches, complexity, erasedSourceGroups[erasedKey], out var erasedCoverage)) { matchedErasedKeys.Add(erasedKey); methods.Add(new MatchedMethod { Complexity = complexity, - Coverage = erasedMatches[0].Coverage + Coverage = erasedCoverage }); continue; } @@ -267,6 +268,20 @@ private static bool SameFile(string? sourcePath, string? coverageFile) Path.GetFileName(source), Path.GetFileName(report), StringComparison.OrdinalIgnoreCase); } + private static void AddToGroup( + Dictionary> groups, + string key, + MethodComplexityResult complexity) + { + if (!groups.TryGetValue(key, out var group)) + { + group = []; + groups[key] = group; + } + + group.Add(complexity); + } + private static void AddToLookup( Dictionary> lookup, string key, diff --git a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs index 1153a0c..1385cfc 100644 --- a/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs +++ b/tests/Crap4DotNet.Core.Tests/Matching/MethodCoverageMatcherTests.cs @@ -540,4 +540,21 @@ public void AsyncMethodInTwoCoverageFiles_IsStillMatchedByTheNameFallback() result.Methods.Single().Coverage.Should().Be(1.0); } + + [Fact] + public void GenericMethodInTwoCoverageFiles_IsStillMatchedByTheErasedFallback() + { + // A generic method resolves only through the arity-erased pass, which needs + // the same tolerance for one method arriving from several coverage files. + var complexity = new[] { MakeComplexity("Apply", signature: "(int)") }; + var coverage = new[] + { + MakeCoverage("Apply", signature: "(System.Int32)", coverage: 0.0), + MakeCoverage("Apply", signature: "(System.Int32)", coverage: 1.0) + }; + + var result = MethodCoverageMatcher.Match(complexity, coverage); + + result.Methods.Single().Coverage.Should().Be(1.0); + } } From 97259b03979ae3df6ce1632400ecd8a77c2121b2 Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Fri, 11 Sep 2026 14:11:50 -0400 Subject: [PATCH 7/8] docs: say plainly that Coverlet measures the coverage "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) --- README.md | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index e0e358f..1c30926 100644 --- a/README.md +++ b/README.md @@ -38,8 +38,12 @@ Requires .NET 8.0 or later. ## Quick Start +Coverage is measured by [Coverlet](https://github.com/coverlet-coverage/coverlet), so every +test project needs a `coverlet.collector` package reference. crap4dotnet reads the Cobertura +XML Coverlet produces and pairs it with the complexity it calculates from your source. + ```bash -# Option A: Let crap4dotnet run tests and generate coverage automatically +# Option A: let crap4dotnet run `dotnet test` for you and pick up the coverage dotnet-crap analyze ./MyApp.sln --run-tests # Option B: Generate coverage yourself, then analyze @@ -65,7 +69,7 @@ dotnet-crap analyze [options] |-----------------|-------------| | `` | Path to `.cs` file, directory, `.csproj`, or `.sln` | | `--coverage ` | Path(s) to Cobertura XML coverage file(s) | -| `--run-tests` | Run `dotnet test` to generate coverage automatically | +| `--run-tests` | Run `dotnet test --collect:"XPlat Code Coverage"` and use the coverage it produces | | `--threshold ` | CRAP threshold (default: 30) | | `--output ` | Write JSON to file instead of stdout | | `--min-crap ` | Only include methods with CRAP >= this value | From b25f014ca74bdd105d2d216fe9dd3c4522c28056 Mon Sep 17 00:00:00 2001 From: Dillon Harless Date: Mon, 14 Sep 2026 10:34:10 -0400 Subject: [PATCH 8/8] ci: install the locally packed tool instead of the published one 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. --- .github/workflows/ci.yml | 2 +- nuget.ci.config | 27 +++++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 1 deletion(-) create mode 100644 nuget.ci.config diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f101af1..9c7e222 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -32,7 +32,7 @@ jobs: run: dotnet pack src/Crap4DotNet.Cli -o ./nupkg - name: Install dotnet-crap - run: dotnet tool install -g Crap4DotNet --add-source ./nupkg + run: dotnet tool install -g Crap4DotNet --configfile nuget.ci.config --prerelease - name: Self-analyze with CRAP metrics continue-on-error: true diff --git a/nuget.ci.config b/nuget.ci.config new file mode 100644 index 0000000..44362f5 --- /dev/null +++ b/nuget.ci.config @@ -0,0 +1,27 @@ + + + + + + + + + + + + +