Uncovered branch in compiler-generated state machine for `yield return`

[bert2]

The C# compiler generates a state machine for each method that uses the yield return syntax. Unfortunately this state machine contains a branch that will never be visited by the execution and thus results in inaccurate branch coverage measurements.

Repro steps

Create a minimal class library with the following code (ClassLibrary1.cs):

namespace ClassLibrary1 {
    public static class Bar {
        public static System.Collections.Generic.IEnumerable<int> Foo() { yield return 1; }
    }
}

Create a minimal xUnit test project. Add the coverlet.msbuild package (2.8.1) and add the following code (UnitTest1.cs):

using Xunit;

namespace XUnitTestProject1 {
    public class UnitTest1
    {
        [Fact]
        public void Test1()
        {
            Assert.Equal(1, Assert.Single(ClassLibrary1.Bar.Foo()));
        }
    }
}

Run coverlet:

PS> dotnet test /p:CollectCoverage=true /p:CoverletOutputFormat=opencover

Expected

Since the method Bar.Foo() has no branches, the reported branch coverage should be 100%.

Actual

Test run for D:\DEV\XUnitTestProject1\bin\Debug\netcoreapp3.1\XUnitTestProject1.dll(.NETCoreApp,Version=v3.1)
Microsoft (R) Test Execution Command Line Tool Version 16.5.0
Copyright (c) Microsoft Corporation.  All rights reserved.

Starting test execution, please wait...

A total of 1 test files matched the specified pattern.

Test Run Successful.
Total tests: 1
     Passed: 1
 Total time: 1.4069 Seconds

Calculating coverage result...
  Generating report 'D:\DEV\XUnitTestProject1\coverage.opencover.xml'

+---------------+------+--------+--------+
| Module        | Line | Branch | Method |
+---------------+------+--------+--------+
| ClassLibrary1 | 100% | 50%    | 100%   |
+---------------+------+--------+--------+

+---------+------+--------+--------+
|         | Line | Branch | Method |
+---------+------+--------+--------+
| Total   | 100% | 50%    | 100%   |
+---------+------+--------+--------+
| Average | 100% | 50%    | 100%   |
+---------+------+--------+--------+

Suspected cause

The report coverage.opencover.xml states that the uncovered branch is in the method ClassLibrary1.Bar.<Foo>d__0.MoveNext():

<Method cyclomaticComplexity="2" nPathComplexity="0" sequenceCoverage="100" branchCoverage="50"
        isConstructor="False" isGetter="False" isSetter="False" isStatic="True">
  <Summary numSequencePoints="1" visitedSequencePoints="1" numBranchPoints="2"
           visitedBranchPoints="1" sequenceCoverage="100" branchCoverage="50"
           maxCyclomaticComplexity="2" minCyclomaticComplexity="2" visitedClasses="0"
           numClasses="0" visitedMethods="1" numMethods="1" />
  <MetadataToken />
  <Name>System.Boolean ClassLibrary1.Bar/&lt;Foo&gt;d__0::MoveNext()</Name>

full report

<?xml version="1.0" encoding="utf-8"?>
<CoverageSession>
  <Summary numSequencePoints="1" visitedSequencePoints="1" numBranchPoints="2" visitedBranchPoints="1" sequenceCoverage="100" branchCoverage="50" maxCyclomaticComplexity="2" minCyclomaticComplexity="2" visitedClasses="1" numClasses="1" visitedMethods="1" numMethods="1" />
  <Modules>
    <Module hash="0AA3E2D3-850E-44FF-A43D-06C13D27F9A8">
      <ModulePath>ClassLibrary1.dll</ModulePath>
      <ModuleTime>2020-04-12T12:08:13</ModuleTime>
      <ModuleName>ClassLibrary1</ModuleName>
      <Files>
        <File uid="1" fullPath="D:\DEV\ClassLibrary1\Class1.cs" />
      </Files>
      <Classes>
        <Class>
          <Summary numSequencePoints="1" visitedSequencePoints="1" numBranchPoints="2" visitedBranchPoints="1" sequenceCoverage="100" branchCoverage="50" maxCyclomaticComplexity="2" minCyclomaticComplexity="2" visitedClasses="1" numClasses="1" visitedMethods="1" numMethods="1" />
          <FullName>ClassLibrary1.Bar/&lt;Foo&gt;d__0</FullName>
          <Methods>
            <Method cyclomaticComplexity="2" nPathComplexity="0" sequenceCoverage="100" branchCoverage="50" isConstructor="False" isGetter="False" isSetter="False" isStatic="True">
              <Summary numSequencePoints="1" visitedSequencePoints="1" numBranchPoints="2" visitedBranchPoints="1" sequenceCoverage="100" branchCoverage="50" maxCyclomaticComplexity="2" minCyclomaticComplexity="2" visitedClasses="0" numClasses="0" visitedMethods="1" numMethods="1" />
              <MetadataToken />
              <Name>System.Boolean ClassLibrary1.Bar/&lt;Foo&gt;d__0::MoveNext()</Name>
              <FileRef uid="1" />
              <SequencePoints>
                <SequencePoint vc="3" uspid="5" ordinal="0" sl="5" sc="1" el="5" ec="2" bec="2" bev="1" fileid="1" />
              </SequencePoints>
              <BranchPoints>
                <BranchPoint vc="0" uspid="5" ordinal="0" path="0" offset="14" offsetend="22" sl="5" fileid="1" />
                <BranchPoint vc="1" uspid="5" ordinal="1" path="1" offset="14" offsetend="48" sl="5" fileid="1" />
              </BranchPoints>
              <MethodPoint vc="1" uspid="0" p8:type="SequencePoint" ordinal="0" offset="0" sc="0" sl="5" ec="1" el="5" bec="0" bev="0" fileid="1" xmlns:p8="xsi" />
            </Method>
          </Methods>
        </Class>
      </Classes>
    </Module>
  </Modules>
