GenAPI generates incorrect class declarations for VB authored classes with extension methods

[mmitche]

Describe the Bug

Extension methods must exist in static partial classes. GenAPI instead generates sealed partial classes and includes a constructor which causes a compile time error.

Steps to Reproduce

1. Generate Microsoft.CodeAnalysis.VisualBasic @ 4.0.1.
2. Compile

Will see things like:

public sealed partial class VisualBasicExtensions
{
  internal VisualBasicExtensions() { }
  public static bool Any(this SyntaxTokenList list, VisualBasic.SyntaxKind kind) { throw null; }
  public static bool Any(this SyntaxTriviaList list, VisualBasic.SyntaxKind kind) { throw null; }
...

Should actually be:

public static partial class VisualBasicExtensions
{
  public static bool Any(this SyntaxTokenList list, VisualBasic.SyntaxKind kind) { throw null; }
  public static bool Any(this SyntaxTriviaList list, VisualBasic.SyntaxKind kind) { throw null; }
  ...

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[MichaelSimons]

FYI - @ViktorHofer

[ViktorHofer]

@ericstj were you aware of continued issues with the emitted constructors?

No idea yet what causes the class to have a sealed keyword instead of static.

[ericstj]

Looks like this is a VB authored library. I had a quick look at the metadata and VisualBasicExtensions is not a static class. https://github.com/dotnet/roslyn/blob/59ceeedbbd7ffbbd5893a5c23787460c22d0a9dd/src/Compilers/VisualBasic/Portable/VisualBasicExtensions.vb#LL20C19-L20C40

It's Public Module.

In the metadata this is represented as

.class public auto ansi sealed Microsoft.CodeAnalysis.VisualBasicExtensions
	extends [System.Runtime]System.Object
{
	.custom instance void Microsoft.VisualBasic.CompilerServices.StandardModuleAttribute::.ctor() = (
		01 00 00 00
	)

Compare that to a C# static class which is

.class public auto ansi abstract sealed beforefieldinit C
    extends [mscorlib]System.Object
{

We're doing something somewhat unusual which is to represent a VB library with generated C# -- we could approximate but there might be some noticeable differences when consuming it from VB. Hypothetically those should be minimal but we'd probably want to consult with a VB expert. FWIW, it looks like the old CCI-based GenAPI had the same issue. It will also omit static when generating this type.

[mmitche]

@ericstj in theory VB should be tolerant to the C# form, otherwise it would not be able to consume extension methods in any C# authored library?

[ericstj]

Possibly. I don’t think we are merely talking about different forms of the same thing. These are different things.

It’s possible there is some fidelity lost for a VB consumer if we produce something different. It’s also possible that whatever we produce doesn’t bind in the same way. We would need to test those scenarios.

I do see cases in dotnet/runtime where we have vb modules represented in C# just no extension methods I guess.
https://github.com/dotnet/runtime/blob/054eb4aa9a836bde419c4bbcf0f66abd51bc391a/src/libraries/Microsoft.VisualBasic.Core/ref/Microsoft.VisualBasic.Core.cs#L741

[ericstj]

I chatted with @cston about using static class with StandardModule attribute in C# to represent VB's Module and we think it might work. @AlekseyTs @jaredpar can you think of any place where this would behave differently, or prevent us from representing something that was possible to represent in VB?