Skip to content
This repository has been archived by the owner on Oct 12, 2022. It is now read-only.

Add support for VB.NET #62

Open
zspitz opened this issue Nov 15, 2018 · 5 comments
Open

Add support for VB.NET #62

zspitz opened this issue Nov 15, 2018 · 5 comments

Comments

@zspitz
Copy link

zspitz commented Nov 15, 2018

Using version 2.0 of the VS extension.

Creating a new VB.NET Windows Desktop project, and typing the following code:

Module Module1
    Sub ParamsMethod(ParamArray a As String())
        ParamsMethod("This is a test")
    End Sub
    Sub Main()
    End Sub
End Module

doesn't raise any warning about implicit allocations, as the following C# code would:

class Program {
    static void ParamsMethod(params string[] a) {
        ParamsMethod("This is a test");
    }
    static void Main(string[] args) {
    }
}
@mjsabby
Copy link
Contributor

mjsabby commented Nov 25, 2018

It will be some work to do this and the community is free to implement this looking over the C# implementation as a reference. I personally don't have VB.NET experience.

@zspitz
Copy link
Author

zspitz commented Nov 29, 2018

@mjsabby Given that the logic in the individual analyzers is very much tied to the classes in Microsoft.CodeAnalysis.CSharp.Syntax:

// https://github.com/Microsoft/RoslynClrHeapAllocationAnalyzer/blob/master/ClrHeapAllocationsAnalyzer/DisplayClassAllocationAnalyzer.cs#L42
if (node is SimpleLambdaExpressionSyntax lambdaExpr)
{
    ...
}

if (node is ParenthesizedLambdaExpressionSyntax parenLambdaExpr)
{
    ...
}  

I don't suppose there is any benefit in extracting the common logic to a base class and having each language-specific analyzer inherit from the corresponding base class.

abstract class DisplayClassAllocationAnalyzerBase { }
class DisplayClassAllocationAnalyzerCS : DisplayClassAllocationAnalyzerBase { }
class DisplayClassAllocationAnalyzerVB : DisplayClassAllocationAnalyzerBase { }

Moreover, this might not be possible, because there often isn't a one-to-one correspondence between syntax nodes: e.g. C# has SimpleLambdaExpressionSyntax and ParenthesizedLambdaExpressionSyntax, while VB.NET has SingleLineLambdaExpressionSyntax and MultiLineLambdaExpressionSyntax; neither language can directly map to the other language's corresponding syntax.

Even the base class for all the analyzers cannot be used without modification for VB.NET, as it defines the Expression property as returning an array of C# SyntaxKind.

Which now leads to the next question: once the VB.NET analyzer is in a separate project, should it be written in C# or VB.NET? If in VB.NET, there would be a smaller pool of potential maintainers, but OTOH I imagine someone unfamiliar with VB.NET would be uninterested in contributing to a VB.NET analyzer in any case.

Also, since VB.NET doesn't yet support pattern matching, the code would be somewhat less elegant. (Not sure if that is a consideration, but just putting it out there.)

@mjsabby
Copy link
Contributor

mjsabby commented Dec 4, 2018

I think there is no reason you can't write it in C#, in fact I looked at it, and all though it would be hard to come up with unit tests :) but using code from C# implementation, it may in fact be easier to write it in C#. In any case, should you choose to write it in VB.NET I have no problems in it residing here since it will beneficial to a broader audience.

@ElektroStudios
Copy link

Hi. I simply join the request to add support for VB.NET. Thanks for share.

@Youssef1313
Copy link
Contributor

Why not try to re-write the existing analyzers as operation analyzers? That has many benefits:

  • More performant.
  • Less error-prone.
  • Gives VB for free (one analyzer implementation for both languages).

Probably not all analyzers can be rewritten as IOperation analyzers, but some of them can.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants