Merge pull request #277 from MarcoRossignoli/fixwithmerge

Fix false negative with "/p:MergeWith" async/await branches

Author: tonerdo

src/coverlet.core/Coverage.cs

@@ -193,7 +193,7 @@ public CoverageResult GetCoverageResult()
                 InstrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier);
             }
 
-            var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules };
+            var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results };
 
             if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && File.Exists(_mergeWith))
             {
@@ -257,14 +257,14 @@ private void CalculateCoverage()
                 }
 
                 // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) 
-                // we'll remove all MoveNext() not covered branch              
+                // we'll remove all MoveNext() not covered branch
                 foreach (var document in result.Documents)
                 {
                     List<KeyValuePair<(int, int), Branch>> branchesToRemove = new List<KeyValuePair<(int, int), Branch>>();
                     foreach (var branch in document.Value.Branches)
                     {
                         //if one branch is covered we search the other one only if it's not covered
-                        if (CecilSymbolHelper.IsMoveNext(branch.Value.Method) && branch.Value.Hits > 0)
+                        if (IsAsyncStateMachineMethod(branch.Value.Method) && branch.Value.Hits > 0)
                         {
                             foreach (var moveNextBranch in document.Value.Branches)
                             {
@@ -286,6 +286,23 @@ private void CalculateCoverage()
             }
         }
 
+        private bool IsAsyncStateMachineMethod(string method)
+        {
+            if (!method.EndsWith("::MoveNext()"))
+            {
+                return false;
+            }
+
+            foreach (var instrumentationResult in _results)
+            {
+                if (instrumentationResult.AsyncMachineStateMethod.Contains(method))
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         private string GetSourceLinkUrl(Dictionary<string, string> sourceLinkDocuments, string document)
         {
             if (sourceLinkDocuments.TryGetValue(document, out string url))

src/coverlet.core/CoverageResult.cs

@@ -3,6 +3,8 @@
 using System.IO;
 using System.Linq;
 using Coverlet.Core.Enums;
+using Coverlet.Core.Instrumentation;
+using Coverlet.Core.Symbols;
 
 namespace Coverlet.Core
 {
@@ -39,6 +41,7 @@ public class CoverageResult
     {
         public string Identifier;
         public Modules Modules;
+        internal List<InstrumenterResult> InstrumentedResults;
 
         internal CoverageResult() { }
 
@@ -105,6 +108,57 @@ internal void Merge(Modules modules)
                     }
                 }
             }
+
+            // for MoveNext() compiler autogenerated method we need to patch false positive (IAsyncStateMachine for instance) 
+            // we'll remove all MoveNext() not covered branch
+            List<BranchInfo> branchesToRemove = new List<BranchInfo>();
+            foreach (var module in this.Modules)
+            {
+                foreach (var document in module.Value)
+                {
+                    foreach (var @class in document.Value)
+                    {
+                        foreach (var method in @class.Value)
+                        {
+                            foreach (var branch in method.Value.Branches)
+                            {
+                                //if one branch is covered we search the other one only if it's not covered
+                                if (IsAsyncStateMachineMethod(method.Key) && branch.Hits > 0)
+                                {
+                                    foreach (var moveNextBranch in method.Value.Branches)
+                                    {
+                                        if (moveNextBranch != branch && moveNextBranch.Hits == 0)
+                                        {
+                                            branchesToRemove.Add(moveNextBranch);
+                                        }
+                                    }
+                                }
+                            }
+                            foreach (var branchToRemove in branchesToRemove)
+                            {
+                                method.Value.Branches.Remove(branchToRemove);
+                            }
+                        }
+                    }
+                }
+            }
+        }
+
+        private bool IsAsyncStateMachineMethod(string method)
+        {
+            if (!method.EndsWith("::MoveNext()"))
+            {
+                return false;
+            }
+
+            foreach (var instrumentedResult in InstrumentedResults)
+            {
+                if (instrumentedResult.AsyncMachineStateMethod.Contains(method))
+                {
+                    return true;
+                }
+            }
+            return false;
         }
 
         public ThresholdTypeFlags GetThresholdTypesBelowThreshold(CoverageSummary summary, double threshold, ThresholdTypeFlags thresholdTypes, ThresholdStatistic thresholdStat)

src/coverlet.core/Instrumentation/Instrumenter.cs

@@ -31,6 +31,7 @@ internal class Instrumenter
         private TypeDefinition _customTrackerTypeDef;
         private MethodReference _customTrackerRegisterUnloadEventsMethod;
         private MethodReference _customTrackerRecordHitMethod;
+        private List<string> _asyncMachineStateMethod;
 
         public Instrumenter(string module, string identifier, string[] excludeFilters, string[] includeFilters, string[] excludedFiles, string[] excludedAttributes)
         {
@@ -61,6 +62,8 @@ public InstrumenterResult Instrument()
 
             InstrumentModule();
 
+            _result.AsyncMachineStateMethod = _asyncMachineStateMethod == null ? Array.Empty<string>() : _asyncMachineStateMethod.ToArray();
+
             return _result;
         }
 
@@ -372,6 +375,7 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
 
             var key = (branchPoint.StartLine, (int)branchPoint.Ordinal);
             if (!document.Branches.ContainsKey(key))
+            { 
                 document.Branches.Add(key,
                     new Branch
                     {
@@ -385,12 +389,43 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
                     }
                 );
 
+                if (IsAsyncStateMachineBranch(method.DeclaringType, method))
+                {
+                    if (_asyncMachineStateMethod == null)
+                    {
+                        _asyncMachineStateMethod = new List<string>();
+                    }
+
+                    if (!_asyncMachineStateMethod.Contains(method.FullName))
+                    {
+                        _asyncMachineStateMethod.Add(method.FullName);
+                    }
+                }
+            }
+
             var entry = (true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal);
             _result.HitCandidates.Add(entry);
 
             return AddInstrumentationInstructions(method, processor, instruction, _result.HitCandidates.Count - 1);
         }
 
+        private bool IsAsyncStateMachineBranch(TypeDefinition typeDef, MethodDefinition method)
+        {
+            if (!method.FullName.EndsWith("::MoveNext()"))
+            {
+                return false;
+            }
+
+            foreach (InterfaceImplementation implementedInterface in typeDef.Interfaces)
+            {
+                if (implementedInterface.InterfaceType.FullName == "System.Runtime.CompilerServices.IAsyncStateMachine")
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+
         private Instruction AddInstrumentationInstructions(MethodDefinition method, ILProcessor processor, Instruction instruction, int hitEntryIndex)
         {
             if (_customTrackerRecordHitMethod == null)

src/coverlet.core/Instrumentation/InstrumenterResult.cs

@@ -41,6 +41,7 @@ public InstrumenterResult()
         }
 
         public string Module;
+        public string[] AsyncMachineStateMethod;
         public string HitsFilePath;
         public string HitsResultGuid;
         public string ModulePath;

src/coverlet.core/Symbols/CecilSymbolHelper.cs

@@ -20,11 +20,6 @@ public static class CecilSymbolHelper
         private const int StepOverLineCode = 0xFEEFEE;
         private static readonly Regex IsMovenext = new Regex(@"\<[^\s>]+\>\w__\w(\w)?::MoveNext\(\)$", RegexOptions.Compiled | RegexOptions.ExplicitCapture);
 
-        public static bool IsMoveNext(string fullName)
-        {
-            return IsMovenext.IsMatch(fullName);
-        }
-
         public static List<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
         {
             var list = new List<BranchPoint>();