Roslyn: XUnit [Fact] tests grayed out due to 0 references

[vsfeedback]

VSF_TYPE_MARKDOWNThe VS 2019 feature to show unreferenced function names in a gray color is great, but it shouldn't apply to XUnit tests marked with [Fact].

_This issue has been moved from https://developercommunity.visualstudio.com/content/problem/560798/xunit-fact-tests-grayed-out-due-to-0-references.html
VSTS ticketId: 889034_
_These are the original issue comments:_

Visual Studio Feedback System on 5/8/2019, 02:45 AM (61 days ago):

We have directed your feedback to the appropriate engineering team for further evaluation. The team will review the feedback and notify you about the next steps.

_These are the original issue solutions:_
(no solutions)

[CyrusNajmabadi]

A simple solution is to make the tests public.

[sharwell]

💡 The xunit analyzers also assume that the pattern from https://github.com/dotnet/roslyn/issues/37050#issuecomment-509413089 is followed. See xUnit1013 for example.

[0xd4d]

A simple solution is to make the tests public.

What should you do if you have more than 10 test methods?

💡 The xunit analyzers also assume that the pattern from #37050 (comment) is followed. See xUnit1013 for example.

The link explains that xunit analyzer warns if you have a public method that is not a test method. This issue seems to be about tests that are private.

```C#
using Xunit;

namespace XUnitTestProject1
{
public class UnitTest1
{
[Fact]
public void Test1() { }

    // IDE0051 Private member 'UnitTest1.Test2' is unused.
    [Fact]
    void Test2() { }

    // xUnit1013: Public method 'Test3' on test class 'UnitTest1' should be marked as a Fact.
    public void Test3() { }
}

}
```

[CyrusNajmabadi]

What should you do if you have more than 10 test methods?

Make them all public.

This issue seems to be about tests that are private.

Don't have private test methods. Your test methods themselves can be public. They can call private helpers. In that case, then helpers will be 'referenced' and will not be marked as gray. All your actual test method sshould be public and should have the appropriate [Fact] attribute on htem.

[0xd4d]

I'm not the one that reported this issue.

Make them all public.

Tests can be private, so I say, the IDE bug should be fixed. :) xunit is a popular testing framework, it should be supported by the IDE analyzer. Right now it wants to remove every single test. It's great if you have 1000s of failing tests, though!

Don't have private test methods. Your test methods themselves can be public. [...] All your actual test method sshould be public and should have the appropriate [Fact] attribute on htem.

Sounds like tabs vs spaces. Some people prefer private methods because it's allowed, others (like you) prefer public test methods.

[CyrusNajmabadi]

Tests can be private, so I say, the IDE bug should be fixed.

It's not a bug. You have a private unreferenced method. That's all the feature is telling you. You don't have to remove it if you believe it will be used (i.e. through reflection). You'll get this behavior with any ode you write that you only intend to be called by reflection. Solutions are to suppress the analysis here, or indicate in some way that you intend for the test code to be called.

Some people prefer private methods because it's allowed

Then suppress the analyzer here. Private unreferenced methods are shown to the user as being exactly that.

[sharwell]

I believe this issue can be resolved by either of the following:

1. The test method can be marked public
2. An analyzer package (e.g. xunit.analyzers) provides a suppression analyzer to suppress the diagnostic reported for the unused method

[Mike-E-angelo]

I believe this issue can be resolved by either of the following:

I understand that I can make my tests public, but xUnit runs the tests fine while making them private, thus private methods are allowed and are not considered an errored state.

_xUnit relies not on the visibility of a method but whether it is decorated with the appropriate metadata._

The message is simply inaccurate as it is stating that a method is not being used when it, in fact, is.

That stated, there is an obvious 3rd option that can be employed, which I am now using since this issue appears to be (wrongly 😝) closed:

<NoWarn>IDE0051</NoWarn>

👆 should have thought of that sooner. 😆

[CyrusNajmabadi]

The message is simply inaccurate as it is stating that a method is not being used when it, in fact, is

The method isn't referenced anywhere. Xunit uses reflection for these. And very specifically reflection can't count as a 'usage' otherwise everything is used.

[Mike-E-angelo]

The method isn't referenced anywhere. _Xunit uses reflection for these._

So... it _is_ referenced? 😉 I get what you're saying, though. In my case, the NoWarn is exactly what I need, which is to remove the message from my error/warning/message list. This should be a more prominent suggestion rather than simply dismissing this issue as "won't fix."

[CyrusNajmabadi]

So... it is referenced?

As mentioned above: And very specifically reflection can't count as a 'usage' otherwise everything is used.

Literally nothing could be greyed out or reported as removable if reflection is party if the equation. That's not helpful, so we take the position of checking non-reflection with the caveat that most find reasonable that it's up to you to not remove things you're reflection tools need.

[sharwell]

@Mike-EEE We already shipped the ability for analyzer packages to provide suppression analyzers. Third-party library authors can use these in an analyzer package to automate the suppression of known false positives caused by the library introducing behavior outside the strict scope of the language rules. In this case, xunit.analyzers would be responsible for suppressing IDE0051 if its associated library deemed this reflection to be a "use" of the method. However, if the library authors instead find that the suggestion above (https://github.com/dotnet/roslyn/issues/37050#issuecomment-509413089) is a more desirable resolution, they may opt to provide a diagnostic and code fix to automatically mark test methods as public rather than suppress the unused diagnostic.

[Mike-E-angelo]

Thank you for the additional context and for your patience in explaining this to me, @sharwell and @CyrusNajmabadi. To add to the chaos, there was some additional confusion on my side as I was under the impression that this message was generated by an xUnit-based analyzer, but that appears to be something altogether different:

Knowing this now, I understand your reasoning here and would, in fact, approve if only I still didn't have a message that I find is in error polluting my development environment. 😆

In any case, the NoWarn works for my purposes. I do appreciate you taking the time to explain your viewpoint and efforts over there. 👍