Do not offer to remove redundant assignment if it is not parented by an...

Do not offer to remove redundant assignment if it is not parented by an explicit block with curly braces

Fixes #32856

For example, consider a redundant assignment to 'x' in `if (...) x = 1;`
Replacing 'x = 1' with an empty statement ';' is not useful, and user is most likely
going to remove the entire if statement in this case. However, we don't
want to suggest removing the entire if statement as that might lead to change of semantics.
So, we conservatively bail out from removable assignment analysis for such cases.

src/EditorFeatures/CSharpTest/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs

@@ -6355,6 +6355,56 @@ void M()
 
         void LocalFunctionHandler(object s, ConsoleCancelEventArgs e) => e.Cancel = j != 0;
     }
+}", options: PreferDiscard);
+        }
+
+        [WorkItem(32856, "https://github.com/dotnet/roslyn/issues/32856")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedValues)]
+        public async Task RedundantAssignment_IfStatementParent()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(int j)
+    {
+        if (M2())
+            [|j|] = 0;
+    }
+
+    bool M2() => true;
+}",
+@"class C
+{
+    void M(int j)
+    {
+        if (M2())
+            _ = 0;
+    }
+
+    bool M2() => true;
+}", options: PreferDiscard);
+        }
+
+        [WorkItem(32856, "https://github.com/dotnet/roslyn/issues/32856")]
+        [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedValues)]
+        public async Task RedundantAssignment_LoopStatementParent()
+        {
+            await TestInRegularAndScriptAsync(
+@"class C
+{
+    void M(int j, int[] array)
+    {
+        for (int i = 0; i < array.Length; i++)
+            [|j|] = i;
+    }
+}",
+@"class C
+{
+    void M(int j, int[] array)
+    {
+        for (int i = 0; i < array.Length; i++)
+            _ = i;
+    }
 }", options: PreferDiscard);
         }
     }

src/Features/CSharp/Portable/RemoveUnusedParametersAndValues/CSharpRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs

@@ -28,6 +28,28 @@ protected override bool MethodHasHandlesClause(IMethodSymbol method)
         protected override bool IsIfConditionalDirective(SyntaxNode node)
             => node is IfDirectiveTriviaSyntax;
 
+        protected override bool ShouldBailOutFromRemovableAssignmentAnalysis(IOperation unusedSymbolWriteOperation)
+        {
+            // We don't want to recommend removing the write operation if it is within a statement
+            // that is not parented by an explicit block with curly braces.
+            // For example, assignment to 'x' in 'if (...) x = 1;'
+            // Replacing 'x = 1' with an empty statement ';' is not useful, and user is most likely
+            // going to remove the entire if statement in this case. However, we don't
+            // want to suggest removing the entire if statement as that might lead to change of semantics.
+            // So, we conservatively bail out from removable assignment analysis for such cases.
+
+            var statementAncestor = unusedSymbolWriteOperation.Syntax.FirstAncestorOrSelf<StatementSyntax>()?.Parent;
+            switch (statementAncestor)
+            {
+                case BlockSyntax _:
+                case SwitchSectionSyntax _:
+                    return false;
+
+                default:
+                    return true;
+            }
+        }
+
         // C# does not have an explicit "call" statement syntax for invocations with explicit value discard.
         protected override bool IsCallStatement(IExpressionStatementOperation expressionStatement)
             => false;

src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs

@@ -567,6 +567,11 @@ private void AnalyzeOperationBlockEnd(OperationBlockAnalysisContext context)
                     // This is true if the expression for the assigned value has no side effects.
                     bool IsRemovableAssignmentWithoutSideEffects(IOperation unusedSymbolWriteOperation)
                     {
+                        if (_symbolStartAnalyzer._compilationAnalyzer.ShouldBailOutFromRemovableAssignmentAnalysis(unusedSymbolWriteOperation))
+                        {
+                            return false;
+                        }
+
                         if (unusedSymbolWriteOperation.Parent is IAssignmentOperation assignment &&
                             assignment.Target == unusedSymbolWriteOperation)
                         {

src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs

@@ -93,6 +93,15 @@ protected AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer()
         protected abstract Option<CodeStyleOption<UnusedValuePreference>> UnusedValueExpressionStatementOption { get; }
         protected abstract Option<CodeStyleOption<UnusedValuePreference>> UnusedValueAssignmentOption { get; }
 
+        /// <summary>
+        /// Indicates if we should bail from removable assignment analysis for the given
+        /// symbol write operation.
+        /// Removable assignment analysis determines if the assigned value for the symbol write
+        /// has no side effects and can be removed without changing the semantics.
+        /// </summary>
+        protected virtual bool ShouldBailOutFromRemovableAssignmentAnalysis(IOperation unusedSymbolWriteOperation)
+            => false;
+
         /// <summary>
         /// Indicates if the given expression statement operation has an explicit "Call" statement syntax indicating explicit discard.
         /// For example, VB "Call" statement.