Match coverage to the methods it belongs to

.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

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 <path> [options]
 |-----------------|-------------|
 | `<path>` | Path to `.cs` file, directory, `.csproj`, or `.sln` |
 | `--coverage <path>` | 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 <n>` | CRAP threshold (default: 30) |
 | `--output <path>` | Write JSON to file instead of stdout |
 | `--min-crap <n>` | Only include methods with CRAP >= this value |

nuget.ci.config

@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  CI-only NuGet configuration, passed to `dotnet tool install` with the configfile
+  option so it applies to that one command.
+
+  It declares the local pack output as the only source, and grants it the only
+  mapping. That exclusivity is the point: with no remote feed available the step
+  cannot quietly resolve the published tool from nuget.org, which is what it did
+  before, smoke testing a release instead of the build under test.
+
+  This deliberately does not live in nuget.config. Source mapping restricts rather
+  than adds, and the more specific pattern wins, so mapping Crap4DotNet to a local
+  folder there would stop it resolving from nuget.org for everyone. The folder only
+  exists after a pack, so anyone installing the published tool from a fresh clone
+  would get a flat "not found".
+-->
+<configuration>
+  <packageSources>
+    <clear />
+    <add key="local" value="./nupkg" />
+  </packageSources>
+  <packageSourceMapping>
+    <packageSource key="local">
+      <package pattern="Crap4DotNet" />
+    </packageSource>
+  </packageSourceMapping>
+</configuration>

src/Crap4DotNet.Core/Coverage/CoberturaCoverageReader.cs

@@ -48,6 +48,7 @@ private static List<CoberturaMethodCoverage> 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);
     }
 
+    /// <summary>
+    /// Lowest source line number recorded under an element, if any.
+    /// </summary>
+    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

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; }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    public int? StartLine { get; init; }
+
     public required double Coverage { get; init; }
 }

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.
 /// </summary>
-public static class CoberturaMethodParser
+public static partial class CoberturaMethodParser
 {
     private static readonly Dictionary<string, string> ClrToCSharpTypes = new(StringComparer.Ordinal)
     {
@@ -64,12 +65,54 @@ public static class CoberturaMethodParser
     /// </summary>
     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}";
     }
 
+    /// <summary>
+    /// Async and iterator bodies are compiled into a generated state machine type named
+    /// "Outer/&lt;MethodName&gt;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.
+    /// </summary>
+    /// <remarks>
+    /// The generated type carries no parameter information, so the key is emitted
+    /// without a signature and resolves through the name-only fallback pass.
+    /// </remarks>
+    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);
+        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(@"^(?<owner>.+)/<(?<method>[^>]+)>d__\d+(?<arity>`\d+)?$")]
+    private static partial Regex StateMachineClassRegex();
+
     private static string NormalizeClassName(string className)
     {
         // Nested type separator: / → .

src/Crap4DotNet.Core/Matching/MethodCoverageMatcher.cs

@@ -17,6 +17,7 @@ public static MatchResult Match(
         // Build coverage lookups by normalized key
         var fullKeyLookup = new Dictionary<string, List<CoberturaMethodCoverage>>(StringComparer.Ordinal);
         var nameKeyLookup = new Dictionary<string, List<CoberturaMethodCoverage>>(StringComparer.Ordinal);
+        var erasedKeyLookup = new Dictionary<string, List<CoberturaMethodCoverage>>(StringComparer.Ordinal);
 
         foreach (var entry in coverageEntries)
         {
@@ -25,10 +26,32 @@ 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);
+        }
+
+        // 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<string, List<MethodComplexityResult>>(StringComparer.Ordinal);
+        var erasedSourceGroups = new Dictionary<string, List<MethodComplexityResult>>(StringComparer.Ordinal);
+        foreach (var complexity in complexityResults)
+        {
+            var key = MethodKeyHelper.GetNameOnlyKey(RoslynMethodParser.ToCanonicalKey(complexity.Identity));
+            AddToGroup(sourceGroups, key, complexity);
+
+            // The erased pass collapses Apply<T> 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<string>(StringComparer.Ordinal);
         var matchedNameKeys = new HashSet<string>(StringComparer.Ordinal);
+        var matchedErasedKeys = new HashSet<string>(StringComparer.Ordinal);
         var methods = new List<MatchedMethod>();
         var unmatchedNames = new List<string>();
         var warnings = new List<DiagnosticWarning>();
@@ -43,21 +66,41 @@ 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;
             }
 
             // 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;
+            }
+
+            // 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)
+                && TryResolveForMethod(
+                    erasedMatches, complexity, erasedSourceGroups[erasedKey], out var erasedCoverage))
+            {
+                matchedErasedKeys.Add(erasedKey);
+                methods.Add(new MatchedMethod
+                {
+                    Complexity = complexity,
+                    Coverage = erasedCoverage
                 });
                 continue;
             }
@@ -85,6 +128,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}"));
@@ -138,6 +185,103 @@ public static MatchResult Match(
         };
     }
 
+    /// <summary>
+    /// Pick the coverage entry belonging to one specific source method from candidates
+    /// that share a name-only key.
+    /// </summary>
+    /// <remarks>
+    /// 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.
+    /// </remarks>
+    private static bool TryResolveForMethod(
+        List<CoberturaMethodCoverage> candidates,
+        MethodComplexityResult complexity,
+        List<MethodComplexityResult> 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;
+    }
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    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 AddToGroup(
+        Dictionary<string, List<MethodComplexityResult>> groups,
+        string key,
+        MethodComplexityResult complexity)
+    {
+        if (!groups.TryGetValue(key, out var group))
+        {
+            group = [];
+            groups[key] = group;
+        }
+
+        group.Add(complexity);
+    }
+
     private static void AddToLookup(
         Dictionary<string, List<CoberturaMethodCoverage>> lookup,
         string key,