From d8cded26105d83d19f2e5c85fda9c70d7f376b39 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller?= Date: Mon, 3 May 2021 15:52:44 +0200 Subject: [PATCH 1/3] skip assigned auto properties and auto records --- .../Abstractions/ICecilSymbolHelper.cs | 1 + .../Instrumentation/Instrumenter.cs | 6 + .../Symbols/CecilSymbolHelper.cs | 55 +++++++++ .../Coverage/CoverageTests.AutoProps.cs | 112 ++++++++++++++++-- .../Samples/Instrumentation.AutoProps.cs | 21 ++++ 5 files changed, 186 insertions(+), 9 deletions(-) diff --git a/src/coverlet.core/Abstractions/ICecilSymbolHelper.cs b/src/coverlet.core/Abstractions/ICecilSymbolHelper.cs index 222a34803..10bf1719a 100644 --- a/src/coverlet.core/Abstractions/ICecilSymbolHelper.cs +++ b/src/coverlet.core/Abstractions/ICecilSymbolHelper.cs @@ -9,5 +9,6 @@ internal interface ICecilSymbolHelper { IReadOnlyList GetBranchPoints(MethodDefinition methodDefinition); bool SkipNotCoverableInstruction(MethodDefinition methodDefinition, Instruction instruction); + bool SkipInlineAssignedAutoProperty(bool skipAutoProps, MethodDefinition methodDefinition, Instruction instruction); } } diff --git a/src/coverlet.core/Instrumentation/Instrumenter.cs b/src/coverlet.core/Instrumentation/Instrumenter.cs index 4ccb0026e..88ca68069 100644 --- a/src/coverlet.core/Instrumentation/Instrumenter.cs +++ b/src/coverlet.core/Instrumentation/Instrumenter.cs @@ -562,6 +562,12 @@ private void InstrumentIL(MethodDefinition method) if (sequencePoint != null && !sequencePoint.IsHidden) { + if (_cecilSymbolHelper.SkipInlineAssignedAutoProperty(_parameters.SkipAutoProps, method, instruction)) + { + index++; + continue; + } + var target = AddInstrumentationCode(method, processor, instruction, sequencePoint); foreach (var _instruction in processor.Body.Instructions) ReplaceInstructionTarget(_instruction, instruction, target); diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 0f823dfb4..ea40ee140 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -1274,6 +1274,61 @@ private bool SkipExpressionBreakpointsSequences(MethodDefinition methodDefinitio return false; } + public bool SkipInlineAssignedAutoProperty(bool skipAutoProps, MethodDefinition methodDefinition, Instruction instruction) + { + if (!skipAutoProps || !methodDefinition.IsConstructor) return false; + + return SkipGeneratedBackingFieldAssignment(methodDefinition, instruction) || + SkipDefaultInitializationSystemObject(instruction); + } + + private static bool SkipGeneratedBackingFieldAssignment(MethodDefinition methodDefinition, Instruction instruction) + { + /* + For inline initialization of properties the compiler generates a field that is set in the constructor of the class. + To skip this we search for compiler generated fields that are set in the constructor. + + .field private string 'k__BackingField' + .custom instance void [System.Runtime]System.Runtime.CompilerServices.CompilerGeneratedAttribute::.ctor() = ( + 01 00 00 00 + ) + + .method public hidebysig specialname rtspecialname + instance void .ctor () cil managed + { + IL_0000: ldarg.0 + IL_0001: ldsfld string[System.Runtime] System.String::Empty + IL_0006: stfld string TestRepro.ClassWithPropertyInit::'k__BackingField' + ... + } + ... + */ + var autogeneratedBackingFields = methodDefinition.DeclaringType.Fields.Where(x => + x.CustomAttributes.Any(ca => ca.AttributeType.FullName.Equals(typeof(CompilerGeneratedAttribute).FullName)) && + x.FullName.Contains("k__BackingField")); + + return instruction.OpCode == OpCodes.Ldarg && + instruction.Next?.Next?.OpCode == OpCodes.Stfld && + instruction.Next?.Next?.Operand is FieldReference fr && + autogeneratedBackingFields.Select(x => x.FullName).Contains(fr.FullName); + } + + private static bool SkipDefaultInitializationSystemObject(Instruction instruction) + { + /* + A type always has a constructor with a default instantiation of System.Object. For record types these + instructions can have a own sequence point. This means that even the default constructor would be instrumented. + To skip this we search for call instructions with a method reference that declares System.Object. + + IL_0000: ldarg.0 + IL_0001: call instance void [System.Runtime]System.Object::.ctor() + IL_0006: ret + */ + return instruction.OpCode == OpCodes.Ldarg && + instruction.Next?.OpCode == OpCodes.Call && + instruction.Next?.Operand is MethodReference mr && mr.DeclaringType.FullName.Equals(typeof(System.Object).FullName); + } + private static bool SkipBranchGeneratedExceptionFilter(Instruction branchInstruction, MethodDefinition methodDefinition) { if (!methodDefinition.Body.HasExceptionHandlers) diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs b/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs index 799912d27..62d971452 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs @@ -15,14 +15,14 @@ public void SkipAutoProps(bool skipAutoProps) string path = Path.GetTempFileName(); try { - FunctionExecutor.Run(async (string[] parameters) => + FunctionExecutor.RunInProcess(async (string[] parameters) => { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { instance.AutoPropsNonInit = 10; instance.AutoPropsInit = 20; - int readVal = instance.AutoPropsNonInit; - readVal = instance.AutoPropsInit; + int readValue = instance.AutoPropsNonInit; + readValue = instance.AutoPropsInit; return Task.CompletedTask; }, persistPrepareResultToFile: parameters[0], skipAutoProps: bool.Parse(parameters[1])); @@ -33,16 +33,110 @@ public void SkipAutoProps(bool skipAutoProps) if (skipAutoProps) { TestInstrumentationHelper.GetCoverageResult(path) - .Document("Instrumentation.AutoProps.cs") - .AssertNonInstrumentedLines(BuildConfiguration.Debug, 12, 12) - .AssertLinesCoveredFromTo(BuildConfiguration.Debug, 7, 11) - .AssertLinesCovered(BuildConfiguration.Debug, (13, 1)); + .Document("Instrumentation.AutoProps.cs") + .AssertNonInstrumentedLines(BuildConfiguration.Debug, 12, 13) + .AssertNonInstrumentedLines(BuildConfiguration.Release, 12, 13) + .AssertLinesCoveredFromTo(BuildConfiguration.Debug, 9, 11) + .AssertLinesCovered(BuildConfiguration.Debug, (7, 1)) + .AssertLinesCovered(BuildConfiguration.Release, (10, 1)); } else { TestInstrumentationHelper.GetCoverageResult(path) - .Document("Instrumentation.AutoProps.cs") - .AssertLinesCoveredFromTo(BuildConfiguration.Debug, 7, 13); + .Document("Instrumentation.AutoProps.cs") + .AssertLinesCoveredFromTo(BuildConfiguration.Debug, 7, 13) + .AssertLinesCoveredFromTo(BuildConfiguration.Release, 10, 10) + .AssertLinesCoveredFromTo(BuildConfiguration.Release, 12, 13); + } + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void SkipAutoPropsInRecords(bool skipAutoProps) + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.RunInProcess(async (string[] parameters) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + instance.RecordAutoPropsNonInit = string.Empty; + instance.RecordAutoPropsInit = string.Empty; + string readValue = instance.RecordAutoPropsInit; + readValue = instance.RecordAutoPropsNonInit; + return Task.CompletedTask; + }, + persistPrepareResultToFile: parameters[0], skipAutoProps: bool.Parse(parameters[1])); + + return 0; + }, new string[] { path, skipAutoProps.ToString() }); + + if (skipAutoProps) + { + TestInstrumentationHelper.GetCoverageResult(path).GenerateReport(show: true) + .Document("Instrumentation.AutoProps.cs") + .AssertNonInstrumentedLines(BuildConfiguration.Debug, 23, 24) + .AssertNonInstrumentedLines(BuildConfiguration.Release, 23, 24) + .AssertLinesCovered(BuildConfiguration.Debug, (18, 1), (20, 1), (21, 1), (22, 1)) + .AssertLinesCovered(BuildConfiguration.Release, (21, 1)); + } + else + { + TestInstrumentationHelper.GetCoverageResult(path) + .Document("Instrumentation.AutoProps.cs") + .AssertLinesCoveredFromTo(BuildConfiguration.Debug, 18, 24) + .AssertLinesCoveredFromTo(BuildConfiguration.Release, 21, 21) + .AssertLinesCoveredFromTo(BuildConfiguration.Release, 23, 24); + } + } + finally + { + File.Delete(path); + } + } + + [Theory] + [InlineData(true)] + [InlineData(false)] + public void SkipRecordWithProperties(bool skipAutoProps) + { + string path = Path.GetTempFileName(); + try + { + FunctionExecutor.RunInProcess(async (string[] parameters) => + { + CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => + { + return Task.CompletedTask; + }, + persistPrepareResultToFile: parameters[0], skipAutoProps: bool.Parse(parameters[1])); + + return 0; + }, new string[] { path, skipAutoProps.ToString() }); + + if (skipAutoProps) + { + TestInstrumentationHelper.GetCoverageResult(path) + .Document("Instrumentation.AutoProps.cs") + .AssertNonInstrumentedLines(BuildConfiguration.Debug, 29, 29) + .AssertNonInstrumentedLines(BuildConfiguration.Release, 29, 29) + .AssertLinesCovered(BuildConfiguration.Debug, (32, 1), (33, 1), (34, 1)) + .AssertLinesCovered(BuildConfiguration.Release, (33, 1)); + + } + else + { + TestInstrumentationHelper.GetCoverageResult(path) + .Document("Instrumentation.AutoProps.cs") + .AssertLinesCovered(BuildConfiguration.Debug, (29, 3), (31, 1), (32, 1), (33, 1), (34, 1)) + .AssertLinesCovered(BuildConfiguration.Release, (29, 3), (31, 1), (33, 1)); } } finally diff --git a/test/coverlet.core.tests/Samples/Instrumentation.AutoProps.cs b/test/coverlet.core.tests/Samples/Instrumentation.AutoProps.cs index 4378ca9e7..852fcb182 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.AutoProps.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.AutoProps.cs @@ -12,4 +12,25 @@ public AutoProps() public int AutoPropsNonInit { get; set; } public int AutoPropsInit { get; set; } = 10; } + + public record RecordWithPropertyInit + { + private int _myRecordVal = 0; + public RecordWithPropertyInit() + { + _myRecordVal = new Random().Next(); + } + public string RecordAutoPropsNonInit { get; set; } + public string RecordAutoPropsInit { get; set; } = string.Empty; + } + + public class ClassWithAutoRecordProperties + { + record AutoRecordWithProperties(string Prop1, string Prop2); + + public ClassWithAutoRecordProperties() + { + var record = new AutoRecordWithProperties(string.Empty, string.Empty); + } + } } From 74c6d037f752ae9e7ace9f5369243fda7d11c126 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller?= Date: Mon, 3 May 2021 16:10:09 +0200 Subject: [PATCH 2/3] bugfix --- .../coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs b/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs index 62d971452..8b923da86 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTests.AutoProps.cs @@ -15,7 +15,7 @@ public void SkipAutoProps(bool skipAutoProps) string path = Path.GetTempFileName(); try { - FunctionExecutor.RunInProcess(async (string[] parameters) => + FunctionExecutor.Run(async (string[] parameters) => { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { @@ -63,7 +63,7 @@ public void SkipAutoPropsInRecords(bool skipAutoProps) string path = Path.GetTempFileName(); try { - FunctionExecutor.RunInProcess(async (string[] parameters) => + FunctionExecutor.Run(async (string[] parameters) => { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { @@ -110,7 +110,7 @@ public void SkipRecordWithProperties(bool skipAutoProps) string path = Path.GetTempFileName(); try { - FunctionExecutor.RunInProcess(async (string[] parameters) => + FunctionExecutor.Run(async (string[] parameters) => { CoveragePrepareResult coveragePrepareResult = await TestInstrumentationHelper.Run(instance => { From 8b1547b441883c26edfcbc68e24102d720a141e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller?= Date: Wed, 5 May 2021 01:03:16 +0200 Subject: [PATCH 3/3] nit --- src/coverlet.core/Symbols/CecilSymbolHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index ea40ee140..44942fab1 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -1305,7 +1305,7 @@ instance void .ctor () cil managed */ var autogeneratedBackingFields = methodDefinition.DeclaringType.Fields.Where(x => x.CustomAttributes.Any(ca => ca.AttributeType.FullName.Equals(typeof(CompilerGeneratedAttribute).FullName)) && - x.FullName.Contains("k__BackingField")); + x.FullName.EndsWith("k__BackingField")); return instruction.OpCode == OpCodes.Ldarg && instruction.Next?.Next?.OpCode == OpCodes.Stfld &&