CA1861 Avoid constant arrays as arguments

src/NetAnalyzers/Core/AnalyzerReleases.Unshipped.md

@@ -13,6 +13,7 @@ CA1857 | Performance | Warning | ConstantExpectedAnalyzer, [Documentation](https
 CA1858 | Performance | Info | UseStartsWithInsteadOfIndexOfComparisonWithZero, [Documentation](https://docs.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1858)
 CA1859 | Performance | Info | UseConcreteTypeAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1859)
 CA1860 | Performance | Info | PreferLengthCountIsEmptyOverAnyAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1860)
+CA1861 | Performance | Info | AvoidConstArrays, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1861)
 CA2021 | Reliability | Warning | DoNotCallEnumerableCastOrOfTypeWithIncompatibleTypesAnalyzer, [Documentation](https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca2021)
 
 ### Removed Rules

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/MicrosoftNetCoreAnalyzersResources.resx

@@ -213,6 +213,20 @@
   <data name="AvoidUnsealedAttributesMessage" xml:space="preserve">
     <value>Avoid unsealed attributes</value>
   </data>
+  <data name="AvoidConstArraysTitle" xml:space="preserve">
+    <value>Avoid constant arrays as arguments</value>
+  </data>
+  <data name="AvoidConstArraysCodeFixTitle" xml:space="preserve">
+    <value>Extract to static readonly field</value>
+  </data>
+  <data name="AvoidConstArraysDescription" xml:space="preserve">
+    <value>Constant arrays passed as arguments are not reused which implies a performance overhead. Consider extracting them to 'static readonly' fields to improve performance.</value>
+    <comment>{Locked="static readonly"}</comment>
+  </data>
+  <data name="AvoidConstArraysMessage" xml:space="preserve">
+    <value>Prefer 'static readonly' fields over local constant array arguments</value>
+    <comment>{Locked="static readonly"}</comment>
+  </data>
   <data name="TestForEmptyStringsUsingStringLengthTitle" xml:space="preserve">
     <value>Test for empty strings using string length</value>
   </data>
@@ -2040,4 +2054,4 @@ Widening and user defined conversions are not supported with generic types.</val
   <data name="PreferIsEmptyOverAnyMessage" xml:space="preserve">
     <value>Prefer an 'IsEmpty' check rather than using 'Any()', both for clarity and for performance</value>
   </data>
-</root>
+</root>

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArrays.Fixer.cs

