"Avoid unnecessary zero-length array allocations" fires on params overloads usage

[davkean]

Diagnostic ID

CA1825

Repro steps

      Dim NewInstanceExpression As New CodeObjectCreateExpression(SettingsClassTypeReference)

CodeObjectCreateExpression has a constructor taking a params array: https://docs.microsoft.com/en-us/dotnet/api/system.codedom.codeobjectcreateexpression.-ctor?view=netframework-4.7.2#System_CodeDom_CodeObjectCreateExpression__ctor_System_String_System_CodeDom_CodeExpression___.

Expected behavior

Not sure what the intention is here, are you firing on C# usage of params array?

Actual behavior

Severity    Code    Description Project Project Rank    Path    File    Line    Column  Category    Source  Suppression State   Tool
Warning CA1825  Avoid unnecessary zero-length array allocations.  Use Array.Empty(Of CodeExpression)() instead. Microsoft.VisualStudio.Editors  3   E:\project-system2\src\Microsoft.VisualStudio.Editors\SettingsDesigner  E:\project-system2\src\Microsoft.VisualStudio.Editors\SettingsDesigner\SettingsSingleFileGeneratorBase.vb   377 46  Performance IntelliSense    Active  Microsoft.NetCore.VisualBasic.Analyzers

[davkean]

This looks fixed, or I can no longer repro.

[davkean]

My mistake this still occurs, apparently the rules were moved underneath me to Microsoft.NetCore.Analyzers.

[Youssef1313]

I very much think this is by-design and should occur in VB but not in C#.

---

VB

Public Class C
    Public Sub M(ParamArray x As String())
        M()
    End Sub
End Class

IL for M:

    .method public 
        instance void M (
            string[] x
        ) cil managed 
    {
        .param [1]
            .custom instance void [System.Runtime]System.ParamArrayAttribute::.ctor() = (
                01 00 00 00
            )
        // Method begins at RVA 0x2058
        // Code size 15 (0xf)
        .maxstack 8

        IL_0000: nop
        IL_0001: ldarg.0
        IL_0002: ldc.i4.0
        IL_0003: newarr [System.Runtime]System.String
        IL_0008: call instance void C::M(string[])
        IL_000d: nop
        IL_000e: ret
    } // end of method C::M

---

C\

public class C
{
    public void M(params string[] x)
    {
        M();
    }
}

IL for M:

    .method public hidebysig 
        instance void M (
            string[] x
        ) cil managed 
    {
        .param [1]
            .custom instance void [System.Runtime]System.ParamArrayAttribute::.ctor() = (
                01 00 00 00
            )
        // Method begins at RVA 0x2069
        // Code size 14 (0xe)
        .maxstack 8

        IL_0000: nop
        IL_0001: ldarg.0
        IL_0002: call !!0[] [System.Runtime]System.Array::Empty<string>()
        IL_0007: call instance void C::M(string[])
        IL_000c: nop
        IL_000d: ret
    } // end of method C::M

[Youssef1313]

Not sure what the intention is here, are you firing on C# usage of params array?

The point here is to suggest Array.Empty<T> instead of allocating a new empty array.

for C#, omitting the params will pass Array.Empty<T>, so no warning is fired. for VB, the compiler emits newarr, and this is the "unnecessary" allocation the analyzer wants to avoid.

[Youssef1313]

It looks like https://github.com/dotnet/roslyn-analyzers/pull/905 has already changed the behavior to not produce a warning for compiler-generated array creations.

I think there is still a value in knowing there is unnecessary array creation.

@mavasani Can you please confirm the expected outcome here?