Skip to content

Commit d5b731b

Browse files
committed
Fixing issue #914
Issue #914 reports that it is not possible to achieve 100% coverage on an "await using" statement. For example, given this class: ``` class AwaitUsing { async public ValueTask AsyncAwait() { await using (var ms = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("abc"))) { } // <--- } } ``` Coverlet will not report coverage on the line marked with "<---" above. The root cause appears to be that there are compiler-generated branches in the async state machine generated for an "await using" statement that are not recognized in CecilSymbolHelper. There are three compiler- generated branches that need to be detected and skipped. This commit adds detection for these branches, including the scenario where "await using" statements are nested. Tests are also added to CecilSymbolHelperTests in an attempt to verify the new behavior.
1 parent 8423fae commit d5b731b

File tree

3 files changed

+157
-2
lines changed

3 files changed

+157
-2
lines changed

src/coverlet.core/Symbols/CecilSymbolHelper.cs

Lines changed: 103 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -395,6 +395,106 @@ private bool SkipGeneratedBranchesForExceptionHandlers(MethodDefinition methodDe
395395
return _compilerGeneratedBranchesToExclude[methodDefinition.FullName].Contains(instruction.Offset);
396396
}
397397

398+
private bool SkipAwaitUsingBranches(List<Instruction> instructions, Instruction instruction)
399+
{
400+
/*
401+
Suppose we have an "await using" statement like this one:
402+
403+
await using (var ms = new MemoryStream(Encoding.ASCII.GetBytes("abc")))
404+
{
405+
}
406+
407+
The asynchronously disposable object is stored in a compiler-generated
408+
field with the name <xxx>5__#, where "xxx" is the name of the variable
409+
and "#" is an increasing index number for compiler-generated variables
410+
(so, in this case, it would be <ms>5__1). Where it increases from 1 is
411+
when "await using" is nested.
412+
413+
There are three kinds of branches that the compiler generates that we
414+
should skip.
415+
*/
416+
417+
int currentIndex = instructions.BinarySearch(instruction, new InstructionByOffsetComparer());
418+
419+
/*
420+
The first kind of branch we want to skip is a check that the variable
421+
holding the asychronously disposable object is null. In the presence
422+
of nested "await using" statements, all of these variables are checked
423+
in various places, but the IL for each one follows the same pattern.
424+
425+
if (<ms>5__1 == null) [<--- we want to skip this]
426+
{
427+
goto IL_00b9;
428+
}
429+
430+
IL_0048: ldarg.0
431+
IL_0049: ldfld class [System.Private.CoreLib]System.IO.MemoryStream AwaitUsing/'<AsyncAwait>d__0'::'<ms>5__1'
432+
IL_004e: brfalse.s IL_00b9 [<--- first kind of branch to skip]
433+
*/
434+
if (currentIndex >= 2 &&
435+
instructions[currentIndex - 2].OpCode == OpCodes.Ldarg_0 &&
436+
instructions[currentIndex - 1].OpCode == OpCodes.Ldfld &&
437+
instructions[currentIndex - 1].Operand is FieldReference disposableRef &&
438+
disposableRef.Name.StartsWith("<") && disposableRef.Name.Contains(">5__"))
439+
{
440+
return true;
441+
}
442+
/*
443+
The second and third kinds of branches we want to skip both appear in a
444+
compiler-generated block that checks if an exception has been thrown by a
445+
task and needs to be re-thrown.
446+
447+
obj2 = <>s__2;
448+
if (obj2 != null) [<--- second kind of branch to skip]
449+
{
450+
Exception ex = obj2 as Exception;
451+
if (ex == null) [<--- third kind of branch to skip]
452+
{
453+
throw obj2;
454+
}
455+
ExceptionDispatchInfo.Capture(ex).Throw();
456+
}
457+
458+
IL_00b9: ldarg.0
459+
IL_00ba: ldfld object AwaitUsing/'<AsyncAwait>d__0'::'<>s__2'
460+
IL_00bf: stloc.1
461+
IL_00c0: ldloc.1
462+
IL_00c1: brfalse.s IL_00de [<--- second branch to skip]
463+
IL_00c3: ldloc.1
464+
IL_00c4: isinst [System.Private.CoreLib]System.Exception
465+
IL_00c9: stloc.s 5
466+
IL_00cb: ldloc.s 5
467+
IL_00cd: brtrue.s IL_00d1 [<--- third branch to skip]
468+
*/
469+
else if (currentIndex >= 3 &&
470+
instructions[currentIndex - 3].OpCode == OpCodes.Ldfld &&
471+
instructions[currentIndex - 3].Operand is FieldReference exceptionRef && exceptionRef.Name.Contains("<>s__") &&
472+
instructions[currentIndex - 2].OpCode == OpCodes.Stloc_1 &&
473+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_1)
474+
{
475+
return true;
476+
}
477+
else if (currentIndex >= 4 &&
478+
instructions[currentIndex - 4].OpCode == OpCodes.Ldloc_1 &&
479+
instructions[currentIndex - 3].OpCode == OpCodes.Isinst &&
480+
instructions[currentIndex - 3].Operand is TypeReference exceptionType && exceptionType.FullName == "System.Exception" &&
481+
instructions[currentIndex - 2].OpCode == OpCodes.Stloc_S &&
482+
instructions[currentIndex - 2].Operand is VariableReference variableStore && variableStore.Index == 5 &&
483+
instructions[currentIndex - 1].OpCode == OpCodes.Ldloc_S &&
484+
instructions[currentIndex - 1].Operand is VariableReference variableLoad && variableLoad.Index == 5)
485+
{
486+
return true;
487+
}
488+
489+
/*
490+
The "await using" state machine also contains a compiler-generated check
491+
that the awaiter is completed, but that's being handled already by
492+
SkipIsCompleteAwaiters.
493+
*/
494+
495+
return false;
496+
}
497+
398498
public IReadOnlyList<BranchPoint> GetBranchPoints(MethodDefinition methodDefinition)
399499
{
400500
var list = new List<BranchPoint>();
@@ -435,7 +535,9 @@ public IReadOnlyList<BranchPoint> GetBranchPoints(MethodDefinition methodDefinit
435535

436536
if (isMoveNextInsideAsyncStateMachineProlog)
437537
{
438-
if (SkipMoveNextPrologueBranches(instruction) || SkipIsCompleteAwaiters(instruction))
538+
if (SkipMoveNextPrologueBranches(instruction) ||
539+
SkipIsCompleteAwaiters(instruction) ||
540+
SkipAwaitUsingBranches(instructions, instruction))
439541
{
440542
continue;
441543
}

test/coverlet.core.tests/Samples/Samples.cs

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,27 @@ async public ValueTask AsyncAwait()
196196
await default(ValueTask);
197197
}
198198
}
199+
public class AwaitUsing
200+
{
201+
async public ValueTask AsyncAwait()
202+
{
203+
await using (var ms = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("abc")))
204+
{
205+
}
206+
}
207+
}
208+
209+
public class NestedAwaitUsing
210+
{
211+
async public ValueTask AsyncAwait()
212+
{
213+
await using (var ms1 = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("abc")))
214+
await using (var ms2 = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("def")))
215+
await using (var ms3 = new MemoryStream(System.Text.Encoding.ASCII.GetBytes("ghi")))
216+
{
217+
}
218+
}
219+
}
199220

