Issue with generated code for excluded methods (coverlet-coverage#1542)

* changed algorithem to exclude methods marked with ExcludeFromCodeCoverage attribute * fix failing tests * update change log * fixed tests * format * update changelog * update changelog --------- Co-authored-by: David Mueller x <[email protected]>
MarcoRossignoli · Nov 19, 2023 · c69d37e · c69d37e
1 parent c309dab
commit c69d37e
Show file tree

Hide file tree

Showing 6 changed files with 161 additions and 191 deletions.
diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md
@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 ## Unreleased
 
 ### Fixed
+-Fix ExcludeFromCodeCoverage does not exclude method in a partial class [#1548](https://github.com/coverlet-coverage/coverlet/issues/1548) 
+-Fix ExcludeFromCodeCoverage does not exclude F# task [#1547](https://github.com/coverlet-coverage/coverlet/issues/1547) 
+-Fix issues where ExcludeFromCodeCoverage ignored [#1431](https://github.com/coverlet-coverage/coverlet/issues/1431)
+-Fix issues with ExcludeFromCodeCoverage attribute [#1484](https://github.com/coverlet-coverage/coverlet/issues/1484)
 -Fix broken links in documentation [#1514](https://github.com/coverlet-coverage/coverlet/issues/1514)
 -Fix problem with coverage for .net5 WPF application [#1221](https://github.com/coverlet-coverage/coverlet/issues/1221) by https://github.com/lg2de
 -Fix unable to instrument module for Microsoft.AspNetCore.Mvc.Razor [#1459](https://github.com/coverlet-coverage/coverlet/issues/1459) by https://github.com/lg2de

diff --git a/coverlet.sln b/coverlet.sln
@@ -74,6 +74,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.tests.projectsampl
 EndProject
 Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "coverlet.msbuild.tasks.tests", "test\coverlet.msbuild.tasks.tests\coverlet.msbuild.tasks.tests.csproj", "{351A034E-E642-4DB9-A21D-F71C8151C243}"
 EndProject
+Project("{778DAE3C-4631-46EA-AA77-85C1314464D9}") = "coverlet.tests.projectsample.vbmynamespace", "test\coverlet.tests.projectsample.vbmynamespace\coverlet.tests.projectsample.vbmynamespace.vbproj", "{03400776-1F9A-4326-B927-1CA9B64B42A1}"
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -168,6 +170,14 @@ Global
 		{351A034E-E642-4DB9-A21D-F71C8151C243}.Debug|Any CPU.Build.0 = Debug|Any CPU
 		{351A034E-E642-4DB9-A21D-F71C8151C243}.Release|Any CPU.ActiveCfg = Release|Any CPU
 		{351A034E-E642-4DB9-A21D-F71C8151C243}.Release|Any CPU.Build.0 = Release|Any CPU
+		{3ABC2066-D1C5-4CAA-8867-9C5DC777CBF8}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{3ABC2066-D1C5-4CAA-8867-9C5DC777CBF8}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{3ABC2066-D1C5-4CAA-8867-9C5DC777CBF8}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{3ABC2066-D1C5-4CAA-8867-9C5DC777CBF8}.Release|Any CPU.Build.0 = Release|Any CPU
+		{03400776-1F9A-4326-B927-1CA9B64B42A1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
+		{03400776-1F9A-4326-B927-1CA9B64B42A1}.Debug|Any CPU.Build.0 = Debug|Any CPU
+		{03400776-1F9A-4326-B927-1CA9B64B42A1}.Release|Any CPU.ActiveCfg = Release|Any CPU
+		{03400776-1F9A-4326-B927-1CA9B64B42A1}.Release|Any CPU.Build.0 = Release|Any CPU
 	EndGlobalSection
 	GlobalSection(SolutionProperties) = preSolution
 		HideSolutionNode = FALSE
@@ -195,6 +205,7 @@ Global
 		{988A5FF0-4326-4F5B-9F05-CB165543A555} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 		{6ACF69B1-C01F-44A4-8F8E-2501884238D4} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 		{F508CCDD-5BC8-4AB6-97B3-D37498813C41} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
+		{03400776-1F9A-4326-B927-1CA9B64B42A1} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 		{351A034E-E642-4DB9-A21D-F71C8151C243} = {2FEBDE1B-83E3-445B-B9F8-5644B0E0E134}
 	EndGlobalSection
 	GlobalSection(ExtensibilityGlobals) = postSolution

diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs
@@ -47,9 +47,8 @@ internal class Instrumenter
     private MethodReference _customTrackerRecordHitMethod;
     private List<string> _excludedSourceFiles;
     private List<string> _branchesInCompiledGeneratedClass;
-    private List<(MethodDefinition, int)> _excludedMethods;
+    private List<(SequencePoint firstSequencePoint, SequencePoint lastSequencePoint)> _excludedMethodSections;
     private List<string> _excludedLambdaMethods;
-    private List<string> _excludedCompilerGeneratedTypes;
     private ReachabilityHelper _reachabilityHelper;
 
     public bool SkipModule { get; set; }
@@ -242,14 +241,7 @@ private void InstrumentModule()
             _instrumentationHelper.IsTypeIncluded(_module, type.FullName, _parameters.IncludeFilters)
             )
         {
-          if (IsSynthesizedMemberToBeExcluded(type))
-          {
-            (_excludedCompilerGeneratedTypes ??= new List<string>()).Add(type.FullName);
-          }
-          else
-          {
-            InstrumentType(type);
-          }
+          InstrumentType(type);
         }
       }
 
@@ -467,9 +459,6 @@ private void InstrumentType(TypeDefinition type)
     {
       IEnumerable<MethodDefinition> methods = type.GetMethods();
 
-      // We keep ordinal index because it's the way used by compiler for generated types/methods to 
-      // avoid ambiguity
-      int ordinal = -1;
       foreach (MethodDefinition method in methods)
       {
         MethodDefinition actualMethod = method;
@@ -490,18 +479,11 @@ private void InstrumentType(TypeDefinition type)
             customAttributes = customAttributes.Union(prop.CustomAttributes);
         }
 
-        ordinal++;
-
         if (IsMethodOfCompilerGeneratedClassOfAsyncStateMachineToBeExcluded(method))
         {
           continue;
         }
 
-        if (IsSynthesizedMemberToBeExcluded(method))
-        {
-          continue;
-        }
-
         if (_excludedLambdaMethods != null && _excludedLambdaMethods.Contains(method.FullName))
         {
           continue;
@@ -514,7 +496,9 @@ private void InstrumentType(TypeDefinition type)
         else
         {
           (_excludedLambdaMethods ??= new List<string>()).AddRange(CollectLambdaMethodsInsideLocalFunction(method));
-          (_excludedMethods ??= new List<(MethodDefinition, int)>()).Add((method, ordinal));
+          _excludedMethodSections ??= new List<(SequencePoint firstSequencePoint, SequencePoint lastSequencePoint)>();
+          AnalyzeCompileGeneratedTypesForExcludedMethod(method);
+          CacheExcludedMethodSection(method);
         }
       }
 
@@ -604,7 +588,8 @@ private void InstrumentIL(MethodDefinition method)
 
         if (sequencePoint != null && !sequencePoint.IsHidden)
         {
-          if (_cecilSymbolHelper.SkipInlineAssignedAutoProperty(_parameters.SkipAutoProps, method, currentInstruction))
+          if (_cecilSymbolHelper.SkipInlineAssignedAutoProperty(_parameters.SkipAutoProps, method,
+                currentInstruction) || IsInsideExcludedMethodSection(sequencePoint))
           {
             index++;
             continue;
@@ -797,59 +782,38 @@ private static MethodBody GetMethodBody(MethodDefinition method)
       }
     }
 
-    // Check if the member (type or method) is generated by the compiler from a method excluded from code coverage
-    private bool IsSynthesizedMemberToBeExcluded(IMemberDefinition definition)
+    private bool IsInsideExcludedMethodSection(SequencePoint sequencePoint)
     {
-      if (_excludedMethods is null)
-      {
-        return false;
-      }
-
-      TypeDefinition declaringType = definition.DeclaringType;
+      if (_excludedMethodSections is null) return false;
 
-      // We check all parent type of current one bottom-up
-      while (declaringType != null)
+      bool IsInsideExcludedSection(SequencePoint firstSequencePoint, SequencePoint lastSequencePoint)
       {
+        bool isInsideSameSourceFile = sequencePoint.Document.Url.Equals(firstSequencePoint.Document.Url);
+        bool isInsideExcludedMethod = sequencePoint.StartLine >= Math.Min(firstSequencePoint.StartLine, firstSequencePoint.EndLine) &&
+                                      sequencePoint.StartLine <= Math.Max(lastSequencePoint.StartLine, lastSequencePoint.EndLine);
+        return isInsideExcludedMethod && isInsideSameSourceFile;
+      }
 
-        // If parent type is excluded return
-        if (_excludedCompilerGeneratedTypes != null &&
-            _excludedCompilerGeneratedTypes.Any(t => t == declaringType.FullName))
-        {
-          return true;
-        }
+      return _excludedMethodSections
+        .Where(x => x is { firstSequencePoint: not null, lastSequencePoint: not null })
+        .Any(x => IsInsideExcludedSection(x.firstSequencePoint, x.lastSequencePoint));
+    }
 
-        // Check methods members and compiler generated types
-        foreach ((MethodDefinition, int) excludedMethods in _excludedMethods)
-        {
-          // Exclude this member if declaring type is the same of the excluded method and 
-          // the name is synthesized from the name of the excluded method.
-          // 
-          if (declaringType.FullName == excludedMethods.Item1.DeclaringType.FullName &&
-              IsSynthesizedNameOf(definition.Name, excludedMethods.Item1.Name, excludedMethods.Item2))
-          {
-            return true;
-          }
-        }
-        declaringType = declaringType.DeclaringType;
-      }
+    private void AnalyzeCompileGeneratedTypesForExcludedMethod(MethodDefinition method)
+    {
+      IEnumerable<TypeDefinition> referencedTypes = method.CustomAttributes.Where(x => x.HasConstructorArguments)
+        .SelectMany(x => x.ConstructorArguments.Select(y => y.Value as TypeDefinition));
 
-      return false;
+      referencedTypes.ToList().ForEach(x =>
+        x?.Methods.Where(y => y.FullName.Contains("MoveNext")).ToList().ForEach(CacheExcludedMethodSection)
+      );
     }
 
-    // Check if the name is synthesized by the compiler
-    // Refer to https://github.com/dotnet/roslyn/blob/master/src/Compilers/CSharp/Portable/Symbols/Synthesized/GeneratedNames.cs
-    // to see how the compiler generate names for lambda, local function, yield or async/await expressions
-    internal bool IsSynthesizedNameOf(string name, string methodName, int methodOrdinal)
+    private void CacheExcludedMethodSection(MethodDefinition method)
     {
-      return
-          // Lambda method
-          name.IndexOf($"<{methodName}>b__{methodOrdinal}") != -1 ||
-          // Lambda display class
-          name.IndexOf($"<>c__DisplayClass{methodOrdinal}_") != -1 ||
-          // State machine
-          name.IndexOf($"<{methodName}>d__{methodOrdinal}") != -1 ||
-          // Local function
-          (name.IndexOf($"<{methodName}>g__") != -1 && name.IndexOf($"|{methodOrdinal}_") != -1);
+      _excludedMethodSections.Add((
+        method.DebugInformation.SequencePoints.FirstOrDefault(x => !x.IsHidden),
+        method.DebugInformation.SequencePoints.LastOrDefault(x => !x.IsHidden)));
     }
 
     private static IEnumerable<string> CollectLambdaMethodsInsideLocalFunction(MethodDefinition methodDefinition)
@@ -858,7 +822,8 @@ private static IEnumerable<string> CollectLambdaMethodsInsideLocalFunction(Metho
 
       foreach (Instruction instruction in methodDefinition.Body.Instructions.ToList())
       {
-        if (instruction.OpCode == OpCodes.Ldftn && instruction.Operand is MethodReference mr && mr.Name.Contains(">b__"))
+        if (instruction.OpCode == OpCodes.Ldftn && instruction.Operand is MethodReference mr &&
+            mr.Name.Contains(">b__"))
         {
           yield return mr.FullName;
         }