Some existing 3rd party Roslyn analyzers are broken by C# 8 NRTs

[CornedBee]

Version Used:

VS preview 2.2, .Net preview 3.0.100-preview-010184, C# 8 with nullable reference types enabled.

Steps to Reproduce:

1. Use xUnit with Roslyn Analyzer integration
2. Add a test class like this, which uses the MemberData attribute:

    public class UnitTest1
    {
        public static readonly object[][] Data =
        {
            new object[] { "1" },
            new object[] { "2" },
        };

        [Theory, MemberData(nameof(Data))]
        public void Test1(string value)
        {
            Assert.NotNull(value);
        }
    }

3. Enable nullable reference types.

Expected Behavior:

There are no errors or warnings.

Actual Behavior:

If you add #nullable enable at the top, you get the following error:

error xUnit1019: MemberData must reference a data type assignable to 'System.Collections.Generic.IEnumerable<object[]>'. The referenced type 'object[][]' is not valid.

I reported this to the xUnit project here: xunit/xunit#1897

They asked me to report this to Roslyn instead. Basically, this appears to be a breaking API change for Roslyn analyzers.

[mavasani]

Nothing related to analyzers here, it seems like xUnit analyzers are based on some compiler APIs which have undergone a breaking API change in Microsoft.CodeAnalysis package (preview package based on xunit/xunit#1897?) , and the xUnit analyzer authors don't want to change their analyzers based on this breaking API change, unless they know it is by design and not just a break in preview packages.

Tagging @333fred @jaredpar who might want to follow up with xUnit analyzer authors to identify which API has undergone a change that has broken their existing analyzer packages.

[jasonmalinowski]

Tagging @333fred and @jcouv to think about this a bit further: I'm guessing the problem here 'object' elements is being treated as non-null, which is tripping up the analyzer because the nested generics aren't being handled correctly. Even if we changed the API to somehow avoid the break (I'm not sure we can in any of our API designs that doesn't break other analyzers), there's still may be work that the analyzer would have to do if you wanted to catch something like:

[Theory]
[InlineData("hello", null]
void Theory(string s, string s2)
{
}

where the Data() method is returning a string? but the parameter isn't. There's some special handling around conversion classification it does which is pretty tricky to sort out.