Fix 'make struct member readonly' with init accessors.

src/Analyzers/CSharp/Analyzers/MakeStructMemberReadOnly/CSharpMakeStructMemberReadOnlyAnalyzer.cs

@@ -2,23 +2,27 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections.Generic;
 using System.Collections.Immutable;
 using System.Diagnostics.CodeAnalysis;
+using System.Linq;
 using System.Threading;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.CodeStyle;
 using Microsoft.CodeAnalysis.CSharp.Extensions;
 using Microsoft.CodeAnalysis.CSharp.Syntax;
 using Microsoft.CodeAnalysis.Diagnostics;
 using Microsoft.CodeAnalysis.Operations;
+using Microsoft.CodeAnalysis.PooledObjects;
 using Microsoft.CodeAnalysis.Shared.Extensions;
 using Roslyn.Utilities;
 
 namespace Microsoft.CodeAnalysis.CSharp.MakeStructMemberReadOnly;
 
 [DiagnosticAnalyzer(LanguageNames.CSharp)]
 internal sealed class CSharpMakeStructMemberReadOnlyDiagnosticAnalyzer()
-    : AbstractBuiltInCodeStyleDiagnosticAnalyzer(IDEDiagnosticIds.MakeStructMemberReadOnlyDiagnosticId,
+    : AbstractBuiltInCodeStyleDiagnosticAnalyzer(
+        IDEDiagnosticIds.MakeStructMemberReadOnlyDiagnosticId,
         EnforceOnBuildValues.MakeStructMemberReadOnly,
         CSharpCodeStyleOptions.PreferReadOnlyStructMember,
         new LocalizableResourceString(nameof(CSharpAnalyzersResources.Make_member_readonly), CSharpAnalyzersResources.ResourceManager, typeof(CSharpAnalyzersResources)),
@@ -39,8 +43,13 @@ protected override void InitializeWorker(AnalysisContext context)
                 if (!ShouldAnalyze(context, out var option))
                     return;
 
+                var methodToDiagnostic = PooledDictionary<IMethodSymbol, Diagnostic>.GetInstance();
+
                 context.RegisterOperationBlockAction(
-                    context => AnalyzeBlock(context, option.Notification.Severity));
+                    context => AnalyzeBlock(context, option.Notification.Severity, methodToDiagnostic));
+
+                context.RegisterSymbolEndAction(
+                    context => ProcessResults(context, option.Notification.Severity, methodToDiagnostic));
             }, SymbolKind.NamedType);
 
             static bool ShouldAnalyze(SymbolStartAnalysisContext context, [NotNullWhen(true)] out CodeStyleOption2<bool>? option)
@@ -88,11 +97,42 @@ static bool ShouldAnalyze(SymbolStartAnalysisContext context, [NotNullWhen(true)
 
                 return true;
             }
+
+            void ProcessResults(
+                SymbolAnalysisContext context, ReportDiagnostic severity, PooledDictionary<IMethodSymbol, Diagnostic> methodToDiagnostic)
+            {
+                var cancellationToken = context.CancellationToken;
+
+                // No need to lock the dictionary here.  Processing only is called once, after all mutation work is done.
+                foreach (var (method, diagnostic) in methodToDiagnostic)
+                {
+                    if (method.IsInitOnly && method.AssociatedSymbol is IPropertySymbol owningProperty)
+                    {
+                        // Iff we have an init method that we want to mark as readonly, we can only do so if there is no
+                        // `get` accessor, or if the `get` method is already `readonly` or would determined we want to
+                        // mark as `readonly`.
+                        var getMethodIsReadOnly =
+                            owningProperty.GetMethod is null ||
+                            owningProperty.GetMethod.IsReadOnly ||
+                            methodToDiagnostic.ContainsKey(owningProperty.GetMethod);
+
+                        // Skip marking this property as readonly for this init method if it would conflict with the get method.
+                        if (!getMethodIsReadOnly)
+                            continue;
+                    }
+
+                    // normal case
+                    context.ReportDiagnostic(diagnostic);
+                }
+
+                methodToDiagnostic.Free();
+            }
         });
 
     private void AnalyzeBlock(
         OperationBlockAnalysisContext context,
-        ReportDiagnostic severity)
+        ReportDiagnostic severity,
+        Dictionary<IMethodSymbol, Diagnostic> methodToDiagnostic)
     {
         var cancellationToken = context.CancellationToken;
 
@@ -115,12 +155,16 @@ private void AnalyzeBlock(
                 return;
         }
 
-        context.ReportDiagnostic(DiagnosticHelper.Create(
-            Descriptor,
-            location,
-            severity,
-            additionalLocations: ImmutableArray.Create(additionalLocation),
-            properties: null));
+        // Called concurrently.  Make sure we write to this dictionary safely.
+        lock (methodToDiagnostic)
+        {
+            methodToDiagnostic[owningMethod] = DiagnosticHelper.Create(
+                Descriptor,
+                location,
+                severity,
+                additionalLocations: ImmutableArray.Create(additionalLocation),
+                properties: null);
+        }
     }
 
     private static (Location? location, Location? additionalLocation) GetDiagnosticLocation(
@@ -133,7 +177,17 @@ private static (Location? location, Location? additionalLocation) GetDiagnosticL
             || owningMethod.IsStatic
             || owningMethod.IsImplicitlyDeclared)
         {
-            return (null, null);
+            return default;
+        }
+
+        // An init accessor in a readonly property is already readonly.  No need to analyze it.  Note: there is no way
+        // to tell this symbolically.  We have to check to the syntax here.
+        if (owningMethod.IsInitOnly &&
+            owningMethod.AssociatedSymbol is IPropertySymbol { DeclaringSyntaxReferences: [var reference, ..] } &&
+            reference.GetSyntax(cancellationToken) is PropertyDeclarationSyntax property &&
+            property.Modifiers.Any(SyntaxKind.ReadOnlyKeyword))
+        {
+            return default;
         }
 
         var methodReference = owningMethod.DeclaringSyntaxReferences[0];