@@ -0,0 +1,184 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System;
+using System.Linq;
+using System.Globalization;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using System.Composition;
+using System.Threading;
+using System.Threading.Tasks;
+using Analyzer.Utilities;
+using Analyzer.Utilities.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.CodeActions;
+using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.Editing;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.NetCore.Analyzers.Runtime
+{
+    /// <summary>
+    /// CA1861: Avoid constant arrays as arguments. Replace with static readonly arrays.
+    /// </summary>
+    [ExportCodeFixProvider(LanguageNames.CSharp, LanguageNames.VisualBasic), Shared]
+    public sealed class AvoidConstArraysFixer : CodeFixProvider
+    {
+        public sealed override ImmutableArray<string> FixableDiagnosticIds { get; } = ImmutableArray.Create(AvoidConstArraysAnalyzer.RuleId);
+
+        private static readonly ImmutableArray<string> s_collectionMemberEndings = ImmutableArray.Create("array", "collection", "enumerable", "list");
+
+        // See https://github.com/dotnet/roslyn/blob/main/docs/analyzers/FixAllProvider.md for more information on Fix All Providers
+        public sealed override FixAllProvider GetFixAllProvider() => WellKnownFixAllProviders.BatchFixer;
+
+        public override async Task RegisterCodeFixesAsync(CodeFixContext context)
+        {
+            Document document = context.Document;
+            SyntaxNode root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
+            SyntaxNode node = root.FindNode(context.Span);
+
+            context.RegisterCodeFix(CodeAction.Create(
+                    MicrosoftNetCoreAnalyzersResources.AvoidConstArraysCodeFixTitle,
+                    async ct => await ExtractConstArrayAsync(document, root, node, context.Diagnostics[0].Properties, ct).ConfigureAwait(false),
+                    equivalenceKey: nameof(MicrosoftNetCoreAnalyzersResources.AvoidConstArraysCodeFixTitle)),
+                context.Diagnostics);
+        }
+
+        private static async Task<Document> ExtractConstArrayAsync(Document document, SyntaxNode root, SyntaxNode node,
+            ImmutableDictionary<string, string?> properties, CancellationToken cancellationToken)
+        {
+            SemanticModel model = await document.GetSemanticModelAsync(cancellationToken).ConfigureAwait(false);
+            DocumentEditor editor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false);
+            SyntaxGenerator generator = editor.Generator;
+            IArrayCreationOperation arrayArgument = GetArrayCreationOperation(node, model, cancellationToken, out bool isInvoked);
+            INamedTypeSymbol containingType = model.GetEnclosingSymbol(node.SpanStart, cancellationToken).ContainingType;
+
+            // Get a valid member name for the extracted constant
+            string newMemberName = GetExtractedMemberName(containingType.MemberNames, properties["paramName"] ?? GetMemberNameFromType(arrayArgument));
+
+            // Get method containing the symbol that is being diagnosed
+            IOperation? methodContext = arrayArgument.GetAncestor<IMethodBodyOperation>(OperationKind.MethodBody);
+            methodContext ??= arrayArgument.GetAncestor<IBlockOperation>(OperationKind.Block); // VB methods have a different structure than CS methods
+
+            // Create the new member
+            SyntaxNode newMember = generator.FieldDeclaration(
+                newMemberName,
+                generator.TypeExpression(arrayArgument.Type),
+                GetAccessibility(methodContext is null ? null : model.GetEnclosingSymbol(methodContext.Syntax.SpanStart, cancellationToken)),
+                DeclarationModifiers.Static | DeclarationModifiers.ReadOnly,
+                arrayArgument.Syntax.WithoutTrailingTrivia() // don't include extra trivia before the end of the declaration
+            );
+
+            // Add any additional formatting
+            if (methodContext is not null)
+            {
+                newMember = newMember.FormatForExtraction(methodContext.Syntax);
+            }
+
+            ISymbol lastFieldOrPropertySymbol = containingType.GetMembers().LastOrDefault(x => x is IFieldSymbol or IPropertySymbol);
+            if (lastFieldOrPropertySymbol is not null)
+            {
+                var span = lastFieldOrPropertySymbol.Locations.First().SourceSpan;
+                if (root.FullSpan.Contains(span))
+                {
+                    // Insert after fields or properties
+                    SyntaxNode lastFieldOrPropertyNode = root.FindNode(span);
+                    editor.InsertAfter(generator.GetDeclaration(lastFieldOrPropertyNode), newMember);
+                }
+                else
+                {
+                    // Span not found
+                    editor.InsertBefore(methodContext?.Syntax, newMember);
+                }
+            }
+            else
+            {
+                // No fields or properties, insert right before the containing method for simplicity
+                editor.InsertBefore(methodContext?.Syntax, newMember);
+            }
+
+            // Replace argument with a reference to our new member
+            SyntaxNode identifier = generator.IdentifierName(newMemberName);
+            if (isInvoked)
+            {
+                editor.ReplaceNode(node, generator.WithExpression(identifier, node));
+            }
+            else
+            {
+                // add any extra trivia that was after the original argument
+                editor.ReplaceNode(node, generator.Argument(identifier).WithTriviaFrom(arrayArgument.Syntax));
+            }
+
+            // Return changed document
+            return editor.GetChangedDocument();
+        }
+
+        private static IArrayCreationOperation GetArrayCreationOperation(SyntaxNode node, SemanticModel model, CancellationToken cancellationToken, out bool isInvoked)
+        {
+            // The analyzer only passes a diagnostic for two scenarios, each having an IArrayCreationOperation:
+            //      1. The node is an IArgumentOperation that is a direct parent of an IArrayCreationOperation
+            //      2. The node is an IArrayCreationOperation already, as it was pulled from an
+            //         invocation, like with LINQ extension methods
+
+            // If this is a LINQ invocation, the node is already an IArrayCreationOperation
+            if (model.GetOperation(node, cancellationToken) is IArrayCreationOperation arrayCreation)
+            {
+                isInvoked = true;
+                return arrayCreation;
+            }
+            // Otherwise, we'll get the IArrayCreationOperation from the argument node's child
+            isInvoked = false;
+            return (IArrayCreationOperation)model.GetOperation(node.ChildNodes().First(), cancellationToken);
+        }
+
+        private static string GetExtractedMemberName(IEnumerable<string> memberNames, string parameterName)
+        {
+            bool hasCollectionEnding = s_collectionMemberEndings.Any(x => parameterName.EndsWith(x, true, CultureInfo.InvariantCulture));
+
+            if (parameterName == "source" // for LINQ, "sourceArray" is clearer than "source"
+                || (memberNames.Contains(parameterName) && !hasCollectionEnding))
+            {
+                parameterName += "Array";
+            }
+
+            if (memberNames.Contains(parameterName))
+            {
+                int suffix = 0;
+                while (memberNames.Contains(parameterName + suffix))
+                {
+                    suffix++;
+                }
+
+                return parameterName + suffix;
+            }
+
+            return parameterName;
+        }
+
+        private static string GetMemberNameFromType(IArrayCreationOperation arrayCreationOperation)
+        {
+#pragma warning disable CA1308 // Normalize strings to uppercase
+            return ((IArrayTypeSymbol)arrayCreationOperation.Type).ElementType.OriginalDefinition.Name.ToLowerInvariant() + "Array";
+#pragma warning restore CA1308 // Normalize strings to uppercase
+        }
+
+        private static Accessibility GetAccessibility(ISymbol? methodSymbol)
+        {
+            // If private or public, return private since public accessibility not wanted for fields by default
+            return methodSymbol?.GetResultantVisibility() switch
+            {
+                // Return internal if internal
+                SymbolVisibility.Internal => Accessibility.Internal,
+                _ => Accessibility.Private
+            };
+        }
+    }
+
+    internal static class SyntaxNodeExtensions
+    {
+        internal static SyntaxNode FormatForExtraction(this SyntaxNode node, SyntaxNode previouslyContainingNode)
+        {
+            return node.HasTrailingTrivia ? node : node.WithTrailingTrivia(previouslyContainingNode.GetTrailingTrivia());
+        }
+    }
+}