200221
[ExcludeFromCoverage]
201222
public class ClassExcludedByCoverletCodeCoverageAttr
@@ -304,4 +325,4 @@ public bool False()
304325
return false;
305326
}
306327
}
307-
}
328+
}

test/coverlet.core.tests/Symbols/CecilSymbolHelperTests.cs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -309,6 +309,38 @@ public void GetBranchPoints_IgnoresBranchesIn_AsyncAwaitValueTaskStateMachine()
309309
// assert
310310
Assert.Empty(points);
311311
}
312+
313+
[Fact]
314+
public void GetBranchPoints_IgnoresBranchesIn_AwaitUsing()
315+
{
316+
// arrange
317+
var nestedName = typeof(AwaitUsing).GetNestedTypes(BindingFlags.NonPublic).First().Name;
318+
var type = _module.Types.FirstOrDefault(x => x.FullName == typeof(AwaitUsing).FullName);
319+
var nestedType = type.NestedTypes.FirstOrDefault(x => x.FullName.EndsWith(nestedName));
320+
var method = nestedType.Methods.First(x => x.FullName.EndsWith("::MoveNext()"));
321+
322+
// act
323+
var points = _cecilSymbolHelper.GetBranchPoints(method);
324+
325+
// assert
326+
Assert.Empty(points);
327+
}
328+
329+
[Fact]
330+
public void GetBranchPoints_IgnoresBranchesIn_NestedAwaitUsing()
331+
{
332+
// arrange
333+
var nestedName = typeof(NestedAwaitUsing).GetNestedTypes(BindingFlags.NonPublic).First().Name;
334+
var type = _module.Types.FirstOrDefault(x => x.FullName == typeof(NestedAwaitUsing).FullName);
335+
var nestedType = type.NestedTypes.FirstOrDefault(x => x.FullName.EndsWith(nestedName));
336+
var method = nestedType.Methods.First(x => x.FullName.EndsWith("::MoveNext()"));
337+
338+
// act
339+
var points = _cecilSymbolHelper.GetBranchPoints(method);
340+
341+
// assert
342+
Assert.Empty(points);
343+
}
312344

313345
[Fact]
314346
public void GetBranchPoints_ExceptionFilter()

0 commit comments

Comments
 (0)