From 81b0dc864b2ad749606d4b1c4167c451bc11753a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher-Marcel=20B=C3=B6ddecker?= Date: Tue, 23 Jun 2020 14:13:35 +0200 Subject: [PATCH 1/4] Fix attribute exclusion for attributes that don't end in "Attribute". --- src/coverlet.core/Instrumentation/Instrumenter.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 43b040f8d..e0b9d971e 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -692,9 +692,10 @@ private bool IsExcludeAttribute(CustomAttribute customAttribute) { excludeAttributeNames = _excludedAttributes.Union(excludeAttributeNames); } - return excludeAttributeNames.Any(a => - customAttribute.AttributeType.Name.Equals(a.EndsWith("Attribute") ? a : $"{a}Attribute")); + a.EndsWith("Attribute") + ? customAttribute.AttributeType.Name.Equals(a) + : customAttribute.AttributeType.Name.Equals($"{a}Attribute")); } private static MethodBody GetMethodBody(MethodDefinition method) From b4003c93f4a2dce5d1c8f472cbe745f590ba7d60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher-Marcel=20B=C3=B6ddecker?= Date: Thu, 25 Jun 2020 06:09:21 +0200 Subject: [PATCH 2/4] Add tests for attribute exclusion of those without "Attribute" suffix. --- .../Instrumentation/InstrumenterTests.cs | 38 ++++++++++++---- test/coverlet.core.tests/Samples/Samples.cs | 13 ++++++ .../Symbols/CecilSymbolHelperTests.cs | 44 +++++++++---------- 3 files changed, 64 insertions(+), 31 deletions(-) diff --git a/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs b/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs index e362b1563..78e5365b5 100644 --- a/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs +++ b/test/coverlet.core.tests/Instrumentation/InstrumenterTests.cs @@ -17,6 +17,7 @@ using Moq; using Xunit; using Microsoft.Extensions.DependencyModel; +using Microsoft.VisualStudio.TestPlatform; namespace Coverlet.Core.Instrumentation.Tests { @@ -136,9 +137,26 @@ public void TestInstrument_ClassesWithExcludeAttributeAreExcluded(Type excludedT instrumenterTest.Directory.Delete(true); } + [Theory] + [InlineData(typeof(ClassExcludedByAttrWithoutAttributeNameSuffix), nameof(TestSDKAutoGeneratedCode))] + public void TestInstrument_ClassesWithExcludeAttributeWithoutAttributeNameSuffixAreExcluded(Type excludedType, string excludedAttribute) + { + var instrumenterTest = CreateInstrumentor(attributesToIgnore: new string[] { excludedAttribute }); + var result = instrumenterTest.Instrumenter.Instrument(); + + var doc = result.Documents.Values.FirstOrDefault(d => Path.GetFileName(d.Path) == "Samples.cs"); + Assert.NotNull(doc); + + var found = doc.Lines.Values.Any(l => l.Class == excludedType.FullName); + Assert.False(found, "Class decorated with with exclude attribute should be excluded"); + + instrumenterTest.Directory.Delete(true); + } + [Theory] [InlineData(nameof(ObsoleteAttribute))] [InlineData("Obsolete")] + [InlineData(nameof(TestSDKAutoGeneratedCode))] public void TestInstrument_ClassesWithCustomExcludeAttributeAreExcluded(string excludedAttribute) { var instrumenterTest = CreateInstrumentor(attributesToIgnore: new string[] { excludedAttribute }); @@ -155,9 +173,10 @@ public void TestInstrument_ClassesWithCustomExcludeAttributeAreExcluded(string e } [Theory] - [InlineData(nameof(ObsoleteAttribute))] - [InlineData("Obsolete")] - public void TestInstrument_ClassesWithMethodWithCustomExcludeAttributeAreExcluded(string excludedAttribute) + [InlineData(nameof(ObsoleteAttribute), "ClassWithMethodExcludedByObsoleteAttr")] + [InlineData("Obsolete", "ClassWithMethodExcludedByObsoleteAttr")] + [InlineData(nameof(TestSDKAutoGeneratedCode), "ClassExcludedByAttrWithoutAttributeNameSuffix")] + public void TestInstrument_ClassesWithMethodWithCustomExcludeAttributeAreExcluded(string excludedAttribute, string testClassName) { var instrumenterTest = CreateInstrumentor(attributesToIgnore: new string[] { excludedAttribute }); var result = instrumenterTest.Instrumenter.Instrument(); @@ -165,7 +184,7 @@ public void TestInstrument_ClassesWithMethodWithCustomExcludeAttributeAreExclude var doc = result.Documents.Values.FirstOrDefault(d => Path.GetFileName(d.Path) == "Samples.cs"); Assert.NotNull(doc); #pragma warning disable CS0612 // Type or member is obsolete - var found = doc.Lines.Values.Any(l => l.Method.Equals("System.String Coverlet.Core.Samples.Tests.ClassWithMethodExcludedByObsoleteAttr::Method(System.String)")); + var found = doc.Lines.Values.Any(l => l.Method.Equals($"System.String Coverlet.Core.Samples.Tests.{testClassName}::Method(System.String)")); #pragma warning restore CS0612 // Type or member is obsolete Assert.False(found, "Method decorated with with exclude attribute should be excluded"); @@ -173,9 +192,10 @@ public void TestInstrument_ClassesWithMethodWithCustomExcludeAttributeAreExclude } [Theory] - [InlineData(nameof(ObsoleteAttribute))] - [InlineData("Obsolete")] - public void TestInstrument_ClassesWithPropertyWithCustomExcludeAttributeAreExcluded(string excludedAttribute) + [InlineData(nameof(ObsoleteAttribute), "ClassWithMethodExcludedByObsoleteAttr")] + [InlineData("Obsolete", "ClassWithMethodExcludedByObsoleteAttr")] + [InlineData(nameof(TestSDKAutoGeneratedCode), "ClassExcludedByAttrWithoutAttributeNameSuffix")] + public void TestInstrument_ClassesWithPropertyWithCustomExcludeAttributeAreExcluded(string excludedAttribute, string testClassName) { var instrumenterTest = CreateInstrumentor(attributesToIgnore: new string[] { excludedAttribute }); var result = instrumenterTest.Instrumenter.Instrument(); @@ -183,12 +203,12 @@ public void TestInstrument_ClassesWithPropertyWithCustomExcludeAttributeAreExclu var doc = result.Documents.Values.FirstOrDefault(d => Path.GetFileName(d.Path) == "Samples.cs"); Assert.NotNull(doc); #pragma warning disable CS0612 // Type or member is obsolete - var getFound = doc.Lines.Values.Any(l => l.Method.Equals("System.String Coverlet.Core.Samples.Tests.ClassWithPropertyExcludedByObsoleteAttr::get_Property()")); + var getFound = doc.Lines.Values.Any(l => l.Method.Equals($"System.String Coverlet.Core.Samples.Tests.{testClassName}::get_Property()")); #pragma warning restore CS0612 // Type or member is obsolete Assert.False(getFound, "Property getter decorated with with exclude attribute should be excluded"); #pragma warning disable CS0612 // Type or member is obsolete - var setFound = doc.Lines.Values.Any(l => l.Method.Equals("System.String Coverlet.Core.Samples.Tests.ClassWithPropertyExcludedByObsoleteAttr::set_Property()")); + var setFound = doc.Lines.Values.Any(l => l.Method.Equals($"System.String Coverlet.Core.Samples.Tests.{testClassName}::set_Property()")); #pragma warning restore CS0612 // Type or member is obsolete Assert.False(setFound, "Property setter decorated with with exclude attribute should be excluded"); diff --git a/test/coverlet.core.tests/Samples/Samples.cs b/test/coverlet.core.tests/Samples/Samples.cs index 12e27d7ba..b3ea51af3 100644 --- a/test/coverlet.core.tests/Samples/Samples.cs +++ b/test/coverlet.core.tests/Samples/Samples.cs @@ -6,6 +6,7 @@ using System.Linq; using System.Threading.Tasks; using Coverlet.Core.Attributes; +using Microsoft.VisualStudio.TestPlatform; namespace Coverlet.Core.Samples.Tests { @@ -213,6 +214,18 @@ public string Method(string input) } } + [TestSDKAutoGeneratedCode] + public class ClassExcludedByAttrWithoutAttributeNameSuffix + { + public string Method(string input) + { + if (string.IsNullOrEmpty(input)) + throw new ArgumentException("Cannot be empty", nameof(input)); + + return input; + } + } + [Obsolete] public class ClassExcludedByObsoleteAttr { diff --git a/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs b/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs index 19964828c..35039a70f 100644 --- a/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs +++ b/test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs @@ -40,8 +40,8 @@ public void GetBranchPoints_OneBranch() Assert.Equal(points[0].Offset, points[1].Offset); Assert.Equal(0, points[0].Path); Assert.Equal(1, points[1].Path); - Assert.Equal(21, points[0].StartLine); - Assert.Equal(21, points[1].StartLine); + Assert.Equal(22, points[0].StartLine); + Assert.Equal(22, points[1].StartLine); Assert.NotNull(points[1].Document); Assert.Equal(points[0].Document, points[1].Document); } @@ -87,8 +87,8 @@ public void GetBranchPoints_TwoBranch() Assert.Equal(4, points.Count()); Assert.Equal(points[0].Offset, points[1].Offset); Assert.Equal(points[2].Offset, points[3].Offset); - Assert.Equal(27, points[0].StartLine); - Assert.Equal(28, points[2].StartLine); + Assert.Equal(28, points[0].StartLine); + Assert.Equal(29, points[2].StartLine); } [Fact] @@ -105,8 +105,8 @@ public void GetBranchPoints_CompleteIf() Assert.NotNull(points); Assert.Equal(2, points.Count()); Assert.Equal(points[0].Offset, points[1].Offset); - Assert.Equal(34, points[0].StartLine); - Assert.Equal(34, points[1].StartLine); + Assert.Equal(35, points[0].StartLine); + Assert.Equal(35, points[1].StartLine); } #if !RELEASE // Issue https://github.com/tonerdo/coverlet/issues/389 @@ -127,10 +127,10 @@ public void GetBranchPoints_Switch() Assert.Equal(points[0].Offset, points[2].Offset); Assert.Equal(3, points[3].Path); - Assert.Equal(46, points[0].StartLine); - Assert.Equal(46, points[1].StartLine); - Assert.Equal(46, points[2].StartLine); - Assert.Equal(46, points[3].StartLine); + Assert.Equal(47, points[0].StartLine); + Assert.Equal(47, points[1].StartLine); + Assert.Equal(47, points[2].StartLine); + Assert.Equal(47, points[3].StartLine); } [Fact] @@ -150,10 +150,10 @@ public void GetBranchPoints_SwitchWithDefault() Assert.Equal(points[0].Offset, points[2].Offset); Assert.Equal(3, points[3].Path); - Assert.Equal(60, points[0].StartLine); - Assert.Equal(60, points[1].StartLine); - Assert.Equal(60, points[2].StartLine); - Assert.Equal(60, points[3].StartLine); + Assert.Equal(61, points[0].StartLine); + Assert.Equal(61, points[1].StartLine); + Assert.Equal(61, points[2].StartLine); + Assert.Equal(61, points[3].StartLine); } [Fact] @@ -173,10 +173,10 @@ public void GetBranchPoints_SwitchWithBreaks() Assert.Equal(points[0].Offset, points[2].Offset); Assert.Equal(3, points[3].Path); - Assert.Equal(76, points[0].StartLine); - Assert.Equal(76, points[1].StartLine); - Assert.Equal(76, points[2].StartLine); - Assert.Equal(76, points[3].StartLine); + Assert.Equal(77, points[0].StartLine); + Assert.Equal(77, points[1].StartLine); + Assert.Equal(77, points[2].StartLine); + Assert.Equal(77, points[3].StartLine); } [Fact] @@ -197,10 +197,10 @@ public void GetBranchPoints_SwitchWithMultipleCases() Assert.Equal(points[0].Offset, points[3].Offset); Assert.Equal(3, points[3].Path); - Assert.Equal(94, points[0].StartLine); - Assert.Equal(94, points[1].StartLine); - Assert.Equal(94, points[2].StartLine); - Assert.Equal(94, points[3].StartLine); + Assert.Equal(95, points[0].StartLine); + Assert.Equal(95, points[1].StartLine); + Assert.Equal(95, points[2].StartLine); + Assert.Equal(95, points[3].StartLine); } #endif From c64e463a6a63b63a052c1f482c98474d719b9fdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher-Marcel=20B=C3=B6ddecker?= Date: Tue, 23 Jun 2020 14:18:12 +0200 Subject: [PATCH 3/4] Reduce allocations from attribute exclusion checks. --- .../Instrumentation/Instrumenter.cs | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index e0b9d971e..23b8f3a26 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -66,7 +66,17 @@ public Instrumenter( _excludeFilters = excludeFilters; _includeFilters = includeFilters; _excludedFilesHelper = new ExcludedFilesHelper(excludedFiles, logger); - _excludedAttributes = excludedAttributes; + _excludedAttributes = (excludedAttributes ?? Array.Empty()) + // In case the attribute class ends in "Attribute", but it wasn't specified. + // Both names are included (if it wasn't specified) because the attribute class might not actually end in the prefix. + .SelectMany(a => a.EndsWith("Attribute") ? new[] { a } : new[] { a, $"{a}Attribute" }) + // The default custom attributes used to exclude from coverage. + .Union(new List() + { + nameof(ExcludeFromCoverageAttribute), + nameof(ExcludeFromCodeCoverageAttribute) + }) + .ToArray(); _singleHit = singleHit; _isCoreLibrary = Path.GetFileNameWithoutExtension(_module) == "System.Private.CoreLib"; _logger = logger; @@ -680,22 +690,7 @@ private static void ReplaceExceptionHandlerBoundary(ExceptionHandler handler, In private bool IsExcludeAttribute(CustomAttribute customAttribute) { - // The default custom attributes used to exclude from coverage. - IEnumerable excludeAttributeNames = new List() - { - nameof(ExcludeFromCoverageAttribute), - nameof(ExcludeFromCodeCoverageAttribute) - }; - - // Include the other attributes to exclude based on incoming parameters. - if (_excludedAttributes != null) - { - excludeAttributeNames = _excludedAttributes.Union(excludeAttributeNames); - } - return excludeAttributeNames.Any(a => - a.EndsWith("Attribute") - ? customAttribute.AttributeType.Name.Equals(a) - : customAttribute.AttributeType.Name.Equals($"{a}Attribute")); + return Array.IndexOf(_excludedAttributes, customAttribute.AttributeType.Name) != -1; } private static MethodBody GetMethodBody(MethodDefinition method) From 272089796b046969178e4f401bcfe37edde8f404 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Christopher-Marcel=20B=C3=B6ddecker?= Date: Tue, 23 Jun 2020 14:27:27 +0200 Subject: [PATCH 4/4] Fix attribute exclusion documentation: Only short names are supported. --- Documentation/MSBuildIntegration.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Documentation/MSBuildIntegration.md b/Documentation/MSBuildIntegration.md index 33b0bf666..0ef0c22f6 100644 --- a/Documentation/MSBuildIntegration.md +++ b/Documentation/MSBuildIntegration.md @@ -111,7 +111,7 @@ dotnet test /p:CollectCoverage=true /p:Threshold=80 /p:ThresholdType=line /p:Thr You can ignore a method an entire class or assembly from code coverage by creating and applying the `ExcludeFromCodeCoverage` attribute present in the `System.Diagnostics.CodeAnalysis` namespace. -You can also ignore additional attributes by using the `ExcludeByAttribute` property (short name or full name supported): +You can also ignore additional attributes by using the `ExcludeByAttribute` property (short name, i.e. the type name without the namespace, supported only): ```bash dotnet test /p:CollectCoverage=true /p:ExcludeByAttribute="Obsolete,GeneratedCodeAttribute,CompilerGeneratedAttribute"