diff --git a/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs b/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs index 464b1299e3da5..1f31932d45c46 100644 --- a/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs +++ b/src/EditorFeatures/CSharpTest/RemoveUnusedVariable/RemoveUnusedVariableTests.cs @@ -308,276 +308,5 @@ bool TrySomething() } }"); } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task RemoveVariableAndComment() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - void M() - { - int [|unused|] = 0; // remove also comment - } -} -", -@" -class C -{ - void M() - { - } -} -"); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task RemoveVariableAndAssgnment() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - void M() - { - int [|b|] = 0; - b = 0; - } -} -", -@" -class C -{ - void M() - { - } -} -"); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task JointDeclarationRemoveFirst() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - int M() - { - int [|unused|] = 0, used = 0; - return used; - } -} -", -@" -class C -{ - int M() - { - int used = 0; - return used; - } -} -"); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task JointDeclarationRemoveSecond() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - int M() - { - int used = 0, [|unused|] = 0; - return used; - } -} -", -@" -class C -{ - int M() - { - int used = 0; - return used; - } -} -"); - } - - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/23322"), Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task JointAssignmentRemoveFirst() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - int M() - { - int [|unused|] = 0; - int used = 0; - unused = used = 0; - return used; - } -} -", -@" -class C -{ - int M() - { - int used = 0; - used = 0; - return used; - } -} -"); - } - - [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task JointAssignmentRemoveSecond() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - int M() - { - int used = 0; - int [|unused|] = 0; - used = unused = 0; - return used; - } -} -", -@" -class C -{ - int M() - { - int used = 0; - used = 0; - return used; - } -} -"); - } - - [Fact(Skip = "https://github.com/dotnet/roslyn/issues/22921"), Trait(Traits.Feature, Traits.Features.CodeActionsRemoveUnusedVariable)] - public async Task RemoveUnusedLambda() - { - await TestInRegularAndScriptAsync( -@" -class C -{ - int M() - { - Func [|unused|] = () => - { - return 0; - }; - return 1; - } -} -", -@" -class C -{ - int M() - { - return 1; - } -} -"); - } - - [Fact] - [Trait(Traits.Feature, Traits.Features.CodeActionsSimplifyTypeNames)] - [Trait(Traits.Feature, Traits.Features.CodeActionsFixAllOccurrences)] - public async Task JointDeclarationRemoveBoth() - { - var input = @" - - - -class C -{ - int M() - { - int {|FixAllInDocument:a|} = 0, b = 0; - return 0; - } -} - - - -"; - - var expected = @" - - - -class C -{ - int M() - { - return 0; - } -} - - - -"; - - await TestInRegularAndScriptAsync(input, expected); - } - - [Fact] - [Trait(Traits.Feature, Traits.Features.CodeActionsSimplifyTypeNames)] - [Trait(Traits.Feature, Traits.Features.CodeActionsFixAllOccurrences)] - public async Task JointAssignment() - { - var input = @" - - - -class C -{ - int M() - { - int a = 0; - int {|FixAllInDocument:b|} = 0; - a = b = 0; - return 0; - } -} - - - -"; - - var expected = @" - - - -class C -{ - int M() - { - int a = 0; - a = 0; - return 0; - } -} - - - -"; - - await TestInRegularAndScriptAsync(input, expected); - } } } diff --git a/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs b/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs index cfb4598ea955e..4e65b6269a0ac 100644 --- a/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs +++ b/src/EditorFeatures/TestUtilities/CodeActions/AbstractCodeActionOrUserDiagnosticTest.cs @@ -200,7 +200,7 @@ protected async Task TestAddDocument( { var codeActions = await GetCodeActionsAsync(workspace, parameters); await TestAddDocument( - workspace, expectedMarkup, index, expectedContainers, + workspace, expectedMarkup, index, expectedContainers, expectedDocumentName, codeActions); } } @@ -344,7 +344,7 @@ private async Task TestAsync( TestParameters parameters) { MarkupTestFile.GetSpans( - expectedMarkup.NormalizeLineEndings(), + expectedMarkup.NormalizeLineEndings(), out var expected, out IDictionary> spanMap); var conflictSpans = spanMap.GetOrAdd("Conflict", _ => ImmutableArray.Empty); @@ -489,7 +489,6 @@ internal static async Task> VerifyInputsAndG int index, ImmutableArray actions, CodeActionPriority? priority = null) { Assert.NotNull(actions); - Assert.NotEmpty(actions); if (actions.Length == 1) { if (actions.Single() is TopLevelSuppressionCodeAction suppressionAction) diff --git a/src/EditorFeatures/VisualBasicTest/RemoveUnusedVariable/RemoveUnusedVariableTest.vb b/src/EditorFeatures/VisualBasicTest/RemoveUnusedVariable/RemoveUnusedVariableTest.vb index 603e1ffafe272..8866afa4237a8 100644 --- a/src/EditorFeatures/VisualBasicTest/RemoveUnusedVariable/RemoveUnusedVariableTest.vb +++ b/src/EditorFeatures/VisualBasicTest/RemoveUnusedVariable/RemoveUnusedVariableTest.vb @@ -56,7 +56,7 @@ End Module Await TestAsync(markup, expected) End Function - + Public Async Function RemoveUnusedVariableFixAll() As Task Dim markup = @@ -73,225 +73,6 @@ Module M Sub Main() End Sub End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function RemoveUnusedVariableAndComment() As Task - Dim markup = - -Module M - Sub Main() - Dim [|a|] As Integer ' inline comment also to be deleted. - End Sub -End Module - - Dim expected = - -Module M - Sub Main() - End Sub -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function RemoveUnusedVariableWithAssignment() As Task - Dim markup = - -Module M - Sub Main() - Dim [|a|] As Integer = 0 - End Sub -End Module - - Dim expected = - -Module M - Sub Main() - End Sub -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function RemoveUnusedWithImplicitConversionAndAssignment() As Task - Dim markup = - -Module M - Sub Main() - Dim [|a|] As Short = 0 - a = 1 - End Sub -End Module - - Dim expected = - -Module M - Sub Main() - End Sub -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function RemoveUnusedLambda() As Task - Dim markup = - -Module M - Public Class C - Function F() As Integer - Dim L As Func(Of Integer) = Function() - Dim a As Integer = 0 - Dim [|unused|] As Func(Of Integer) = Function() - Dim b As Integer = 0 - Return 1 - End Function - Return 1 - End Function - Return L() - End Function - End Class -End Module - - Dim expected = - -Module M - Public Class C - Function F() As Integer - Dim L As Func(Of Integer) = Function() - Dim a As Integer = 0 - Return 1 - End Function - Return L() - End Function - End Class -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function JointDeclarationRemoveFirst() As Task - Dim markup = - -Module M - Function F() As Integer - Dim [|a|] As Integer, b As Integer - Return b - End Function -End Module - - Dim expected = - -Module M - Function F() As Integer - Dim b As Integer - Return b - End Function -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function JointDeclarationAndAssignmentRemoveFirst() As Task - Dim markup = - -Module M - Function F() As Integer - Dim [|a|] As Integer = 0, b As Integer = 0 - Return b - End Function -End Module - - Dim expected = - -Module M - Function F() As Integer - Dim b As Integer - Return b - End Function -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function JointDeclarationRemoveSecond() As Task - Dim markup = - -Module M - Function F() As Integer - Dim a As Integer, [|b|] As Integer - Return a - End Function -End Module - - Dim expected = - -Module M - Function F() As Integer - Dim a As Integer - Return a - End Function -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function JointDeclarationAndAssignmentRemoveSecond() As Task - Dim markup = - -Module M - Function F() As Integer - Dim a As Integer = 0, [|b|] As Integer = 0 - Return a - End Function -End Module - - Dim expected = - -Module M - Function F() As Integer - Dim a As Integer - Return a - End Function -End Module - - - Await TestAsync(markup, expected) - End Function - - - Public Async Function JointDeclarationAndAssignmentRemoveBoth() As Task - Dim markup = - -Module M - Sub F() - Dim {|FixAllInDocument:a as Integer|} = 0, b As Integer = 0 - End Sub -End Module - - Dim expected = - -Module M - Sub F() - End Sub -End Module Await TestAsync(markup, expected) diff --git a/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs b/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs index b3c0f35ddf0aa..3be20f579c30e 100644 --- a/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs +++ b/src/Features/CSharp/Portable/RemoveUnusedVariable/CSharpRemoveUnusedVariableCodeFixProvider.cs @@ -4,8 +4,6 @@ using System.Composition; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.CSharp.Syntax; -using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.RemoveUnusedVariable; namespace Microsoft.CodeAnalysis.CSharp.RemoveUnusedVariable @@ -22,40 +20,5 @@ public sealed override ImmutableArray FixableDiagnosticIds protected override bool IsCatchDeclarationIdentifier(SyntaxToken token) => token.Parent is CatchDeclarationSyntax catchDeclaration && catchDeclaration.Identifier == token; - - protected override SyntaxNode GetNodeToRemoveOrReplace(SyntaxNode node) - { - node = node.Parent; - if (node.Kind() == SyntaxKind.SimpleAssignmentExpression) - { - var parent = node.Parent; - if (parent.Kind() == SyntaxKind.ExpressionStatement) - { - return parent; - } - else - { - return node; - } - } - - return null; - } - - protected override void RemoveOrReplaceNode(SyntaxEditor editor, SyntaxNode node, ISyntaxFactsService syntaxFacts) - { - switch (node.Kind()) - { - case SyntaxKind.SimpleAssignmentExpression: - editor.ReplaceNode(node, ((AssignmentExpressionSyntax)node).Right); - return; - default: - RemoveNode(editor, node, syntaxFacts); - return; - } - } - - protected override SeparatedSyntaxList GetVariables(LocalDeclarationStatementSyntax localDeclarationStatement) - => localDeclarationStatement.Declaration.Variables; } } diff --git a/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs b/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs index 2718e6623c930..1b88322d8be11 100644 --- a/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs +++ b/src/Features/Core/Portable/RemoveUnusedVariable/AbstractRemoveUnusedVariableCodeFixProvider.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft. All Rights Reserved. Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; using System.Threading; @@ -9,7 +8,6 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.CodeFixes; using Microsoft.CodeAnalysis.Editing; -using Microsoft.CodeAnalysis.FindSymbols; using Microsoft.CodeAnalysis.Formatting; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; @@ -24,159 +22,85 @@ internal abstract class AbstractRemoveUnusedVariableCodeFixProvider(); - protected abstract SeparatedSyntaxList GetVariables(TLocalDeclarationStatement localDeclarationStatement); + if (ancestor == null) + { + return; + } + } + } - public sealed override Task RegisterCodeFixesAsync(CodeFixContext context) - { - Diagnostic diagnostic = context.Diagnostics.Single(); - context.RegisterCodeFix(new MyCodeAction(c => FixAsync(context.Document, diagnostic, c)), diagnostic); - return Task.CompletedTask; + context.RegisterCodeFix( + new MyCodeAction(c => FixAsync(context.Document, context.Diagnostics.First(), c)), + context.Diagnostics); } - protected override async Task FixAllAsync(Document document, ImmutableArray diagnostics, SyntaxEditor syntaxEditor, CancellationToken cancellationToken) + protected override Task FixAllAsync(Document document, ImmutableArray diagnostics, SyntaxEditor editor, CancellationToken cancellationToken) { - var nodesToRemove = new HashSet(); - - // Create actions and keep their SpanStart. - // Execute actions ordered descending by SpanStart to avoid conflicts. - var actionsToPerform = new List<(int, Action)>(); - var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); - var documentEditor = await DocumentEditor.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var documentsToBeSearched = ImmutableHashSet.Create(document); - + var syntaxFacts = document.GetLanguageService(); + var root = editor.OriginalRoot; foreach (var diagnostic in diagnostics) { var token = diagnostic.Location.FindToken(cancellationToken); - var node = root.FindNode(diagnostic.Location.SourceSpan); if (IsCatchDeclarationIdentifier(token)) { - (int, Action) pair = (token.Parent.SpanStart, - () => syntaxEditor.ReplaceNode( - token.Parent, - token.Parent.ReplaceToken(token, default(SyntaxToken)).WithAdditionalAnnotations(Formatter.Annotation))); - actionsToPerform.Add(pair); + editor.ReplaceNode( + token.Parent, + token.Parent.ReplaceToken(token, default(SyntaxToken)).WithAdditionalAnnotations(Formatter.Annotation)); } else { - nodesToRemove.Add(node); - } - - var symbol = documentEditor.SemanticModel.GetDeclaredSymbol(node); - var referencedSymbols = await SymbolFinder.FindReferencesAsync(symbol, document.Project.Solution, documentsToBeSearched, cancellationToken).ConfigureAwait(false); + var variableDeclarator = token.GetAncestor(); + var variableDeclarators = token.GetAncestor().ChildNodes().Where(x => x is TVariableDeclarator); - foreach (var referencedSymbol in referencedSymbols) - { - if (referencedSymbol?.Locations != null) + if (variableDeclarators.Count() == 1) { - foreach (var location in referencedSymbol.Locations) + var localDeclaration = token.GetAncestor(); + var removeOptions = SyntaxGenerator.DefaultRemoveOptions; + + if (localDeclaration.GetLeadingTrivia().Contains(t => t.IsDirective)) + { + removeOptions |= SyntaxRemoveOptions.KeepLeadingTrivia; + } + else { - var referencedSymbolNode = root.FindNode(location.Location.SourceSpan); - if (referencedSymbolNode != null) + var statementParent = localDeclaration.Parent; + if (syntaxFacts.IsExecutableBlock(statementParent)) { - var nodeToRemoveOrReplace = GetNodeToRemoveOrReplace(referencedSymbolNode); - if (nodeToRemoveOrReplace != null) + var siblings = syntaxFacts.GetExecutableBlockStatements(statementParent); + var localDeclarationIndex = siblings.IndexOf(localDeclaration); + if (localDeclarationIndex != 0) { - nodesToRemove.Add(nodeToRemoveOrReplace); + // if we're removing hte first statement in a block, then we + // want to have the elastic marker on it so that the next statement + // properly formats with the space left behind. But if it's + // not the first statement then just keep the trivia as is + // so that the statement before and after it stay appropriately + // spaced apart. + removeOptions &= ~SyntaxRemoveOptions.AddElasticMarker; } } } - } - } - } - MergeNodesToRemove(nodesToRemove); - var syntaxFacts = document.GetLanguageService(); - foreach (var node in nodesToRemove) - { - actionsToPerform.Add((node.SpanStart, () => RemoveOrReplaceNode(syntaxEditor, node, syntaxFacts))); - } - - // Process nodes in reverse order - // to complete with nested declarations before processing the outer ones. - foreach (var node in actionsToPerform.OrderByDescending(n => n.Item1)) - { - node.Item2(); - } - } - - protected void RemoveNode(SyntaxEditor editor, SyntaxNode node, ISyntaxFactsService syntaxFacts) - { - var localDeclaration = node.GetAncestorOrThis(); - var removeOptions = CreateSyntaxRemoveOptions(localDeclaration, syntaxFacts); - editor.RemoveNode(node, removeOptions); - } - - private SyntaxRemoveOptions CreateSyntaxRemoveOptions(TLocalDeclarationStatement localDeclaration, ISyntaxFactsService syntaxFacts) - { - var removeOptions = SyntaxGenerator.DefaultRemoveOptions; - - if (localDeclaration != null) - { - if (localDeclaration.GetLeadingTrivia().Contains(t => t.IsDirective)) - { - removeOptions |= SyntaxRemoveOptions.KeepLeadingTrivia; - } - else - { - var statementParent = localDeclaration.Parent; - if (syntaxFacts.IsExecutableBlock(statementParent)) - { - var siblings = syntaxFacts.GetExecutableBlockStatements(statementParent); - var localDeclarationIndex = siblings.IndexOf(localDeclaration); - if (localDeclarationIndex != 0) - { - // if we're removing the first statement in a block, then we - // want to have the elastic marker on it so that the next statement - // properly formats with the space left behind. But if it's - // not the first statement then just keep the trivia as is - // so that the statement before and after it stay appropriately - // spaced apart. - removeOptions &= ~SyntaxRemoveOptions.AddElasticMarker; - } + editor.RemoveNode(localDeclaration, removeOptions); } - } - } - - return removeOptions; - } - - // Merges node like - // var unused1 = 0, unused2 = 0; - // to remove the whole line. - private void MergeNodesToRemove(HashSet nodesToRemove) - { - var candidateLocalDeclarationsToRemove = new HashSet(); - foreach (var variableDeclarator in nodesToRemove.OfType()) - { - var localDeclaration = (TLocalDeclarationStatement)variableDeclarator.Parent.Parent; - candidateLocalDeclarationsToRemove.Add(localDeclaration); - } - - foreach (var candidate in candidateLocalDeclarationsToRemove) - { - var hasUsedLocal = false; - foreach (var variable in GetVariables(candidate)) - { - if (!nodesToRemove.Contains(variable)) + else if (variableDeclarators.Count() > 1) { - hasUsedLocal = true; - break; - } - } - - if (!hasUsedLocal) - { - nodesToRemove.Add(candidate); - foreach (var variable in GetVariables(candidate)) - { - nodesToRemove.Remove(variable); + editor.RemoveNode(variableDeclarator); } } } + + return SpecializedTasks.EmptyTask; } private class MyCodeAction : CodeAction.DocumentChangeAction diff --git a/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb b/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb index ae5ef29e6660d..8d2c952d62582 100644 --- a/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb +++ b/src/Features/VisualBasic/Portable/RemoveUnusedVariable/VisualBasicRemoveUnusedVariableCodeFixProvider.vb @@ -3,8 +3,6 @@ Imports System.Collections.Immutable Imports System.Composition Imports Microsoft.CodeAnalysis.CodeFixes -Imports Microsoft.CodeAnalysis.Editing -Imports Microsoft.CodeAnalysis.LanguageServices Imports Microsoft.CodeAnalysis.RemoveUnusedVariable Imports Microsoft.CodeAnalysis.VisualBasic.Syntax @@ -24,18 +22,5 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.RemoveUnusedVariable ' VB does not support catch declarations without an identifier in them Return False End Function - - Protected Overrides Function GetNodeToRemoveOrReplace(node As SyntaxNode) As SyntaxNode - node = node.Parent - Return If(node.Kind() = SyntaxKind.SimpleAssignmentStatement, node, Nothing) - End Function - - Protected Overrides Sub RemoveOrReplaceNode(editor As SyntaxEditor, node As SyntaxNode, syntaxFacts As ISyntaxFactsService) - RemoveNode(editor, node, syntaxFacts) - End Sub - - Protected Overrides Function GetVariables(localDeclarationStatement As LocalDeclarationStatementSyntax) As SeparatedSyntaxList(Of SyntaxNode) - Return localDeclarationStatement.Declarators - End Function End Class End Namespace