From 723d3418409b928670bf658d3632756c8d1c7579 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20M=C3=BCller?= Date: Thu, 11 Apr 2024 21:00:41 +0200 Subject: [PATCH] Implemented fix and refactoring (#1647) --- Documentation/Changelog.md | 3 ++ .../Symbols/CecilSymbolHelper.cs | 52 +++++++++++-------- .../Coverage/CoverageTests.AwaitUsing.cs | 5 +- .../Samples/Instrumentation.AwaitUsing.cs | 5 ++ 4 files changed, 42 insertions(+), 23 deletions(-) diff --git a/Documentation/Changelog.md b/Documentation/Changelog.md index f18a95d0e..67fea02d4 100644 --- a/Documentation/Changelog.md +++ b/Documentation/Changelog.md @@ -6,6 +6,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed +- Fix incorrect coverage await using in generic method [#1490](https://github.com/coverlet-coverage/coverlet/issues/1490) + ## Release date 2024-03-13 ### Packages coverlet.msbuild 6.0.2 diff --git a/src/coverlet.core/Symbols/CecilSymbolHelper.cs b/src/coverlet.core/Symbols/CecilSymbolHelper.cs index 2f381118e..3b4413780 100644 --- a/src/coverlet.core/Symbols/CecilSymbolHelper.cs +++ b/src/coverlet.core/Symbols/CecilSymbolHelper.cs @@ -45,6 +45,22 @@ private static bool IsCompilerGenerated(FieldDefinition fieldDefinition) return fieldDefinition.DeclaringType.CustomAttributes.Any(ca => ca.AttributeType.FullName == typeof(CompilerGeneratedAttribute).FullName); } + private static bool IsCompilerGeneratedField(Instruction instruction, out FieldDefinition field) + { + switch (instruction.Operand) + { + case FieldDefinition fieldDefinition when IsCompilerGenerated(fieldDefinition): + field = fieldDefinition; + return true; + case FieldReference fieldReference when IsCompilerGenerated(fieldReference.Resolve()): + field = fieldReference.Resolve(); + return true; + default: + field = null; + return false; + } + } + private static bool IsMoveNextInsideAsyncStateMachine(MethodDefinition methodDefinition) { if (methodDefinition.FullName.EndsWith("::MoveNext()") && IsCompilerGenerated(methodDefinition)) @@ -537,11 +553,9 @@ static bool CheckForAsyncEnumerator(List instructions, Instruction (instructions[currentIndex - 2].OpCode == OpCodes.Ldarg || instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0) && instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && - ( - (instructions[currentIndex - 1].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")) || - (instructions[currentIndex - 1].Operand is FieldReference fieldRef && IsCompilerGenerated(fieldRef.Resolve()) && fieldRef.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator")) - ) - ) + IsCompilerGeneratedField(instructions[currentIndex - 1], out FieldDefinition field) && + field.FieldType.FullName.StartsWith("System.Collections.Generic.IAsyncEnumerator") + ) { return true; } @@ -582,10 +596,8 @@ static bool CheckIfExceptionThrown(List instructions, Instruction i for (int i = currentIndex - 1; i >= minFieldIndex; --i) { if (instructions[i].OpCode == OpCodes.Ldfld && - ( - (instructions[i].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FieldType.FullName == "System.Object") || - (instructions[i].Operand is FieldReference fieldRef && IsCompilerGenerated(fieldRef.Resolve()) && fieldRef.FieldType.FullName == "System.Object") - )) + IsCompilerGeneratedField(instructions[i], out FieldDefinition field) && + field.FieldType.FullName == "System.Object") { // We expect the call to GetResult() to be no more than four // instructions before the loading of the field's value. @@ -708,16 +720,16 @@ static bool CheckForSkipDisposal(List instructions, Instruction ins bool isFollowedByDisposeAsync = false; - if (instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && - instructions[currentIndex - 1].Operand is FieldDefinition field && - IsCompilerGenerated(field)) + if (instructions[currentIndex - 1].OpCode == OpCodes.Ldfld) { + if(! IsCompilerGeneratedField(instructions[currentIndex - 1], out FieldDefinition field)) return false; + int maxReloadFieldIndex = Math.Min(currentIndex + 2, instructions.Count - 2); for (int i = currentIndex + 1; i <= maxReloadFieldIndex; ++i) { if (instructions[i].OpCode == OpCodes.Ldfld && - instructions[i].Operand is FieldDefinition reloadedField && + IsCompilerGeneratedField(instructions[i], out FieldDefinition reloadedField) && field.Equals(reloadedField) && instructions[i + 1].OpCode == OpCodes.Callvirt && instructions[i + 1].Operand is MethodReference method && @@ -758,8 +770,8 @@ instructions[i].Operand is FieldDefinition reloadedField && if ((instructions[i].OpCode == OpCodes.Leave || instructions[i].OpCode == OpCodes.Leave_S) && instructions[i - 1].OpCode == OpCodes.Stfld && - instructions[i - 1].Operand is FieldDefinition storeField && - IsCompilerGenerated(storeField)) + IsCompilerGeneratedField(instructions[i - 1], out FieldDefinition _) + ) { return true; } @@ -811,9 +823,7 @@ static bool CheckForCleanup(List instructions, Instruction instruct for (int i = currentIndex - 2; i >= minLoadFieldIndex; --i) { - if (instructions[i].OpCode == OpCodes.Ldfld && - instructions[i].Operand is FieldDefinition loadedField && - IsCompilerGenerated(loadedField)) + if (instructions[i].OpCode == OpCodes.Ldfld && IsCompilerGeneratedField(instructions[i], out FieldDefinition _)) { int minRethrowIndex = Math.Max(0, i - 4); @@ -918,10 +928,8 @@ static bool DisposeCheck(List instructions, Instruction instruction if (currentIndex >= 2 && instructions[currentIndex - 1].OpCode == OpCodes.Ldfld && - ( - (instructions[currentIndex - 1].Operand is FieldDefinition field && IsCompilerGenerated(field) && field.FullName.EndsWith("__disposeMode")) || - (instructions[currentIndex - 1].Operand is FieldReference fieldRef && IsCompilerGenerated(fieldRef.Resolve()) && fieldRef.FullName.EndsWith("__disposeMode")) - ) && + IsCompilerGeneratedField(instructions[currentIndex - 1], out FieldDefinition field) && + field.FullName.EndsWith("__disposeMode") && (instructions[currentIndex - 2].OpCode == OpCodes.Ldarg || instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0)) { diff --git a/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs b/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs index e6c7be373..75d7b2acd 100644 --- a/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs +++ b/test/coverlet.core.tests/Coverage/CoverageTests.AwaitUsing.cs @@ -23,6 +23,7 @@ public void AwaitUsing() { await (ValueTask)instance.HasAwaitUsing(); await (Task)instance.Issue914_Repro(); + await (Task)instance.Issue1490_Repro(); }, persistPrepareResultToFile: pathSerialize[0]); return 0; @@ -39,8 +40,10 @@ public void AwaitUsing() (28, 1), (29, 1), (30, 1), // Issue914_Repro_Example2() (34, 1), (35, 1), (36, 1), (37, 1), + // Issue1490_Repro() + (40, 1), (41, 1), (42, 1), (43, 1), // MyTransaction.DisposeAsync() - (43, 2), (44, 2), (45, 2) + (48, 3), (49, 3), (50, 3) ) .ExpectedTotalNumberOfBranches(BuildConfiguration.Debug, 0); } diff --git a/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs b/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs index 9353a8c46..a0586ab7c 100644 --- a/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs +++ b/test/coverlet.core.tests/Samples/Instrumentation.AwaitUsing.cs @@ -36,6 +36,11 @@ async private Task Issue914_Repro_Example2() await transaction.DisposeAsync(); } + async public Task Issue1490_Repro() + { + await using var transaction = new MyTransaction(); + return default(T); + } private class MyTransaction : IAsyncDisposable {