Merge pull request #33094 from mavasani/Issue32856

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

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

@@ -568,6 +568,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.