Do not convert mutable value type fields to properties

Fixes #28511

src/EditorFeatures/CSharpTest/UseAutoProperty/UseAutoPropertyTests.cs

@@ -39,6 +39,50 @@ int P
 }");
         }
 
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)]
+        [WorkItem(28511, "https://github.com/dotnet/roslyn/issues/28511")]
+        public async Task TestMutableValueType1()
+        {
+            // Nullable<T> is a mutable value type. The diagnostic is not offered if the field is writable.
+            await TestMissingInRegularAndScriptAsync(
+@"class Class
+{
+    [|int? i|];
+
+    int? P
+    {
+        get
+        {
+            return i;
+        }
+    }
+}");
+        }
+
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)]
+        [WorkItem(28511, "https://github.com/dotnet/roslyn/issues/28511")]
+        public async Task TestMutableValueType2()
+        {
+            // Nullable<T> is a mutable value type. The diagnostic is offered if the field is read-only.
+            await TestInRegularAndScriptAsync(
+@"class Class
+{
+    [|readonly int? i|];
+
+    int? P
+    {
+        get
+        {
+            return i;
+        }
+    }
+}",
+@"class Class
+{
+    int? P { get; }
+}");
+        }
+
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)]
         public async Task TestCSharp5_1()
         {
@@ -951,7 +995,7 @@ public async Task Tuple_SingleGetterFromField()
             await TestInRegularAndScriptAsync(
 @"class Class
 {
-    [|(int, string) i|];
+    [|readonly (int, string) i|];
 
     (int, string) P
     {
@@ -973,7 +1017,7 @@ public async Task TupleWithNames_SingleGetterFromField()
             await TestInRegularAndScriptAsync(
 @"class Class
 {
-    [|(int a, string b) i|];
+    [|readonly (int a, string b) i|];
 
     (int a, string b) P
     {
@@ -995,7 +1039,7 @@ public async Task TupleWithDifferentNames_SingleGetterFromField()
             await TestMissingInRegularAndScriptAsync(
 @"class Class
 {
-    [|(int a, string b) i|];
+    [|readonly (int a, string b) i|];
 
     (int c, string d) P
     {
@@ -1013,7 +1057,7 @@ public async Task TupleWithOneName_SingleGetterFromField()
             await TestInRegularAndScriptAsync(
 @"class Class
 {
-    [|(int a, string) i|];
+    [|readonly (int a, string) i|];
 
     (int a, string) P
     {
@@ -1035,7 +1079,7 @@ public async Task Tuple_Initializer()
             await TestInRegularAndScriptAsync(
 @"class Class
 {
-    [|(int, string) i = (1, ""hello"")|];
+    [|readonly (int, string) i = (1, ""hello"")|];
 
     (int, string) P
     {
@@ -1054,7 +1098,7 @@ public async Task Tuple_Initializer()
         [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)]
         public async Task Tuple_GetterAndSetter()
         {
-            await TestInRegularAndScriptAsync(
+            await TestMissingInRegularAndScriptAsync(
 @"class Class
 {
     [|(int, string) i|];
@@ -1071,10 +1115,6 @@ public async Task Tuple_GetterAndSetter()
             i = value;
         }
     }
-}",
-@"class Class
-{
-    (int, string) P { get; set; }
 }");
         }
 

src/EditorFeatures/VisualBasicTest/UseAutoProperty/UseAutoPropertyTests.vb

@@ -62,6 +62,39 @@ end class",
 end class")
         End Function
 
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)>
+        <WorkItem(28511, "https://github.com/dotnet/roslyn/issues/28511")>
+        Public Async Function TestSingleGetter4() As Task
+            ' Nullable(Of T) is a mutable value type. The diagnostic is not offered if the field is writable.
+            Await TestMissingInRegularAndScriptAsync(
+"class Class1
+    [|dim i as integer?|]
+    readonly property P as integer?
+        get
+            return i
+        end get
+    end property
+end class")
+        End Function
+
+        <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)>
+        <WorkItem(28511, "https://github.com/dotnet/roslyn/issues/28511")>
+        Public Async Function TestSingleGetter5() As Task
+            ' Nullable(Of T) is a mutable value type. The diagnostic is offered if the field is read-only.
+            Await TestInRegularAndScriptAsync(
+"class Class1
+    [|readonly dim i as integer?|]
+    readonly property P as integer?
+        get
+            return i
+        end get
+    end property
+end class",
+"class Class1
+    readonly property P as integer?
+end class")
+        End Function
+
         <Fact, Trait(Traits.Feature, Traits.Features.CodeActionsUseAutoProperty)>
         Public Async Function TestSingleSetter() As Task
             Await TestMissingInRegularAndScriptAsync(

src/Features/Core/Portable/MakeFieldReadonly/MakeFieldReadonlyDiagnosticAnalyzer.cs

@@ -107,7 +107,7 @@ void OnSymbolEnd(SymbolAnalysisContext symbolEndContext)
                         !symbol.IsConst &&
                         !symbol.IsImplicitlyDeclared &&
                         symbol.Locations.Length == 1 &&
-                        !IsMutableValueType(symbol.Type);
+                        !symbol.Type.IsMutableValueType();
 
                 // Method to update the field state for a candidate field written outside constructor and field initializer.
                 void UpdateFieldStateOnWrite(IFieldSymbol field)
@@ -216,24 +216,5 @@ private static CodeStyleOption<bool> GetCodeStyleOption(IFieldSymbol field, Anal
             var optionSet = options.GetDocumentOptionSetAsync(field.Locations[0].SourceTree, cancellationToken).GetAwaiter().GetResult();
             return optionSet?.GetOption(CodeStyleOptions.PreferReadonly, field.Language);
         }
-
-        private static bool IsMutableValueType(ITypeSymbol type)
-        {
-            if (type.TypeKind != TypeKind.Struct)
-            {
-                return false;
-            }
-
-            foreach (var member in type.GetMembers())
-            {
-                if (member is IFieldSymbol fieldSymbol &&
-                    !(fieldSymbol.IsConst || fieldSymbol.IsReadOnly || fieldSymbol.IsStatic))
-                {
-                    return true;
-                }
-            }
-
-            return false;
-        }
     }
 }

src/Features/Core/Portable/UseAutoProperty/AbstractUseAutoPropertyAnalyzer.cs

@@ -5,6 +5,7 @@
 using System.Threading;
 using Microsoft.CodeAnalysis.CodeStyle;
 using Microsoft.CodeAnalysis.Diagnostics;
+using Microsoft.CodeAnalysis.Shared.Extensions;
 
 namespace Microsoft.CodeAnalysis.UseAutoProperty
 {
@@ -163,6 +164,12 @@ private void AnalyzeSemanticModel(SemanticModelAnalysisContext context)
                 return;
             }
 
+            // The field can't be a mutable value type unless it's marked readonly
+            if (!getterField.IsReadOnly && getterField.Type.IsMutableValueType())
+            {
+                return;
+            }
+
             // Don't want to remove constants and volatile fields.
             if (getterField.IsConst || getterField.IsVolatile)
             {

src/Workspaces/Core/Portable/Shared/Extensions/ITypeSymbolExtensions.cs

@@ -854,5 +854,24 @@ public static bool IsEnumType(this ITypeSymbol type)
         {
             return type.IsValueType && type.TypeKind == TypeKind.Enum;
         }
+
+        public static bool IsMutableValueType(this ITypeSymbol type)
+        {
+            if (type.TypeKind != TypeKind.Struct)
+            {
+                return false;
+            }
+
+            foreach (var member in type.GetMembers())
+            {
+                if (member is IFieldSymbol fieldSymbol &&
+                    !(fieldSymbol.IsConst || fieldSymbol.IsReadOnly || fieldSymbol.IsStatic))
+                {
+                    return true;
+                }
+            }
+
+            return false;
+        }
     }
 }