diff --git a/src/EditorFeatures/CSharpTest/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs b/src/EditorFeatures/CSharpTest/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs index 1540d99375188c7be8f63ec8fe1fe4c0ec6e7abf..c3e75a05b8119bd018de11a4f9165fd983303ed1 100644 --- a/src/EditorFeatures/CSharpTest/RemoveUnusedParametersAndValues/RemoveUnusedValueAssignmentTests.cs +++ b/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); } } diff --git a/src/Features/CSharp/Portable/RemoveUnusedParametersAndValues/CSharpRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs b/src/Features/CSharp/Portable/RemoveUnusedParametersAndValues/CSharpRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs index a3f7b0ce27ed9e7cdfac2cc7f30ffef2f56f1330..5805c2e0c8d72b0e2362f7be8e3b149aed1f6ab1 100644 --- a/src/Features/CSharp/Portable/RemoveUnusedParametersAndValues/CSharpRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs +++ b/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()?.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; diff --git a/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs b/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs index 291dcd981fc2e286503af96aa6c86b7dc56ac51a..bffbf9b22b1360b43781530950d225e55410b512 100644 --- a/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.SymbolStartAnalyzer.BlockAnalyzer.cs +++ b/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) { diff --git a/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs b/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs index f2ee7e3fd21c1d782cbb7a7a72be427dfef7f6c8..3ba239e6f251ececeb5b0c7cecc933e610413bf0 100644 --- a/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs +++ b/src/Features/Core/Portable/RemoveUnusedParametersAndValues/AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer.cs @@ -93,6 +93,15 @@ protected AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer() protected abstract Option> UnusedValueExpressionStatementOption { get; } protected abstract Option> UnusedValueAssignmentOption { get; } + /// + /// 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. + /// + protected virtual bool ShouldBailOutFromRemovableAssignmentAnalysis(IOperation unusedSymbolWriteOperation) + => false; + /// /// Indicates if the given expression statement operation has an explicit "Call" statement syntax indicating explicit discard. /// For example, VB "Call" statement.