Can't use [ExcludeFromCodeCoverage] on a static method?

[romainf-ubi]

I have this C# code:

internal static class PolicyNames
{
    public const string Read = nameof(Read);
    public const string Write = nameof(Write);

    [ExcludeFromCodeCoverage]
    public static IEnumerable<string> All()
    {
        yield return Read;
        yield return Write;
    }
}

I use the NuGet package coverlet.collector 3.2.0, and I use dotnet test -c Release --collect="XPlat Code Coverage" to generate the coverage reports.

Apparently my method All() is still covered. If I move [ExcludeFromCodeCoverage] over to the static class, then All() is excluded (but also is the whole class).

Note: I opened this issue after writing this comment on another closed issue.

[daveMueller]

Thanks for reporting this. I can reproduce it 👍 . It seems to be somehow related to the yield statement. If I just returned a collection

[ExcludeFromCodeCoverage]
public static IEnumerable<string> All()
{
    return new []{Read, Write};
}

everything works fine. Peaking into the IL it seems the compiler is generating a class that additionally needs to be excluded. A bit weird is that there was a similar issue like this in the past where I couldn't reproduce it (#1431). I'm glad we now have a simple repro for that. 😊

[daveMueller]

@MarcoRossignoli @petli I'm really sure that this is a problem in our algorithm for excluding synthesized members (https://github.com/coverlet-coverage/coverlet/blob/master/src/coverlet.core/Instrumentation/Instrumenter.cs#L801).
I reached out to the roslyn team but haven't gotten a real answer tho dotnet/roslyn#68664.

Do you have an idea who wrote this algorithm? Or do you think we might can reduce complexity here by not checking the exact ordinal anymore?

[MarcoRossignoli]

@daveMueller at line 839 of the same file you should see the link to the roslyn repo for the symbol name generation, this part was done years ago and it's possible and expected some change in the compiler that will invalidate our heuristic. The code was written by my friend @matteoerigozzi

[daveMueller]

I start working on this issue ...

[romainf-ubi]

Thanks @daveMueller!

Could this be related to #1503, where I have some [Test] methods that are not excluded from the code coverage, thus drastically lowering the final score?

[daveMueller]

@romainf-ubi No this is a different issue. Here it really seems that the roslyn team did some changes in the compiler and one of our algorithms to detect IL code that belongs together doesn't work anymore. The compiler generates code that we are not mapping properly to the excluded function anymore.

[daveMueller]

I have realized that I currently can't solve this issue as it is much more complicated than I initially thought. The problem is that the compiler generates IL code that we can't map properly to the excluded method. I'm not sure if this is because the roslyn team changed something or if this bug has always existed.
There are special types the compiler generates for lambda methods, lambda display classes, state machines and local functions. The names of these types are generated using the method name and the member-index of the method. The member-index is the index the method has in the member list of the containing type. And here is the issue with our current algorithm. Currently we only count methods as members. For instance in the following example the method TestLambda(string input) has the index 0 and TestLambda(string input, int value) has the index 1.

public class MethodsWithExcludeFromCodeCoverageAttr
{
    [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
    public int TestLambda(string input)
    {
        System.Func<string, int> lambdaFunc = s => s.Length;
        return lambdaFunc(input);
    }

    public int TestLambda(string input, int value)
    {
        System.Func<string, int> lambdaFunc = s => s.Length;
        return lambdaFunc(input) + value;
    }
}

Now if we add another member to the class that isn't a method, the indices change. Just adding a simple field like in the following snipped changes the indices for the method TestLambda(string input) to 1 and TestLambda(string input, int value) to 2 and our current algorithm fails.

public class MethodsWithExcludeFromCodeCoverageAttr
{
    private string _fieldToInfluenceSynthesizedName;

    [System.Diagnostics.CodeAnalysis.ExcludeFromCodeCoverage]
    public int TestLambda(string input)
    {
        _fieldToInfluenceSynthesizedName = string.Empty;
        System.Func<string, int> lambdaFunc = s => s.Length;
        return lambdaFunc(input);
    }

    public int TestLambda(string input, int value)
    {
        System.Func<string, int> lambdaFunc = s => s.Length;
        return lambdaFunc(input) + value;
    }
}

Now one might think why is this index so important? Lets just use a regular expression to find that type e.g. something like

Regex.IsMatch(name, $@"<{methodName}>b__(\d+)")

The problem here is that the names of those generated types don't consider the parameters of overloaded methods. For the example above the compile would generate the types <TestLambda>b__1 and <TestLambda>b__2 which both would match the expression.

I tried out a lot of different options on how to solve this issue but there are always some cases I couldn't cover without knowing this index. Unfortunately up to now I couldn't figure out an algorithm to map those generated types to the excluded methods which covers all edge cases.

Thus I now reached out to the community of cecil and roslyn and hope I get some response. Best would be if I can find the member index somewhere in the PDB...
@MarcoRossignoli maybe you can ask @jakubch1 during a coffee break for some advice 😉.

[MarcoRossignoli]

@jakubch1 do you recall how/if we handle this use case?

[jakubch1]

@daveMueller I was in past investigating same issue and was not able to find way to get this ID. So, we decided to use different approach. We store information for each method with ExcludeFromCodeCoverage attibute: source files + first and last line for each source file (based on sequence points). Then when any method is inside such "excluded" range we are also excluding it.

In your example if public int TestLambda(string input) is excluded we know from sequence points that all methods which have sequence points in file MethodsWithExcludeFromCodeCoverageAttr.cs in lines range 4-8 should also be excluded.

For async methods you can find connection to MoveNext methods in more elegant way (without checking sources like above):

    public int GetStateMachineKickoffMethod(MethodDefinitionHandle handle)
    {
        var method = _moduleDefinition.LookupToken(MetadataTokens.GetToken(handle)) as MethodDefinition;
        return method?.DebugInformation?.StateMachineKickOffMethod?.MetadataToken.ToInt32() ?? -1;
    }

[daveMueller]

OK thanks a lot @jakubch1 ☺️. I tried something similar but then had issues with nested async methods. I didn't try to use the StateMachineKickoffMethod tho'. So I will give it another try with your suggestions. Thanks again 🙏.