GenAPI generates incorrect nullable annotations for explicit interface implementations

[safern]

• Is this issue blocking no
• Is this issue causing unreasonable pain no

Given an interface:

public interface IFoo {
    object? A(object? sender);
}

and an explicit implementation of this interface:

public class FooImpl : IFoo {
    object? IFoo.A(object? sender);
}

and when the assembly omits private members nullable metadata, so when the [module: NullablePublicOnly(true)] is present on the assembly, the compiler will not emit any nullable metadata for object? IFoo.A(object? sender);, so GenAPI will emit the wrong nullability for return type and parameters, it will emit it based on the closest NullableContextAttribute it finds which is wrong.

What we're missing, is that whenever it is an explicit interface implementation, we should be looking at the implemented member declaration and match that definition. We should be getting the implemented member and mapping the parameter nullability and return type nullability to the original declaration, rather than the member implementation itself. We're missing to do that here for the return type: https://github.com/dotnet/arcade/blob/14abaee3dba41fbe608431fb6a4f4b2435dcac33/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Methods.cs#L146

and here for the parameters: https://github.com/dotnet/arcade/blob/14abaee3dba41fbe608431fb6a4f4b2435dcac33/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Methods.cs#L165

If it is an explicit implementation we already resolve the implemented interface and the nullability we're implementing it with for generics, so that the signature matches and code is compilable: https://github.com/dotnet/arcade/blob/14abaee3dba41fbe608431fb6a4f4b2435dcac33/src/Microsoft.Cci.Extensions/Writers/CSharp/CSDeclarationWriter.Methods.cs#L111

cc: @ericstj @eiriktsarpalis @stephentoub

[eiriktsarpalis]

I'm assuming this would require reading all assembly dependencies as well, which presumably wasn't needed until today?

[ericstj]

No, we pass in dependencies and resolve them already.

[eerhardt]

FYI - we hit this issue here:

https://github.com/dotnet/runtime/blob/6ce761095b57ed38242c8140bc4f1d6da7251567/src/libraries/System.Security.Principal.Windows/ref/System.Security.Principal.Windows.cs#L268

[safern]

Related: https://github.com/dotnet/arcade/issues/6016