Fix false positive 'Unnecessary assignment of a value' (#77297)

src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs

@@ -23,13 +23,9 @@ namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RemoveUnusedParametersA
     CSharpRemoveUnusedValuesCodeFixProvider>;
 
 [Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedValues)]
-public class RemoveUnusedValueAssignmentTests : RemoveUnusedValuesTestsBase
+public sealed class RemoveUnusedValueAssignmentTests(ITestOutputHelper logger)
+    : RemoveUnusedValuesTestsBase(logger)
 {
-    public RemoveUnusedValueAssignmentTests(ITestOutputHelper logger)
-      : base(logger)
-    {
-    }
-
     private protected override OptionsCollection PreferNone
         => Option(CSharpCodeStyleOptions.UnusedValueAssignment,
                new CodeStyleOption2<UnusedValuePreference>(UnusedValuePreference.DiscardVariable, NotificationOption2.None));
@@ -10092,4 +10088,105 @@ void M()
             }
             """);
     }
+
+    [Fact]
+    public async Task TestWriteIntoPropertyOfRefStructParameter()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                internal sealed class C
+                {
+                    private static void M(ref int destinationIndex, Span<byte> buffer)
+                    {
+                        buffer[destinationIndex++] = (byte)0;
+                    }
+                }
+                """,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
+
+    [Fact]
+    public async Task TestWriteIntoPropertyOfRefStructParameterThenWriteTheParameter()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                internal sealed class C
+                {
+                    private static void M(ref int destinationIndex, Span<byte> buffer)
+                    {
+                        buffer[destinationIndex++] = (byte)0;
+                        // /0/Test0.cs(8,9): info IDE0059: Unnecessary assignment of a value to 'buffer'
+                        {|IDE0059:buffer|} = default;
+                    }
+                }
+                """,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
+
+    [Fact]
+    public async Task TestWriteIntoPropertyOfStructParameter()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+                internal struct S
+                {
+                    public byte this[int index] { get => 0; set => _ = value; }
+                }
+
+                internal sealed class C
+                {
+                    private static void M(ref int destinationIndex, S buffer)
+                    {
+                        // Don't want to report IDE0059 here.  This write might be necessary as the struct might be wrapping
+                        // memory visible elsewhere.
+                        buffer[destinationIndex++] = (byte)0;
+                    }
+                }
+                """,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+        }.RunAsync();
+    }
+
+    [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/77258")]
+    public async Task TestWriteIntoStructIndexer()
+    {
+        await new VerifyCS.Test
+        {
+            TestCode = """
+                using System;
+
+                int[] a = new int[5];
+                MyStruct m = new MyStruct(a);
+                m[0] = 1;
+                Console.WriteLine(a[0]);
+
+                struct MyStruct(int[] a)
+                {
+                    private int[] array = a;
+
+                    public int this[int index]
+                    {
+                        get => array[index];
+                        set => array[index] = value;
+                    }
+                }
+                """,
+            ReferenceAssemblies = ReferenceAssemblies.Net.Net80,
+            LanguageVersion = LanguageVersion.CSharp12,
+            TestState =
+            {
+                OutputKind = OutputKind.ConsoleApplication,
+            }
+        }.RunAsync();
+    }
 }

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

@@ -234,11 +234,26 @@ static bool IsDataContractSerializable(IFieldSymbol symbol, INamedTypeSymbol? da
     private static Location GetDiagnosticLocation(IFieldSymbol field)
         => field.Locations[0];
 
+    private static bool IsWrittenTo(IOperation? operation, ISymbol owningSymbol)
+    {
+        if (operation is null)
+            return false;
+
+        var result = operation.GetValueUsageInfo(owningSymbol);
+        if (result.IsWrittenTo())
+            return true;
+
+        // Accessing an indexer/property off of a value type will read/write the value type depending on how the
+        // indexer/property itself is used.
+        return operation is { Type.IsValueType: true, Parent: IPropertyReferenceOperation }
+            ? IsWrittenTo(operation.Parent, owningSymbol)
+            : false;
+    }
+
     private static bool IsFieldWrite(IFieldReferenceOperation fieldReference, ISymbol owningSymbol)
     {
         // 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())
+        if (!IsWrittenTo(fieldReference, owningSymbol))
             return false;
 
         // Writes to fields inside constructor are ignored, except for the below cases:

src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs

@@ -24,6 +24,13 @@ public static bool IsTargetOfObjectMemberInitializer(this IOperation operation)
     /// Returns the <see cref="ValueUsageInfo"/> for the given operation.
     /// This extension can be removed once https://github.com/dotnet/roslyn/issues/25057 is implemented.
     /// </summary>
+    /// <remarks>
+    /// When referring to a variable, this method should only return a 'write' result if the variable is entirely
+    /// overwritten.  Not if the variable is written <em>through</em>.  For example, a write to a property on a struct
+    /// variable is not a write to the struct variable (though at runtime it might impact the value in some fashion).
+    /// <para/> Put another way, this only returns 'write' when certain that the entire value <em>is</em> absolutely
+    /// entirely overwritten.
+    /// </remarks>
     public static ValueUsageInfo GetValueUsageInfo(this IOperation operation, ISymbol containingSymbol)
     {
         /*
@@ -205,13 +212,6 @@ where void Foo(in T v)
         {
             return ValueUsageInfo.Write;
         }
-        else if (operation is { Type.IsValueType: true, Parent: IPropertyReferenceOperation })
-        {
-            // Accessing an indexer/property off of a value type will read/write the value type depending on how the
-            // indexer/property itself is used. We add ValueUsageInfo.Read to the result since the value on which we are
-            // accessing a property is always read.
-            return ValueUsageInfo.Read | GetValueUsageInfo(operation.Parent, containingSymbol);
-        }
         else if (operation.Parent is IVariableInitializerOperation variableInitializerOperation)
         {
             if (variableInitializerOperation.Parent is IVariableDeclaratorOperation variableDeclaratorOperation)