RoslynClrHeapAllocationAnalyzer icon indicating copy to clipboard operation
RoslynClrHeapAllocationAnalyzer copied to clipboard

Add support for VB.NET

Open zspitz opened this issue 6 years ago • 5 comments

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) {
    }
}

zspitz avatar Nov 15 '18 11:11 zspitz

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.

mjsabby avatar Nov 25 '18 23:11 mjsabby

@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.)

zspitz avatar Nov 29 '18 21:11 zspitz

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.

mjsabby avatar Dec 04 '18 12:12 mjsabby

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

ElektroStudios avatar Oct 18 '19 11:10 ElektroStudios

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.

Youssef1313 avatar Oct 15 '20 21:10 Youssef1313