</CoverageSession>

Inspecting the <Foo>d__0 class generated by the compiler we can see that its MoveNext() method has a default case inside the switch on the <>1__state variable:

private int <>1__state;

// ...

private bool MoveNext()
{
    switch (<>1__state)
    {
    default:
        return false;
    case 0:
        <>1__state = -1;
        <>2__current = 1;
        <>1__state = 1;
        return true;
    case 1:
        <>1__state = -1;
        return false;
    }
}

full decompilation

// ClassLibrary1.Bar.<Foo>d__0
using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Runtime.CompilerServices;

[CompilerGenerated]
private sealed class <Foo>d__0 : IEnumerable<int>, IEnumerable, IEnumerator<int>, IEnumerator, IDisposable
{
	private int <>1__state;

	private int <>2__current;

	private int <>l__initialThreadId;

	int IEnumerator<int>.Current
	{
		[DebuggerHidden]
		get
		{
			return <>2__current;
		}
	}

	object IEnumerator.Current
	{
		[DebuggerHidden]
		get
		{
			return <>2__current;
		}
	}

	[DebuggerHidden]
	public <Foo>d__0(int <>1__state)
	{
		this.<>1__state = <>1__state;
		<>l__initialThreadId = Environment.CurrentManagedThreadId;
	}

	[DebuggerHidden]
	void IDisposable.Dispose()
	{
	}

	private bool MoveNext()
	{
		switch (<>1__state)
		{
		default:
			return false;
		case 0:
			<>1__state = -1;
			<>2__current = 1;
			<>1__state = 1;
			return true;
		case 1:
			<>1__state = -1;
			return false;
		}
	}

	bool IEnumerator.MoveNext()
	{
		//ILSpy generated this explicit interface implementation from .override directive in MoveNext
		return this.MoveNext();
	}

	[DebuggerHidden]
	void IEnumerator.Reset()
	{
		throw new NotSupportedException();
	}

	[DebuggerHidden]
	IEnumerator<int> IEnumerable<int>.GetEnumerator()
	{
		if (<>1__state == -2 && <>l__initialThreadId == Environment.CurrentManagedThreadId)
		{
			<>1__state = 0;
			return this;
		}
		return new <Foo>d__0(0);
	}

	[DebuggerHidden]
	IEnumerator IEnumerable.GetEnumerator()
	{
		return ((IEnumerable<int>)this).GetEnumerator();
	}
}

I'm not 100% sure, but I think this default case is never being visited.

[MarcoRossignoli]

Thank's for reporting this, coverlet "handles" compilers generated branch https://github.com/tonerdo/coverlet/blob/master/test/coverlet.core.tests/Samples/Samples.cs#L168, maybe here there is a coverage but, I'll take a look asap.

[bert2]

That's interesting. If you turn the iterator sample from...

public IEnumerable<string> Fetch()
{
    yield return "one";
    yield return "two";
}

...into...

public IEnumerable<string> Fetch()
{
    yield return "one";
}

...then the test GetBranchPoints_IgnoresSwitchIn_GeneratedMoveNext will fail:

Assert.Empty() Failure
Collection: [BranchPoint { Document = "D:\DEV\coverlet\test\coverlet.core.tests\Samp"..., EndOffset = 22, Offset = 14, OffsetPoints = [...], Ordinal = 0, ... }, BranchPoint { Document = "D:\DEV\coverlet\test\coverlet.core.tests\Samp"..., EndOffset = 52, Offset = 14, OffsetPoints = [...], Ordinal = 1, ... }]
Stack Trace:
CecilSymbolHelperTests.GetBranchPoints_IgnoresSwitchIn_GeneratedMoveNext() line 261

[MarcoRossignoli]

With only one yield seem no more a switch emitted or a bug on pattern recognition, we need to fix.
Thank's for help on investigation.