src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArrays.cs

@@ -0,0 +1,153 @@
+// Copyright (c) Microsoft.  All Rights Reserved.  Licensed under the MIT license.  See License.txt in the project root for license information.
+
+using System.Linq;
+using System.Collections.Generic;
+using System.Collections.Immutable;
+using Analyzer.Utilities;
+using Analyzer.Utilities.Extensions;
+using Microsoft.CodeAnalysis;
+using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Operations;
+
+namespace Microsoft.NetCore.Analyzers.Runtime
+{
+    using static MicrosoftNetCoreAnalyzersResources;
+
+    /// <summary>
+    /// CA1861: Avoid constant arrays as arguments. Replace with static readonly arrays.
+    /// </summary>
+    [DiagnosticAnalyzer(LanguageNames.CSharp, LanguageNames.VisualBasic)]
+    public sealed class AvoidConstArraysAnalyzer : DiagnosticAnalyzer
+    {
+        internal const string RuleId = "CA1861";
+
+        internal static readonly DiagnosticDescriptor Rule = DiagnosticDescriptorHelper.Create(RuleId,
+            CreateLocalizableResourceString(nameof(AvoidConstArraysTitle)),
+            CreateLocalizableResourceString(nameof(AvoidConstArraysMessage)),
+            DiagnosticCategory.Performance,
+            RuleLevel.IdeSuggestion,
+            CreateLocalizableResourceString(nameof(AvoidConstArraysDescription)),
+            isPortedFxCopRule: false,
+            isDataflowRule: false);
+
+        public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule);
+
+        public override void Initialize(AnalysisContext context)
+        {
+            context.EnableConcurrentExecution();
+            context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None);
+
+            context.RegisterCompilationStartAction(context =>
+            {
+                INamedTypeSymbol? readonlySpanType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemReadOnlySpan1);
+                INamedTypeSymbol? functionType = context.Compilation.GetOrCreateTypeByMetadataName(WellKnownTypeNames.SystemFunc2);
+
+                // Analyzes an argument operation
+                context.RegisterOperationAction(context =>
+                {
+                    IArgumentOperation? argumentOperation;
+
+                    if (context.ContainingSymbol is IMethodSymbol method && method.MethodKind == MethodKind.StaticConstructor)
+                    {
+                        return;
+                    }
+
+                    if (context.Operation is IArrayCreationOperation arrayCreationOperation) // For arrays passed as arguments
+                    {
+                        argumentOperation = arrayCreationOperation.GetAncestor<IArgumentOperation>(OperationKind.Argument);
+
+                        // If no argument, return
+                        // If argument is passed as a params array but isn't itself an array, return
+                        if (argumentOperation is null || (argumentOperation.Parameter.IsParams && arrayCreationOperation.IsImplicit))
+                        {
+                            return;
+                        }
+                    }
+                    else if (context.Operation is IInvocationOperation invocationOperation) // For arrays passed in extension methods, like in LINQ
+                    {
+                        IEnumerable<IOperation> invocationDescendants = invocationOperation.Descendants();
+                        if (invocationDescendants.Any(x => x is IArrayCreationOperation)
+                            && invocationDescendants.Any(x => x is IArgumentOperation))
+                        {
+                            // This is an invocation that contains an array as an argument
+                            // This will get caught by the first case in another cycle
+                            return;
+                        }
+
+                        argumentOperation = invocationOperation.Arguments.FirstOrDefault();
+                        if (argumentOperation is not null)
+                        {
+                            if (argumentOperation.Value is not IConversionOperation conversionOperation
+                                || conversionOperation.Operand is not IArrayCreationOperation arrayCreation)
+                            {
+                                return;
+                            }
+
+                            arrayCreationOperation = arrayCreation;
+                        }
+                        else // An invocation, extension or regular, has an argument, unless it's a VB extension method call
+                        {
+                            // For VB extension method invocations, find a matching child
+                            arrayCreationOperation = (IArrayCreationOperation)invocationDescendants
+                                .FirstOrDefault(x => x is IArrayCreationOperation);
+                            if (arrayCreationOperation is null)
+                            {
+                                return;
+                            }
+                        }
+                    }
+                    else
+                    {
+                        return;
+                    }
+
+                    // Must be literal array
+                    if (arrayCreationOperation.Initializer.ElementValues.Any(x => x is not ILiteralOperation))
+                    {
+                        return;
+                    }
+
+                    string? paramName = null;
+                    if (argumentOperation is not null)
+                    {
+                        IFieldInitializerOperation? fieldInitializer = argumentOperation.GetAncestor<IFieldInitializerOperation>(
+                            OperationKind.FieldInitializer, f => f.InitializedFields.Any(x => x.IsReadOnly));
+                        IPropertyInitializerOperation? propertyInitializer = argumentOperation.GetAncestor<IPropertyInitializerOperation>(
+                            OperationKind.PropertyInitializer, p => p.InitializedProperties.Any(x => x.IsReadOnly));
+
+                        if (fieldInitializer is not null || propertyInitializer is not null)
+                        {
+                            return;
+                        }
+
+                        ITypeSymbol originalDefinition = argumentOperation.Parameter.Type.OriginalDefinition;
+
+                        // Can't be a ReadOnlySpan, as those are already optimized
+                        if (SymbolEqualityComparer.Default.Equals(readonlySpanType, originalDefinition))
+                        {
+                            return;
+                        }
+
+                        // Check if the parameter is a function so the name can be set to null
+                        // Otherwise, the parameter name doesn't reflect the array creation as well
+                        bool isDirectlyInsideLambda = originalDefinition.Equals(functionType);
+
+                        // Parameter shouldn't have same containing type as the context, to prevent naming ambiguity
+                        // Ignore parameter name if we're inside a lambda function
+                        if (!isDirectlyInsideLambda && !argumentOperation.Parameter.ContainingType.Equals(context.ContainingSymbol.ContainingType))
+                        {
+                            paramName = argumentOperation.Parameter.Name;
+                        }
+                    }
+
+                    ImmutableDictionary<string, string?>.Builder properties = ImmutableDictionary.CreateBuilder<string, string?>();
+                    properties.Add("paramName", paramName);
+
+                    context.ReportDiagnostic(arrayCreationOperation.CreateDiagnostic(Rule, properties.ToImmutable()));
+                },
+                OperationKind.ArrayCreation,
+                OperationKind.Invocation);
+            });
+        }
+    }
+}