diff --git a/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs b/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs index 48b3eab5b066c..a0cd8ae3d72fe 100644 --- a/src/Analyzers/CSharp/Tests/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs +++ b/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.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 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 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(); + } } diff --git a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs b/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs index b1e5e48045c6f..22979e5ea92f6 100644 --- a/src/Analyzers/Core/Analyzers/MakeFieldReadonly/AbstractMakeFieldReadonlyDiagnosticAnalyzer.cs +++ b/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: diff --git a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs index cedc0a32aab41..0ae859bd6b33a 100644 --- a/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs +++ b/src/Workspaces/SharedUtilitiesAndExtensions/Compiler/Core/Extensions/OperationExtensions.cs @@ -24,6 +24,13 @@ public static bool IsTargetOfObjectMemberInitializer(this IOperation operation) /// Returns the for the given operation. /// This extension can be removed once https://github.com/dotnet/roslyn/issues/25057 is implemented. /// + /// + /// 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 through. 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). + /// Put another way, this only returns 'write' when certain that the entire value is absolutely + /// entirely overwritten. + /// 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)