@@ -153,7 +207,7 @@ private static (Location? location, Location? additionalLocation) GetDiagnosticL
             declaration = declaration.GetRequiredParent();
 
         if (nameToken is null)
-            return (null, null);
+            return default;
 
         return (nameToken.Value.GetLocation(), declaration.GetLocation());
     }

src/Analyzers/CSharp/CodeFixes/MakeStructMemberReadOnly/CSharpMakeStructMemberReadOnlyCodeFixProvider.cs

@@ -20,14 +20,10 @@
 namespace Microsoft.CodeAnalysis.CSharp.MakeStructMemberReadOnly;
 
 [ExportCodeFixProvider(LanguageNames.CSharp, Name = PredefinedCodeFixProviderNames.MakeStructMemberReadOnly), Shared]
-internal sealed class CSharpMakeStructMemberReadOnlyCodeFixProvider : SyntaxEditorBasedCodeFixProvider
+[method: ImportingConstructor]
+[method: Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
+internal sealed class CSharpMakeStructMemberReadOnlyCodeFixProvider() : SyntaxEditorBasedCodeFixProvider
 {
-    [ImportingConstructor]
-    [Obsolete(MefConstruction.ImportingConstructorMessage, error: true)]
-    public CSharpMakeStructMemberReadOnlyCodeFixProvider()
-    {
-    }
-
     public override ImmutableArray<string> FixableDiagnosticIds { get; } =
         ImmutableArray.Create(IDEDiagnosticIds.MakeStructMemberReadOnlyDiagnosticId);
 

src/Analyzers/CSharp/Tests/MakeStructMemberReadOnly/MakeStructMemberReadOnlyTests.cs

@@ -2106,4 +2106,143 @@ public void RemoveBit(int candidate)
             LanguageVersion = LanguageVersion.CSharp12,
         }.RunAsync();
     }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70864")]
+    public async Task TestInitAccessor1()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                public record struct TypeMapCapacity
+                {
+                    private readonly int _MaxSize;
+
+                    public readonly int? MaxSize
+                    {
+                        get => _MaxSize;
+
+                        // missing, since the property is already readonly
+                        init
+                        {
+                        }
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70864")]
+    public async Task TestInitAccessor2()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                public record struct TypeMapCapacity
+                {
+                    private readonly int _MaxSize;
+
+                    public int? MaxSize
+                    {
+                        readonly get => _MaxSize;
+
+                        [|init|]
+                        {
+                        }
+                    }
+                }
+                """,
+            FixedCode = """
+                using System;
+
+                public record struct TypeMapCapacity
+                {
+                    private readonly int _MaxSize;
+
+                    public readonly int? MaxSize
+                    {
+                        get => _MaxSize;
+
+                        init
+                        {
+                        }
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70864")]
+    public async Task TestInitAccessor3()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                public record struct TypeMapCapacity
+                {
+                    private int _MaxSize;
+
+                    public int? MaxSize
+                    {
+                        get => _MaxSize++;
+
+                        init
+                        {
+                        }
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/70864")]
+    public async Task TestInitAccessor4()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                public record struct TypeMapCapacity
+                {
+                    private readonly int _MaxSize;
+
+                    public int? MaxSize
+                    {
+                        [|init|]
+                        {
+                        }
+                    }
+                }
+                """,
+            FixedCode = """
+                using System;
+
+                public record struct TypeMapCapacity
+                {
+                    private readonly int _MaxSize;
+
+                    public readonly int? MaxSize
+                    {
+                        init
+                        {
+                        }
+                    }
+                }
+                """,
+            LanguageVersion = LanguageVersion.CSharp12,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net70,
+        }.RunAsync();
+    }
 }