Improve lambda scenario coverage (#583)

Improve lambda scenario coverage

Author: MarcoRossignoli

src/coverlet.core/Coverage.cs

@@ -217,6 +217,63 @@ public CoverageResult GetCoverageResult()
                 _instrumentationHelper.RestoreOriginalModule(result.ModulePath, _identifier);
             }
 
+            // In case of anonymous delegate compiler generate a custom class and passes it as type.method delegate.
+            // If in delegate method we've a branches we need to move these to "actual" class/method that use it.
+            // We search "method" with same "Line" of closure class method and add missing branches to it,
+            // in this way we correctly report missing branch inside compiled generated anonymous delegate.
+            List<string> compileGeneratedClassToRemove = null;
+            foreach (var module in 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 (BranchInCompilerGeneratedClass(method.Key))
+                                {
+                                    Method actualMethod = GetMethodWithSameLineInSameDocument(document.Value, @class.Key, branch.Line);
+
+                                    if (actualMethod is null)
+                                    {
+                                        continue;
+                                    }
+
+                                    actualMethod.Branches.Add(branch);
+
+                                    if (compileGeneratedClassToRemove is null)
+                                    {
+                                        compileGeneratedClassToRemove = new List<string>();
+                                    }
+
+                                    if (!compileGeneratedClassToRemove.Contains(@class.Key))
+                                    {
+                                        compileGeneratedClassToRemove.Add(@class.Key);
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+
+            // After method/branches analysis of compiled generated class we can remove noise from reports
+            if (!(compileGeneratedClassToRemove is null))
+            {
+                foreach (var module in modules)
+                {
+                    foreach (var document in module.Value)
+                    {
+                        foreach (var classToRemove in compileGeneratedClassToRemove)
+                        {
+                            document.Value.Remove(classToRemove);
+                        }
+                    }
+                }
+            }
+
             var coverageResult = new CoverageResult { Identifier = _identifier, Modules = modules, InstrumentedResults = _results };
 
             if (!string.IsNullOrEmpty(_mergeWith) && !string.IsNullOrWhiteSpace(_mergeWith) && _fileSystem.Exists(_mergeWith))
@@ -228,6 +285,41 @@ public CoverageResult GetCoverageResult()
             return coverageResult;
         }
 
+        private bool BranchInCompilerGeneratedClass(string methodName)
+        {
+            foreach (var instrumentedResult in _results)
+            {
+                if (instrumentedResult.BranchesInCompiledGeneratedClass.Contains(methodName))
+                {
+                    return true;
+                }
+            }
+            return false;
+        }
+
+        private Method GetMethodWithSameLineInSameDocument(Classes documentClasses, string compilerGeneratedClassName, int branchLine)
+        {
+            foreach (var @class in documentClasses)
+            {
+                if (@class.Key == compilerGeneratedClassName)
+                {
+                    continue;
+                }
+
+                foreach (var method in @class.Value)
+                {
+                    foreach (var line in method.Value.Lines)
+                    {
+                        if (line.Key == branchLine)
+                        {
+                            return method.Value;
+                        }
+                    }
+                }
+            }
+            return null;
+        }
+
         private void CalculateCoverage()
         {
             foreach (var result in _results)
@@ -253,15 +345,15 @@ private void CalculateCoverage()
                     }
                 }
 
+                List<(int docIndex, int line)> zeroHitsLines = new List<(int docIndex, int line)>();
+                var documentsList = result.Documents.Values.ToList();
                 using (var fs = _fileSystem.NewFileStream(result.HitsFilePath, FileMode.Open))
                 using (var br = new BinaryReader(fs))
                 {
                     int hitCandidatesCount = br.ReadInt32();
 
                     // TODO: hitCandidatesCount should be verified against result.HitCandidates.Count
 
-                    var documentsList = result.Documents.Values.ToList();
-
                     for (int i = 0; i < hitCandidatesCount; ++i)
                     {
                         var hitLocation = result.HitCandidates[i];
@@ -279,10 +371,29 @@ private void CalculateCoverage()
                             {
                                 var line = document.Lines[j];
                                 line.Hits += hits;
+
+                                // We register 0 hit lines for later cleanup false positive of nested lambda closures
+                                if (hits == 0)
+                                {
+                                    zeroHitsLines.Add((hitLocation.docIndex, line.Number));
+                                }
                             }
                         }
                     }
                 }
+
+                // Cleanup nested state machine false positive hits
+                foreach (var (docIndex, line) in zeroHitsLines)
+                {
+                    foreach (var lineToCheck in documentsList[docIndex].Lines)
+                    {
+                        if (lineToCheck.Key == line)
+                        {
+                            lineToCheck.Value.Hits = 0;
+                        }
+                    }
+                }
+
                 _instrumentationHelper.DeleteHitsFile(result.HitsFilePath);
                 _logger.LogVerbose($"Hit file '{result.HitsFilePath}' deleted");
             }

src/coverlet.core/Helpers/InstrumentationHelper.cs

@@ -198,7 +198,7 @@ public void BackupOriginalModule(string module, string identifier)
             }
         }
 
-        public void RestoreOriginalModule(string module, string identifier)
+        public virtual void RestoreOriginalModule(string module, string identifier)
         {
             var backupPath = GetBackupPath(module, identifier);
             var backupSymbolPath = Path.ChangeExtension(backupPath, ".pdb");
@@ -226,7 +226,7 @@ public void RestoreOriginalModule(string module, string identifier)
             }, retryStrategy, 10);
         }
 
