Update 'make field readonly' to understand a special strict behaviour around generic assignment.

src/Analyzers/CSharp/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.cs

@@ -2,8 +2,10 @@
 // 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.Threading.Tasks;
 using Microsoft.CodeAnalysis.CodeFixes;
+using Microsoft.CodeAnalysis.CSharp;
 using Microsoft.CodeAnalysis.CSharp.Analyzers.MakeFieldReadonly;
 using Microsoft.CodeAnalysis.CSharp.MakeFieldReadonly;
 using Microsoft.CodeAnalysis.Diagnostics;
@@ -18,6 +20,8 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.MakeFieldReadonly
     [Trait(Traits.Feature, Traits.Features.CodeActionsMakeFieldReadonly)]
     public class MakeFieldReadonlyTests : AbstractCSharpDiagnosticProviderBasedUserDiagnosticTest
     {
+        private static readonly ParseOptions s_strictFeatureFlag = CSharpParseOptions.Default.WithFeatures(new[] { new KeyValuePair<string, string>("strict", "true") });
+
         public MakeFieldReadonlyTests(ITestOutputHelper logger)
           : base(logger)
         {
@@ -1814,7 +1818,7 @@ void M()
         }
 
         [Fact]
-        public async Task FieldAssignedToLocalReadOnlyRef()
+        public async Task FieldAssignedToLocalreadonlyRef()
         {
             await TestInRegularAndScript1Async(
                 """
@@ -2035,7 +2039,7 @@ await TestMissingAsync(
                 public class MyClass
                 {
                     [System.Runtime.Serialization.DataMember]
-                    private bool [|isReadOnly|];
+                    private bool [|isreadonly|];
                 }
                         </Document>
                     </Project>
@@ -2054,7 +2058,7 @@ await TestInRegularAndScript1Async(
                 public class MyClass
                 {
                     [System.Runtime.Serialization.DataMember]
-                    private bool [|isReadOnly|];
+                    private bool [|isreadonly|];
                 }
                         </Document>
                     </Project>
@@ -2067,7 +2071,7 @@ public class MyClass
                 public class MyClass
                 {
                     [System.Runtime.Serialization.DataMember]
-                    private readonly bool isReadOnly;
+                    private readonly bool isreadonly;
                 }
                         </Document>
                     </Project>
@@ -2087,9 +2091,9 @@ await TestInRegularAndScript1Async(
                 public class MyClass
                 {
                     [System.Runtime.Serialization.DataMember]
-                    private bool isReadOnly;
+                    private bool isreadonly;
 
-                    private bool [|isReadOnly2|];
+                    private bool [|isreadonly2|];
                 }
                         </Document>
                     </Project>
@@ -2103,9 +2107,9 @@ public class MyClass
                 public class MyClass
                 {
                     [System.Runtime.Serialization.DataMember]
-                    private bool isReadOnly;
+                    private bool isreadonly;
 
-                    private readonly bool isReadOnly2;
+                    private readonly bool isreadonly2;
                 }
                         </Document>
                     </Project>
@@ -2124,7 +2128,7 @@ await TestMissingAsync(
                 [System.Runtime.Serialization.DataContractAttribute]
                 public class MyClass
                 {
-                    public bool [|isReadOnly|];
+                    public bool [|isreadonly|];
                 }
                         </Document>
                     </Project>
@@ -2188,5 +2192,93 @@ public class C
                 }
                 """);
         }
+
+        [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")]
+        public async Task StrictFeatureFlagAssignment1()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                using System;
+                using System.Collections.Generic;
+
+                class C<T>
+                {
+                    private static IEqualityComparer<T> [|s_value|];
+
+                    static C()
+                    {
+                        C<T>.s_value = null;
+                    }
+                }
+                """,
+                """
+                using System;
+                using System.Collections.Generic;
+
+                class C<T>
+                {
+                    private static readonly IEqualityComparer<T> s_value;
+
+                    static C()
+                    {
+                        C<T>.s_value = null;
+                    }
+                }
+                """, parseOptions: s_strictFeatureFlag);
+        }
+
+        [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")]
+        public async Task StrictFeatureFlagAssignment2()
+        {
+            await TestMissingInRegularAndScriptAsync(
+                """
+                using System;
+                using System.Collections.Generic;
+
+                class C<T>
+                {
+                    private static IEqualityComparer<T> [|s_value|];
+
+                    static C()
+                    {
+                        C<string>.s_value = null;
+                    }
+                }
+                """, new TestParameters(parseOptions: s_strictFeatureFlag));
+        }
+
+        [Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")]
+        public async Task StrictFeatureFlagAssignment3()
+        {
+            await TestInRegularAndScriptAsync(
+                """
+                using System;
+                using System.Collections.Generic;
+
+                class C<T>
+                {
+                    private static IEqualityComparer<T> [|s_value|];
+
+                    static C()
+                    {
+                        C<string>.s_value = null;
+                    }
+                }
+                """,
+                """
+                using System;
+                using System.Collections.Generic;
+
+                class C<T>
+                {
+                    private static readonly IEqualityComparer<T> s_value;
+
+                    static C()
+                    {
+                        C<string>.s_value = null;
+                    }
+                }
+                """);
+        }
     }
 }

src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs

@@ -96,15 +96,13 @@ void AnalyzeOperation(OperationAnalysisContext operationContext)
 
                     // Ignore fields that are not candidates or have already been written outside the constructor/field initializer.
                     if (!isCandidate || written)
-                    {
                         return;
-                    }
 
                     // Check if this is a field write outside constructor and field initializer, and update field state accordingly.
-                    if (IsFieldWrite(fieldReference, operationContext.ContainingSymbol))
-                    {
-                        UpdateFieldStateOnWrite(fieldReference.Field);
-                    }
+                    if (!IsFieldWrite(fieldReference, operationContext.ContainingSymbol))
+                        return;
+
+                    UpdateFieldStateOnWrite(fieldReference.Field);
                 }
 
                 void OnSymbolEnd(SymbolAnalysisContext symbolEndContext)
@@ -113,7 +111,7 @@ void OnSymbolEnd(SymbolAnalysisContext symbolEndContext)
                     var members = ((INamedTypeSymbol)symbolEndContext.Symbol).GetMembers();
                     foreach (var member in members)
                     {
-                        if (member is IFieldSymbol field && fieldStateMap.TryRemove(field, out var value))
+                        if (member is IFieldSymbol field && fieldStateMap.TryRemove(field.OriginalDefinition, out var value))
                         {
                             var (isCandidate, written) = value;
                             if (isCandidate && !written)
@@ -160,9 +158,9 @@ static bool IsDataContractSerializable(IFieldSymbol symbol, INamedTypeSymbol? da
                 void UpdateFieldStateOnWrite(IFieldSymbol field)
                 {
                     Debug.Assert(IsCandidateField(field, threadStaticAttribute, dataContractAttribute, dataMemberAttribute));
-                    Debug.Assert(fieldStateMap.ContainsKey(field));
+                    Debug.Assert(fieldStateMap.ContainsKey(field.OriginalDefinition));
 
-                    fieldStateMap[field] = (isCandidate: true, written: true);
+                    fieldStateMap[field.OriginalDefinition] = (isCandidate: true, written: true);
                 }
 
                 // Method to get or initialize the field state.
@@ -173,13 +171,13 @@ void UpdateFieldStateOnWrite(IFieldSymbol field)
                         return default;
                     }
 
-                    if (fieldStateMap.TryGetValue(fieldSymbol, out var result))
+                    if (fieldStateMap.TryGetValue(fieldSymbol.OriginalDefinition, out var result))
                     {
                         return result;
                     }
 
                     result = ComputeInitialFieldState(fieldSymbol, options, threadStaticAttribute, dataContractAttribute, dataMemberAttribute, cancellationToken);
-                    return fieldStateMap.GetOrAdd(fieldSymbol, result);
+                    return fieldStateMap.GetOrAdd(fieldSymbol.OriginalDefinition, result);
                 }
 
                 // Method to compute the initial field state.
@@ -209,9 +207,7 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo
             // Check if the underlying member is being written or a writable reference to the member is taken.
             var valueUsageInfo = fieldReference.GetValueUsageInfo(owningSymbol);
             if (!valueUsageInfo.IsWrittenTo())
-            {
                 return false;
-            }
 
             // Writes to fields inside constructor are ignored, except for the below cases:
             //  1. Instance reference of an instance field being written is not the instance being initialized by the constructor.
@@ -221,8 +217,9 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo
             var isInConstructor = owningSymbol.IsConstructor();
             var isInStaticConstructor = owningSymbol.IsStaticConstructor();
             var field = fieldReference.Field;
+
             if ((isInConstructor || isInStaticConstructor) &&
-                field.ContainingType == owningSymbol.ContainingType)
+                field.ContainingType.OriginalDefinition.Equals(owningSymbol.ContainingType.OriginalDefinition))
             {
                 // For instance fields, ensure that the instance reference is being initialized by the constructor.
                 var instanceFieldWrittenInCtor = isInConstructor &&
@@ -235,11 +232,23 @@ private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbo
                 if (instanceFieldWrittenInCtor || staticFieldWrittenInStaticCtor)
                 {
                     // Finally, ensure that the write is not inside a lambda or local function.
-                    if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is null)
+                    if (fieldReference.TryGetContainingAnonymousFunctionOrLocalFunction() is not null)
+                        return true;
+
+                    // Legacy 'strict' C#/VB behavior.  If a special "feature:strict" (different from "option strict") flag Is on,
+                    // then this write Is only ok if the containing types are the same.  *Not* simply the original
+                    // definitions being the same (which the caller has already checked):
+                    //
+                    // https://github.com/dotnet/roslyn/blob/8770fb62a36157ed4ca38a16a0283d27321a01a7/src/Compilers/CSharp/Portable/Binder/Binder.ValueChecks.cs#L1201-L1203
+                    // https//github.com/dotnet/roslyn/blob/93d3aa1a2cf1790b1a0fe2d120f00987d50445c0/src/Compilers/VisualBasic/Portable/Binding/Binder_Expressions.vb#L1868-L1871
+                    if (fieldReference.SemanticModel?.SyntaxTree.Options.Features.ContainsKey("strict") is true &&
+                        !field.ContainingType.Equals(owningSymbol.ContainingType))
                     {
-                        // It is safe to ignore this write.
-                        return false;
+                        return true;
                     }
+
+                    // It is safe to ignore this write.
+                    return false;
                 }
             }
 

src/Analyzers/VisualBasic/Tests/MakeFieldReadonly/MakeFieldReadonlyTests.vb

@@ -12,6 +12,8 @@ Namespace Microsoft.CodeAnalysis.Editor.VisualBasic.UnitTests.MakeFieldReadonly
     Public Class MakeFieldReadonlyTests
         Inherits AbstractVisualBasicDiagnosticProviderBasedUserDiagnosticTest
 
+        Private Shared ReadOnly s_strictFeatureFlag As ParseOptions = VisualBasicParseOptions.Default.WithFeatures({New KeyValuePair(Of String, String)("strict", "true")})
+
         Friend Overrides Function CreateDiagnosticProviderAndFixer(workspace As Workspace) As (DiagnosticAnalyzer, CodeFixProvider)
             Return (New VisualBasicMakeFieldReadonlyDiagnosticAnalyzer(), New VisualBasicMakeFieldReadonlyCodeFixProvider())
         End Function
@@ -1123,5 +1125,79 @@ End Module
             Await TestInRegularAndScript1Async(initialMarkup, expectedMarkup)
         End Function
 
+        <Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")>
+        Public Async Function VBStrictFeatureFlagAssignment1() As Task
+            Await TestInRegularAndScriptAsync(
+"
+imports System
+imports System.Collections.Generic
+
+class C(Of T)
+    private shared [|s_value|] as IEqualityComparer(Of T)
+
+    shared sub new()
+        C(Of T).s_value = nothing
+    end sub
+end class
+",
+"
+imports System
+imports System.Collections.Generic
+
+class C(Of T)
+    private shared ReadOnly s_value as IEqualityComparer(Of T)
+
+    shared sub new()
+        C(Of T).s_value = nothing
+    end sub
+end class
+", parseOptions:=s_strictFeatureFlag)
+        End Function
+
+        <Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")>
+        Public Async Function VBStrictFeatureFlagAssignment2() As Task
+            Await TestMissingInRegularAndScriptAsync(
+"
+imports System
+imports System.Collections.Generic
+
+class C(Of T)
+    private shared [|s_value|] as IEqualityComparer(Of T)
+
+    shared sub new()
+        C(Of string).s_value = nothing
+    end sub
+end class
+", New TestParameters(parseOptions:=s_strictFeatureFlag))
+        End Function
+
+        <Fact, WorkItem(47197, "https://github.com/dotnet/roslyn/issues/47197")>
+        Public Async Function VBStrictFeatureFlagAssignment3() As Task
+            Await TestInRegularAndScriptAsync(
+"
+imports System
+imports System.Collections.Generic
+
+class C(Of T)
+    private shared [|s_value|] as IEqualityComparer(Of T)
+
+    shared sub new()
+        C(Of string).s_value = nothing
+    end sub
+end class
+",
+"
+imports System
+imports System.Collections.Generic
+
+class C(Of T)
+    private shared ReadOnly s_value as IEqualityComparer(Of T)
+
+    shared sub new()
+        C(Of string).s_value = nothing
+    end sub
+end class
+")
+        End Function
     End Class
 End Namespace