Fix issues in fixer, do not warn for static constructor and readonly …

…properties
dotnet · May 5, 2023 · fc8992d · fc8992d
1 parent fec9a2f
commit fc8992d
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 18 deletions.
diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArrays.Fixer.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArrays.Fixer.cs
@@ -37,20 +37,20 @@ public override async Task RegisterCodeFixesAsync(CodeFixContext context)
             SyntaxNode root = await document.GetSyntaxRootAsync(context.CancellationToken).ConfigureAwait(false);
             SyntaxNode node = root.FindNode(context.Span);
             SemanticModel model = await document.GetSemanticModelAsync(context.CancellationToken).ConfigureAwait(false);
-            DocumentEditor editor = await DocumentEditor.CreateAsync(document, context.CancellationToken).ConfigureAwait(false);
 
             string title = MicrosoftNetCoreAnalyzersResources.AvoidConstArraysCodeFixTitle;
             context.RegisterCodeFix(CodeAction.Create(
                     title,
-                    async ct => await ExtractConstArrayAsync(root, node, model, editor, editor.Generator,
-                        context.Diagnostics.First().Properties, ct).ConfigureAwait(false),
+                    async ct => await ExtractConstArrayAsync(document, root, node, model, context.Diagnostics.First().Properties, ct).ConfigureAwait(false),
                     equivalenceKey: title),
                 context.Diagnostics);
         }
 
-        private static Task<Document> ExtractConstArrayAsync(SyntaxNode root, SyntaxNode node, SemanticModel model, DocumentEditor editor,
-            SyntaxGenerator generator, ImmutableDictionary<string, string?> properties, CancellationToken cancellationToken)
+        private static async Task<Document> ExtractConstArrayAsync(Document document, SyntaxNode root, SyntaxNode node,
+            SemanticModel model, ImmutableDictionary<string, string?> properties, CancellationToken cancellationToken)
         {
+            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;
 
@@ -76,12 +76,21 @@ private static Task<Document> ExtractConstArrayAsync(SyntaxNode root, SyntaxNode
                 newMember = newMember.FormatForExtraction(methodContext.Syntax);
             }
 
-            ISymbol lastFieldOrPropertSymbol = containingType.GetMembers().LastOrDefault(x => x is IFieldSymbol or IPropertySymbol);
-            if (lastFieldOrPropertSymbol is not null)
+            ISymbol lastFieldOrPropertySymbol = containingType.GetMembers().LastOrDefault(x => x is IFieldSymbol or IPropertySymbol);
+            if (lastFieldOrPropertySymbol is not null)
             {
-                // Insert after fields or properties
-                SyntaxNode lastFieldOrPropertyNode = root.FindNode(lastFieldOrPropertSymbol.Locations.First().SourceSpan);
-                editor.InsertAfter(generator.GetDeclaration(lastFieldOrPropertyNode), newMember);
+                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
             {
@@ -102,7 +111,7 @@ private static Task<Document> ExtractConstArrayAsync(SyntaxNode root, SyntaxNode
             }
 
             // Return changed document
-            return Task.FromResult(editor.GetChangedDocument());
+            return editor.GetChangedDocument();
         }
 
         private static IArrayCreationOperation GetArrayCreationOperation(SyntaxNode node, SemanticModel model, CancellationToken cancellationToken, out bool isInvoked)

diff --git a/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArrays.cs b/src/NetAnalyzers/Core/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArrays.cs
@@ -47,6 +47,11 @@ public override void Initialize(AnalysisContext 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);
@@ -105,8 +110,12 @@ public override void Initialize(AnalysisContext context)
                     string? paramName = null;
                     if (argumentOperation is not null)
                     {
-                        IFieldInitializerOperation? fieldInitializer = argumentOperation.GetAncestor<IFieldInitializerOperation>(OperationKind.FieldInitializer);
-                        if (fieldInitializer is not null && fieldInitializer.InitializedFields.Any(x => x.IsReadOnly))
+                        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;
                         }
@@ -131,12 +140,10 @@ public override void Initialize(AnalysisContext context)
                         }
                     }
 
-                    Dictionary<string, string?> properties = new()
-                    {
-                        { "paramName", paramName }
-                    };
+                    ImmutableDictionary<string, string?>.Builder properties = ImmutableDictionary.CreateBuilder<string, string?>();
+                    properties.Add("paramName", paramName);
 
-                    context.ReportDiagnostic(arrayCreationOperation.CreateDiagnostic(Rule, properties.ToImmutableDictionary()));
+                    context.ReportDiagnostic(arrayCreationOperation.CreateDiagnostic(Rule, properties.ToImmutable()));
                 },
                 OperationKind.ArrayCreation,
                 OperationKind.Invocation);

diff --git a/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArraysTests.cs b/src/NetAnalyzers/UnitTests/Microsoft.NetCore.Analyzers/Runtime/AvoidConstArraysTests.cs
@@ -718,5 +718,28 @@ public B(string[] s)
 }
 ");
         }
+
+        [Fact]
+        public async Task IgnoreReadOnlyProperties_NoDiagnostic()
+        {
+            await VerifyCS.VerifyAnalyzerAsync(@"
+using System;
+using System.Collections.Generic;
+
+public class A
+{
+    public static readonly A Field; 
+    public static List<string> Property { get; } = GetValues(new string[] { ""close"" });
+    public static string[] Property2 { get; } = new string[] { ""close"" };
+
+    static A() // Exclude initialization in static constructors
+    {
+        Property = GetValues(new string[] { ""close"" });
+        Field = new A(new string[] { ""close"" });
+    }
+    public A(string[] arr) { }
+    private static List<string> GetValues(string[] arr) => null;
+}");
+        }
     }
 }