dotnet / arcade

Tools that provide common build infrastructure for multiple .NET Foundation projects.
MIT License
673 stars 347 forks source link

GenAPI puts nullable method attribute in wrong location: `public static [NotNullIf...]T As<T>(...)`, error CS1031 #6718

Open dagood opened 3 years ago

dagood commented 3 years ago

GenAPI 1.0.0-beta.18619.2

In https://www.nuget.org/packages/System.Runtime.CompilerServices.Unsafe/5.0.0 ref/netstandard2.1/System.Runtime.CompilerServices.Unsafe.dll we have this method (output from ILSpy):

using System.Diagnostics.CodeAnalysis;

[return: NotNullIfNotNull("o")]
public static T? As<T>(object? o) where T : class?
{
    throw null;
}
.method public hidebysig static 
    !!T As<class T> (
        object o
    ) cil managed 
{
    .custom instance void System.Runtime.CompilerServices.NullableContextAttribute::.ctor(uint8) = (
        01 00 02 00 00
    )
    .param [0]
        .custom instance void [netstandard]System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute::.ctor(string) = (
            01 00 01 6f 00 00
        )
    // Method begins at RVA 0x20b5
    // Code size 2 (0x2)
    .maxstack 8

    IL_0000: ldnull
    IL_0001: throw
} // end of method Unsafe::As

But GenAPI produced this source code:

public static [System.Diagnostics.CodeAnalysis.NotNullIfNotNullAttribute("o")]T As<T>(object o) where T : class { throw null; }

Which results in this compile error:

...ref/netstandard2.1/System.Runtime.CompilerServices.Unsafe.cs(44,23): error CS1031: Type expected
dagood commented 3 years ago

I suppose it's also incorrectly producing [NotNullIfNotNullAttribute(...)] rather than [return: NotNullIfNotNullAttribute(...)]. Wasn't aware of this syntax before today.

safern commented 3 years ago

Yeah this is wrong. However, we're going to be moving GenAPI to use Roslyn instead of CCI and that will fix these kind of issues. Here is a small POC: https://github.com/safern/roslyn-genapi

dagood commented 3 years ago

Wonderful. Can also then build it from source. 😄

MichaelSimons commented 2 years ago

This appears fixed in Microsoft.DotNet.GenAPI, 7.0.0-beta.22166.1.