-        public void RestoreOriginalModules()
+        public virtual void RestoreOriginalModules()
         {
             // Restore the original module - retry up to 10 times, since the destination file could be locked
             // See: https://github.com/tonerdo/coverlet/issues/25

src/coverlet.core/Instrumentation/Instrumenter.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics.CodeAnalysis;
 using System.IO;
 using System.Linq;
+using System.Runtime.CompilerServices;
 using Coverlet.Core.Abstracts;
 using Coverlet.Core.Attributes;
 using Coverlet.Core.Logging;
@@ -37,6 +38,7 @@ internal class Instrumenter
         private MethodReference _customTrackerRegisterUnloadEventsMethod;
         private MethodReference _customTrackerRecordHitMethod;
         private List<string> _excludedSourceFiles;
+        private List<string> _branchesInCompiledGeneratedClass;
 
         public Instrumenter(
             string module,
@@ -130,6 +132,8 @@ public InstrumenterResult Instrument()
                 }
             }
 
+            _result.BranchesInCompiledGeneratedClass = _branchesInCompiledGeneratedClass == null ? Array.Empty<string>() : _branchesInCompiledGeneratedClass.ToArray();
+
             return _result;
         }
 
@@ -454,7 +458,8 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
             BranchKey key = new BranchKey(branchPoint.StartLine, (int)branchPoint.Ordinal);
             if (!document.Branches.ContainsKey(key))
             {
-                document.Branches.Add(key,
+                document.Branches.Add(
+                    key,
                     new Branch
                     {
                         Number = branchPoint.StartLine,
@@ -466,6 +471,19 @@ private Instruction AddInstrumentationCode(MethodDefinition method, ILProcessor
                         Ordinal = branchPoint.Ordinal
                     }
                 );
+
+                if (method.DeclaringType.CustomAttributes.Any(x => x.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName))
+                {
+                    if (_branchesInCompiledGeneratedClass == null)
+                    {
+                        _branchesInCompiledGeneratedClass = new List<string>();
+                    }
+
+                    if (!_branchesInCompiledGeneratedClass.Contains(method.FullName))
+                    {
+                        _branchesInCompiledGeneratedClass.Add(method.FullName);
+                    }
+                }
             }
 
             _result.HitCandidates.Add(new HitCandidate(true, document.Index, branchPoint.StartLine, (int)branchPoint.Ordinal));

src/coverlet.core/Instrumentation/InstrumenterResult.cs

@@ -5,6 +5,7 @@
 
 namespace Coverlet.Core.Instrumentation
 {
+    [DebuggerDisplay("Number = {Number} Hits = {Hits} Class = {Class} Method = {Method}")]
     [DataContract]
     public class Line
     {
@@ -73,6 +74,7 @@ public Document()
         public Dictionary<BranchKey, Branch> Branches { get; private set; }
     }
 
+    [DebuggerDisplay("isBranch = {isBranch} docIndex = {docIndex} start = {start} end = {end}")]
     [DataContract]
     public class HitCandidate
     {
@@ -100,6 +102,8 @@ public InstrumenterResult()
         [DataMember]
         public string Module;
         [DataMember]
+        public string[] BranchesInCompiledGeneratedClass;
+        [DataMember]
         public string HitsFilePath;
         [DataMember]
         public string ModulePath;

test/coverlet.core.tests/CoverageTests.cs

@@ -92,15 +92,15 @@ public void SelectionStatements_If()
                 // Similar to msbuild coverage result task
                 CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);
 
+                // Generate html report to check
+                // TestInstrumentationHelper.GenerateHtmlReport(result);
+
                 // Asserts on doc/lines/branches
                 result.Document("Instrumentation.SelectionStatements.cs")
                       // (line, hits)
                       .AssertLinesCovered((11, 1), (15, 0))
                       // (line,ordinal,hits)
                       .AssertBranchesCovered((9, 0, 1), (9, 1, 0));
-
-                // if need to generate html report for debugging purpose
-                // TestInstrumentationHelper.GenerateHtmlReport(result);
             }
             finally
             {
@@ -163,6 +163,7 @@ public void AsyncAwait()
                 }, path).Dispose();
 
                 CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);
+
                 result.Document("Instrumentation.AsyncAwait.cs")
                       .AssertLinesCovered(BuildConfiguration.Debug,
                                             // AsyncExecution(bool)
@@ -192,5 +193,51 @@ public void AsyncAwait()
                 File.Delete(path);
             }
         }
+
+        [Fact]
+        public void Lambda_Issue343()
+        {
+            string path = Path.GetTempFileName();
+            try
+            {
+                RemoteExecutor.Invoke(async pathSerialize =>
+                {
+                    CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run<Lambda_Issue343>(instance =>
+                    {
+                        instance.InvokeAnonymous_Test();
+                        ((Task<bool>)instance.InvokeAnonymousAsync_Test()).ConfigureAwait(false).GetAwaiter().GetResult();
+                        return Task.CompletedTask;
+                    }, pathSerialize);
+                    return 0;
+                }, path).Dispose();
+
+                CoverageResult result = TestInstrumentationHelper.GetCoverageResult(path);
+
+                result.Document("Instrumentation.Lambda.cs")
+                      .AssertLinesCoveredAllBut(BuildConfiguration.Debug, 23, 51)
+                      .AssertBranchesCovered(BuildConfiguration.Debug,
+                        // Expected branches
+                        (22, 0, 0),
+                        (22, 1, 1),
+                        (50, 2, 0),
+                        (50, 3, 1),
+                        // Unexpected branches
+                        (20, 0, 1),
+                        (20, 1, 1),
+                        (49, 0, 1),
+                        (49, 1, 0),
+                        (54, 4, 0),
+                        (54, 5, 1),
+                        (39, 0, 1),
+                        (39, 1, 0),
+                        (48, 0, 1),
+                        (48, 1, 1)
+                      );
+            }
+            finally
+            {
+                File.Delete(path);
+            }
+        }
     }
 }