diff --git a/src/EditorFeatures/CSharpTest/CodeRefactorings/UseExplicitOrImplicitType/UseImplicitTypeRefactoringTests.cs b/src/EditorFeatures/CSharpTest/CodeRefactorings/UseExplicitOrImplicitType/UseImplicitTypeRefactoringTests.cs index 529826fffb10c3159b959898b384ee8e23c1cdf9..5bce364bd1ad02995a96de1131af6dd13209e867 100644 --- a/src/EditorFeatures/CSharpTest/CodeRefactorings/UseExplicitOrImplicitType/UseImplicitTypeRefactoringTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeRefactorings/UseExplicitOrImplicitType/UseImplicitTypeRefactoringTests.cs @@ -105,7 +105,6 @@ static void Main() await TestMissingInRegularAndScriptAsync(code); } - [Fact] public async Task TestForeachInsideLocalDeclaration() { diff --git a/src/EditorFeatures/CSharpTest/RefactoringHelpers/RefactoringHelpersTests.cs b/src/EditorFeatures/CSharpTest/RefactoringHelpers/RefactoringHelpersTests.cs index fadb821e4c909eedfae7c626868a94be9c712bd8..d859286d33e6d4e4c48dc4bccb26c970d1433014 100644 --- a/src/EditorFeatures/CSharpTest/RefactoringHelpers/RefactoringHelpersTests.cs +++ b/src/EditorFeatures/CSharpTest/RefactoringHelpers/RefactoringHelpersTests.cs @@ -1,12 +1,10 @@ // 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.Threading.Tasks; -using ICSharpCode.Decompiler.CSharp.Syntax; using Microsoft.CodeAnalysis.CSharp.Syntax; using Microsoft.CodeAnalysis.Editor.UnitTests.RefactoringHelpers; using Microsoft.CodeAnalysis.Editor.UnitTests.Workspaces; using Roslyn.Test.Utilities; -using Roslyn.Utilities; using Xunit; namespace Microsoft.CodeAnalysis.Editor.CSharp.UnitTests.RefactoringHelpers diff --git a/src/EditorFeatures/TestUtilities/RefactoringHelpers/RefactoringHelpersTestBase.cs b/src/EditorFeatures/TestUtilities/RefactoringHelpers/RefactoringHelpersTestBase.cs index a6f847e3ea05f91072683bf11e7d335d49ea5dd4..24c229f976b2b195ffded4c313cab773e79158e4 100644 --- a/src/EditorFeatures/TestUtilities/RefactoringHelpers/RefactoringHelpersTestBase.cs +++ b/src/EditorFeatures/TestUtilities/RefactoringHelpers/RefactoringHelpersTestBase.cs @@ -89,9 +89,9 @@ private static string GetSelectionAndResultSpans(string text, out TextSpan selec private async Task GetNodeForSelection(string text, TextSpan selection, Func predicate) where TNode : SyntaxNode { var document = fixture.UpdateDocument(text, SourceCodeKind.Regular); - var resultNode = await document.TryGetSelectedNodeAsync(selection, predicate, CancellationToken.None).ConfigureAwait(false); + var relevantNodes = await document.TryGetRelevantNodesAsync(selection, CancellationToken.None).ConfigureAwait(false); - return resultNode; + return relevantNodes.FirstOrDefault(predicate); } } } diff --git a/src/Features/CSharp/Portable/CodeRefactorings/CSharpRefactoringHelpersService.cs b/src/Features/CSharp/Portable/CodeRefactorings/CSharpRefactoringHelpersService.cs index 992aca3b2bc634092c8bfa4382d5ff1c01b98c11..d906bc1e9bd3facdb0e3dd166395da5b665303e6 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/CSharpRefactoringHelpersService.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/CSharpRefactoringHelpersService.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings { [ExportLanguageService(typeof(IRefactoringHelpersService), LanguageNames.CSharp), Shared] - internal class CSharpRefactoringHelpersService : AbstractRefactoringHelpersService + internal class CSharpRefactoringHelpersService : AbstractRefactoringHelpersService { protected override IEnumerable ExtractNodesSimple(SyntaxNode node, ISyntaxFactsService syntaxFacts) { diff --git a/src/Features/CSharp/Portable/NameTupleElement/CSharpNameTupleElementCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/NameTupleElement/CSharpNameTupleElementCodeRefactoringProvider.cs index 695e7bba652cf46ec846fc866ed5e8f625fe1c49..2f80a3fbc44666c886be5c4c357511c312eadcc6 100644 --- a/src/Features/CSharp/Portable/NameTupleElement/CSharpNameTupleElementCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/NameTupleElement/CSharpNameTupleElementCodeRefactoringProvider.cs @@ -11,7 +11,7 @@ namespace Microsoft.CodeAnalysis.CSharp.NameTupleElement { [ExtensionOrder(After = PredefinedCodeRefactoringProviderNames.IntroduceVariable)] [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = nameof(CSharpNameTupleElementCodeRefactoringProvider)), Shared] - internal class CSharpNameTupleElementCodeRefactoringProvider : AbstractNameTupleElementCodeRefactoringProvider + internal class CSharpNameTupleElementCodeRefactoringProvider : AbstractNameTupleElementCodeRefactoringProvider { [ImportingConstructor] public CSharpNameTupleElementCodeRefactoringProvider() diff --git a/src/Features/CSharp/Portable/UseNamedArguments/CSharpUseNamedArgumentsCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/UseNamedArguments/CSharpUseNamedArgumentsCodeRefactoringProvider.cs index 66f5ebf60ba1b271cff770f484775abba0ddab46..719ffe3c0e9d2327df3c19351b59036e207c3e15 100644 --- a/src/Features/CSharp/Portable/UseNamedArguments/CSharpUseNamedArgumentsCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/UseNamedArguments/CSharpUseNamedArgumentsCodeRefactoringProvider.cs @@ -15,7 +15,7 @@ namespace Microsoft.CodeAnalysis.CSharp.UseNamedArguments [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = nameof(CSharpUseNamedArgumentsCodeRefactoringProvider)), Shared] internal class CSharpUseNamedArgumentsCodeRefactoringProvider : AbstractUseNamedArgumentsCodeRefactoringProvider { - private abstract class BaseAnalyzer : Analyzer + private abstract class BaseAnalyzer : Analyzer where TSyntax : SyntaxNode where TSyntaxList : SyntaxNode { diff --git a/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs b/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs index 62ebd386127167b5b91492fbd2a9eae16899a185..36514536f1a5c5789bb6df0713c5214c6b97661b 100644 --- a/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs @@ -4,26 +4,23 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Linq; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.PooledObjects; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeRefactorings { - internal abstract class AbstractRefactoringHelpersService : IRefactoringHelpersService + internal abstract class AbstractRefactoringHelpersService : IRefactoringHelpersService where TExpressionSyntax : SyntaxNode - where TArgumentSyntax : SyntaxNode { public async Task> GetRelevantNodesAsync( Document document, TextSpan selectionRaw, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode { - var relevantNodesBuilder = ArrayBuilder.GetInstance(); - // Given selection is trimmed first to enable over-selection that spans multiple lines. Since trailing whitespace ends // at newline boundary over-selection to e.g. a line after LocalFunctionStatement would cause FindNode to find enclosing // block's Node. That is because in addition to LocalFunctionStatement the selection would also contain trailing trivia @@ -43,82 +40,91 @@ internal abstract class AbstractRefactoringHelpersService.Empty; } - // Every time a Node is considered an extractNodes method is called to add all nodes around the original one - // that should also be considered. - // - // That enables us to e.g. return node `b` when Node `var a = b;` is being considered without a complex (and potentially - // lang. & situation dependent) into Children descending code here. We can't just try extracted Node because we might - // want the whole node `var a = b;` - // - // In addition to per-node extractions we also check if current location (if selection is empty) is in a header of higher level - // desired node once. We do that only for locations because otherwise `[|int|] A { get; set; }) would trigger all refactorings for - // Property Decl. - - // Handle selections: - // - The smallest node whose FullSpan includes the whole (trimmed) selection - // - Using FullSpan is important because it handles over-selection with comments - // - Travels upwards through same-sized (FullSpan) nodes, extracting - // - Handles situations where: - // - Token with wanted Node as direct parent is selected (e.g. IdentifierToken for LocalFunctionStatement: `C [|Fun|]() {}`) - // - Most/the whole wanted Node is selected (e.g. `C [|Fun() {}|]` - // Note: Whether we have selection or location has to be checked against original selection because selecting just - // whitespace could collapse selectionTrimmed into and empty Location. But we don't want `[| |]token` - // registering as ` [||]token`. - - if (!selectionTrimmed.IsEmpty) + var relevantNodesBuilder = ArrayBuilder.GetInstance(); + try { - var selectionNode = root.FindNode(selectionTrimmed, getInnermostNodeForTie: true); - var prevNode = selectionNode; - do + // Every time a Node is considered an extractNodes method is called to add all nodes around the original one + // that should also be considered. + // + // That enables us to e.g. return node `b` when Node `var a = b;` is being considered without a complex (and potentially + // lang. & situation dependent) into Children descending code here. We can't just try extracted Node because we might + // want the whole node `var a = b;` + + // Handle selections: + // - The smallest node whose FullSpan includes the whole (trimmed) selection + // - Using FullSpan is important because it handles over-selection with comments + // - Travels upwards through same-sized (FullSpan) nodes, extracting + // - Handles situations where: + // - Token with wanted Node as direct parent is selected (e.g. IdentifierToken for LocalFunctionStatement: `C [|Fun|]() {}`) + // - Most/the whole wanted Node is selected (e.g. `C [|Fun() {}|]` + // Note: Whether we have selection or location has to be checked against original selection because selecting just + // whitespace could collapse selectionTrimmed into and empty Location. But we don't want `[| |]token` + // registering as ` [||]token`. + if (!selectionTrimmed.IsEmpty) { - var nonHiddenExtractedSelectedNodes = ExtractNodesSimple(selectionNode, syntaxFacts).OfType().Where(n => !n.OverlapsHiddenPosition(cancellationToken)); - foreach (var selectedNode in nonHiddenExtractedSelectedNodes) + AddRelevantNodesForSelection(syntaxFacts, root, selectionTrimmed, relevantNodesBuilder, cancellationToken); + } + else + { + // No more selection -> Handle what current selection is touching: + // + // Consider touching only for empty selections. Otherwise `[|C|] methodName(){}` would be considered as + // touching the Method's Node (through the left edge, see below) which is something the user probably + // didn't want since they specifically selected only the return type. + // + // What the selection is touching is used in two ways. + // - Firstly, it is used to handle situation where it touches a Token whose direct ancestor is wanted Node. + // While having the (even empty) selection inside such token or to left of such Token is already handle + // by code above touching it from right `C methodName[||](){}` isn't (the FindNode for that returns Args node). + // - Secondly, it is used for left/right edge climbing. E.g. `[||]C methodName(){}` the touching token's direct + // ancestor is TypeNode for the return type but it is still reasonable to expect that the user might want to + // be given refactorings for the whole method (as he has caret on the edge of it). Therefore we travel the + // Node tree upwards and as long as we're on the left edge of a Node's span we consider such node & potentially + // continue traveling upwards. The situation for right edge (`C methodName(){}[||]`) is analogical. + // E.g. for right edge `C methodName(){}[||]`: CloseBraceToken -> BlockSyntax -> LocalFunctionStatement -> null (higher + // node doesn't end on position anymore) + // Note: left-edge climbing needs to handle AttributeLists explicitly, see below for more information. + // - Thirdly, if location isn't touching anything, we move the location to the token in whose trivia location is in. + // more about that below. + // - Fourthly, if we're in an expression / argument we consider touching a parent expression whenever we're within it + // as long as it is on the first line of such expression (arbitrary heuristic). + + // First we need to get tokens we might potentially be touching, tokenToRightOrIn and tokenToLeft. + var (tokenToRightOrIn, tokenToLeft, location) = await GetTokensToRightOrInToLeftAndUpdatedLocation( + document, root, selectionTrimmed, cancellationToken).ConfigureAwait(false); + + + // In addition to per-node extr also check if current location (if selection is empty) is in a header of higher level + // desired node once. We do that only for locations because otherwise `[|int|] A { get; set; }) would trigger all refactorings for + // Property Decl. + // We cannot check this any sooner because the above code could've changed current location. + AddNonHiddenCorrectTypeNodes(ExtractNodesInHeader(root, location, syntaxFacts), relevantNodesBuilder, cancellationToken); + + // Add Nodes for touching tokens as described above. + AddNodesForTokenToRightOrIn(syntaxFacts, root, relevantNodesBuilder, location, tokenToRightOrIn, cancellationToken); + AddNodesForTokenToLeft(syntaxFacts, relevantNodesBuilder, location, tokenToLeft, cancellationToken); + + // If the wanted node is an expression syntax -> traverse upwards even if location is deep within a SyntaxNode. + if (typeof(TSyntaxNode).IsSubclassOf(typeof(TExpressionSyntax)) || typeof(TSyntaxNode) == typeof(TExpressionSyntax)) { - // For selections we need to handle an edge case where only AttributeLists are within selection (e.g. `Func([|[in][out]|] arg1);`). - // In that case the smallest encompassing node is still the whole argument node but it's hard to justify showing refactorings for it - // if user selected only its attributes. - - // Selection contains only AttributeLists -> don't consider current Node - var spanWithoutAttributes = GetSpanWithoutAttributes(selectedNode, root, syntaxFacts); - if (!selectionTrimmed.IntersectsWith(spanWithoutAttributes)) - { - break; - } - - relevantNodesBuilder.Add(selectedNode); + await AddNodesDeepInExpression(document, location, relevantNodesBuilder, cancellationToken).ConfigureAwait(false); } - - prevNode = selectionNode; - selectionNode = selectionNode.Parent; } - while (selectionNode != null && prevNode.FullWidth() == selectionNode.FullWidth()); - return relevantNodesBuilder.ToImmutableAndFree(); + return relevantNodesBuilder.ToImmutable(); } + finally + { + relevantNodesBuilder.Free(); + } + } - // Handle what current selection is touching: - // - // Consider touching only for empty selections. Otherwise `[|C|] methodName(){}` would be considered as - // touching the Method's Node (through the left edge, see below) which is something the user probably - // didn't want since they specifically selected only the return type. - // - // What the selection is touching is used in two ways. - // - Firstly, it is used to handle situation where it touches a Token whose direct ancestor is wanted Node. - // While having the (even empty) selection inside such token or to left of such Token is already handle - // by code above touching it from right `C methodName[||](){}` isn't (the FindNode for that returns Args node). - // - Secondly, it is used for left/right edge climbing. E.g. `[||]C methodName(){}` the touching token's direct - // ancestor is TypeNode for the return type but it is still reasonable to expect that the user might want to - // be given refactorings for the whole method (as he has caret on the edge of it). Therefore we travel the - // Node tree upwards and as long as we're on the left edge of a Node's span we consider such node & potentially - // continue traveling upwards. The situation for right edge (`C methodName(){}[||]`) is analogical. - // E.g. for right edge `C methodName(){}[||]`: CloseBraceToken -> BlockSyntax -> LocalFunctionStatement -> null (higher - // node doesn't end on position anymore) - // Note: left-edge climbing needs to handle AttributeLists explicitly, see below for more information. - // - Thirdly, if location isn't touching anything, we move the location to the token in whose trivia location is in. - // more about that below. - // - Fourthly, if we're in an expression / argument we consider touching a parent expression whenever we're within it - // as long as it is on the first line of such expression (arbitrary heuristic). - + private async Task<(SyntaxToken tokenToRightOrIn, SyntaxToken tokenToLeft, int location)> GetTokensToRightOrInToLeftAndUpdatedLocation( + Document document, + SyntaxNode root, + TextSpan selectionTrimmed, + CancellationToken cancellationToken) + { // get Token for current location var location = selectionTrimmed.Start; var tokenOnLocation = root.FindToken(location); @@ -175,14 +181,41 @@ internal abstract class AbstractRefactoringHelpersService(ISyntaxFactsService syntaxFacts, ArrayBuilder relevantNodesBuilder, int location, SyntaxToken tokenToLeft, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode + { + // there could be multiple (n) tokens to the left if first n-1 are Empty -> iterate over all of them + while (tokenToLeft != default) + { + var leftNode = tokenToLeft.Parent; + do + { + // Consider either a Node that is: + // - Ancestor Node of such Token as long as their span ends on location (it's still on the edge) + AddNonHiddenCorrectTypeNodes(ExtractNodesSimple(leftNode, syntaxFacts), relevantNodesBuilder, cancellationToken); + + leftNode = leftNode.Parent; + if (leftNode == null || !(leftNode.GetLastToken().Span.End == location || leftNode.Span.End == location)) + { + break; + } + } + while (true); + // as long as current tokenToLeft is empty -> its previous token is also tokenToLeft + tokenToLeft = tokenToLeft.Span.IsEmpty + ? tokenToLeft.GetPreviousToken(includeZeroWidth: true) + : default; + } + } + + private void AddNodesForTokenToRightOrIn(ISyntaxFactsService syntaxFacts, SyntaxNode root, ArrayBuilder relevantNodesBuilder, int location, SyntaxToken tokenToRightOrIn, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode + { if (tokenToRightOrIn != default) { - var rightNode = tokenOnLocation.Parent; + var rightNode = tokenToRightOrIn.Parent; do { // Consider either a Node that is: @@ -214,71 +247,35 @@ internal abstract class AbstractRefactoringHelpersService iterate over all of them - while (tokenToLeft != default) + private void AddRelevantNodesForSelection(ISyntaxFactsService syntaxFacts, SyntaxNode root, TextSpan selectionTrimmed, ArrayBuilder relevantNodesBuilder, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode + { + var selectionNode = root.FindNode(selectionTrimmed, getInnermostNodeForTie: true); + var prevNode = selectionNode; + do { - var leftNode = tokenToLeft.Parent; - do + var nonHiddenExtractedSelectedNodes = ExtractNodesSimple(selectionNode, syntaxFacts).OfType().Where(n => !n.OverlapsHiddenPosition(cancellationToken)); + foreach (var selectedNode in nonHiddenExtractedSelectedNodes) { - // Consider either a Node that is: - // - Ancestor Node of such Token as long as their span ends on location (it's still on the edge) - AddNonHiddenCorrectTypeNodes(ExtractNodesSimple(leftNode, syntaxFacts), relevantNodesBuilder, cancellationToken); + // For selections we need to handle an edge case where only AttributeLists are within selection (e.g. `Func([|[in][out]|] arg1);`). + // In that case the smallest encompassing node is still the whole argument node but it's hard to justify showing refactorings for it + // if user selected only its attributes. - leftNode = leftNode.Parent; - if (leftNode == null || !(leftNode.GetLastToken().Span.End == location || leftNode.Span.End == location)) + // Selection contains only AttributeLists -> don't consider current Node + var spanWithoutAttributes = GetSpanWithoutAttributes(selectedNode, root, syntaxFacts); + if (!selectionTrimmed.IntersectsWith(spanWithoutAttributes)) { break; } - } - while (true); - // as long as current tokenToLeft is empty -> its previous token is also tokenToLeft - tokenToLeft = tokenToLeft.Span.IsEmpty - ? tokenToLeft.GetPreviousToken(includeZeroWidth: true) - : default; - - } - - // If the wanted node is an expression syntax -> traverse upwards even if location is deep within a SyntaxNode - // ArgumentSyntax isn't an "expression" syntax but we want to treat it as such - if (typeof(TSyntaxNode).IsSubclassOf(typeof(TExpressionSyntax)) || typeof(TSyntaxNode) == typeof(TArgumentSyntax)) - { - await AddNodesDeepInExpression(document, location, relevantNodesBuilder, cancellationToken).ConfigureAwait(false); - } - - return relevantNodesBuilder.ToImmutableAndFree(); - - static void AddNonHiddenCorrectTypeNodes(IEnumerable nodes, ArrayBuilder resultBuilder, CancellationToken cancellationToken) - { - var correctTypeNonHiddenNodes = nodes.OfType().Where(n => !n.OverlapsHiddenPosition(cancellationToken)); - foreach (var nodeToBeAdded in correctTypeNonHiddenNodes) - { - resultBuilder.Add(nodeToBeAdded); + relevantNodesBuilder.Add(selectedNode); } - } - } - - private static TextSpan GetSpanWithoutAttributes(SyntaxNode node, SyntaxNode root, ISyntaxFactsService syntaxFacts) - { - // Span without AttributeLists - // - No AttributeLists -> original .Span - // - Some AttributeLists -> (first non-trivia/comment Token.Span.Begin, original.Span.End) - // - We need to be mindful about comments due to: - // // [Test1] - // //Comment1 - // [||]object Property1 { get; set; } - // the comment node being part of the next token's (`object`) leading trivia and not the AttributeList's node. - var attributeList = syntaxFacts.GetAttributeLists(node); - if (attributeList.Any()) - { - var endOfAttributeLists = attributeList.Last().Span.End; - var afterAttributesToken = root.FindTokenOnRightOfPosition(endOfAttributeLists); - return TextSpan.FromBounds(afterAttributesToken.Span.Start, node.Span.End); + prevNode = selectionNode; + selectionNode = selectionNode.Parent; } - - return node.Span; + while (selectionNode != null && prevNode.FullWidth() == selectionNode.FullWidth()); } /// @@ -441,5 +438,37 @@ protected virtual IEnumerable ExtractNodesInHeader(SyntaxNode root, ancestor = ancestor.Parent; } } + + private static TextSpan GetSpanWithoutAttributes(SyntaxNode node, SyntaxNode root, ISyntaxFactsService syntaxFacts) + { + // Span without AttributeLists + // - No AttributeLists -> original .Span + // - Some AttributeLists -> (first non-trivia/comment Token.Span.Begin, original.Span.End) + // - We need to be mindful about comments due to: + // // [Test1] + // //Comment1 + // [||]object Property1 { get; set; } + // the comment node being part of the next token's (`object`) leading trivia and not the AttributeList's node. + var attributeList = syntaxFacts.GetAttributeLists(node); + if (attributeList.Any()) + { + var endOfAttributeLists = attributeList.Last().Span.End; + var afterAttributesToken = root.FindTokenOnRightOfPosition(endOfAttributeLists); + + return TextSpan.FromBounds(afterAttributesToken.Span.Start, node.Span.End); + } + + return node.Span; + } + + void AddNonHiddenCorrectTypeNodes(IEnumerable nodes, ArrayBuilder resultBuilder, CancellationToken cancellationToken) + where TSyntaxNode : SyntaxNode + { + var correctTypeNonHiddenNodes = nodes.OfType().Where(n => !n.OverlapsHiddenPosition(cancellationToken)); + foreach (var nodeToBeAdded in correctTypeNonHiddenNodes) + { + resultBuilder.Add(nodeToBeAdded); + } + } } } diff --git a/src/Features/Core/Portable/CodeRefactorings/CodeRefactoringContextExtensions.cs b/src/Features/Core/Portable/CodeRefactorings/CodeRefactoringContextExtensions.cs index 284e2ed1203da9a5afb8214e42c5f24830518207..ee8f7db6c605430cbb73825163aca95fc3139e4d 100644 --- a/src/Features/Core/Portable/CodeRefactorings/CodeRefactoringContextExtensions.cs +++ b/src/Features/Core/Portable/CodeRefactorings/CodeRefactoringContextExtensions.cs @@ -8,7 +8,6 @@ using Microsoft.CodeAnalysis.CodeActions; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.CodeRefactorings { @@ -30,29 +29,28 @@ internal static class CodeRefactoringContextExtensions } } - internal static async Task TryGetSelectedNodeAsync(this CodeRefactoringContext context) + internal static Task TryGetSelectedNodeAsync(this CodeRefactoringContext context) where TSyntaxNode : SyntaxNode - { - (var document, var span, var cancellationToken) = context; - var potentialNodes = await GetRelevantNodes(document, span, cancellationToken).ConfigureAwait(false); - return potentialNodes.FirstOrDefault(); - } + => TryGetRelevantNodeAsync(context.Document, context.Span, context.CancellationToken); - internal static async Task TryGetSelectedNodeAsync(this Document document, TextSpan span, Func predicate, CancellationToken cancellationToken) -where TSyntaxNode : SyntaxNode - { - var potentialNodes = await GetRelevantNodes(document, span, cancellationToken).ConfigureAwait(false); - return potentialNodes.FirstOrDefault(predicate); - } + internal static Task> TryGetSelectedNodesAsync(this CodeRefactoringContext context) + where TSyntaxNode : SyntaxNode + => TryGetRelevantNodesAsync(context.Document, context.Span, context.CancellationToken); - internal static async Task TryGetSelectedNodeAsync(this Document document, TextSpan span, CancellationToken cancellationToken) -where TSyntaxNode : SyntaxNode + internal static async Task TryGetRelevantNodeAsync( + this Document document, + TextSpan span, + CancellationToken cancellationToken) + where TSyntaxNode : SyntaxNode { - var potentialNodes = await GetRelevantNodes(document, span, cancellationToken).ConfigureAwait(false); + var potentialNodes = await TryGetRelevantNodesAsync(document, span, cancellationToken).ConfigureAwait(false); return potentialNodes.FirstOrDefault(); } - private static async Task> GetRelevantNodes(Document document, TextSpan span, CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode + internal static async Task> TryGetRelevantNodesAsync( + this Document document, + TextSpan span, + CancellationToken cancellationToken) where TSyntaxNode : SyntaxNode { var helpers = document.GetLanguageService(); var potentialNodes = await helpers.GetRelevantNodesAsync(document, span, cancellationToken).ConfigureAwait(false); diff --git a/src/Features/Core/Portable/ConvertAnonymousTypeToClass/AbstractConvertAnonymousTypeToClassCodeRefactoringProvider.cs b/src/Features/Core/Portable/ConvertAnonymousTypeToClass/AbstractConvertAnonymousTypeToClassCodeRefactoringProvider.cs index fd07c0242838523f72ab87a28c4b32c552e84236..3d53a068c2a6e3164334713c5a881c5b3eace7bd 100644 --- a/src/Features/Core/Portable/ConvertAnonymousTypeToClass/AbstractConvertAnonymousTypeToClassCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/ConvertAnonymousTypeToClass/AbstractConvertAnonymousTypeToClassCodeRefactoringProvider.cs @@ -71,8 +71,8 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte // Due to the way `TryGetSelectedNodeAsync` works and how `TAnonymousObjectCreationExpressionSyntax` is e.g. for C# constructed // it matches even when caret is next to some tokens within the anonymous object creation node. // E.g.: `var a = new [||]{ b=1,[||] c=2 };` both match due to the caret being next to `,` and `{`. - var helper = document.GetLanguageService(); - var anonymousObject = await document.TryGetSelectedNodeAsync(span, cancellationToken).ConfigureAwait(false); + var anonymousObject = await document.TryGetRelevantNodeAsync( + span, cancellationToken).ConfigureAwait(false); if (anonymousObject == null) { return default; diff --git a/src/Features/Core/Portable/IntroduceUsingStatement/AbstractIntroduceUsingStatementCodeRefactoringProvider.cs b/src/Features/Core/Portable/IntroduceUsingStatement/AbstractIntroduceUsingStatementCodeRefactoringProvider.cs index e3c530216522d1ac145e3cae835fdbc07825ef6e..706e6ffc4a9837bd51baafcac5f91dd8b43e02ee 100644 --- a/src/Features/Core/Portable/IntroduceUsingStatement/AbstractIntroduceUsingStatementCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/IntroduceUsingStatement/AbstractIntroduceUsingStatementCodeRefactoringProvider.cs @@ -46,9 +46,7 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte private async Task FindDisposableLocalDeclaration(Document document, TextSpan selection, CancellationToken cancellationToken) { - var refactoringHelperService = document.GetLanguageService(); - var declarationSyntax = await document.TryGetSelectedNodeAsync(selection, cancellationToken).ConfigureAwait(false); - + var declarationSyntax = await document.TryGetRelevantNodeAsync(selection, cancellationToken).ConfigureAwait(false); if (declarationSyntax is null || !CanRefactorToContainBlockStatements(declarationSyntax.Parent)) { return default; diff --git a/src/Features/Core/Portable/NameTupleElement/AbstractNameTupleElementCodeRefactoringProvider.cs b/src/Features/Core/Portable/NameTupleElement/AbstractNameTupleElementCodeRefactoringProvider.cs index 223a123abb5845fdac810f3da1a2f6abbea8afc7..a19f47e18dc006192f2abfca3d289f1c1b5aea1d 100644 --- a/src/Features/Core/Portable/NameTupleElement/AbstractNameTupleElementCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/NameTupleElement/AbstractNameTupleElementCodeRefactoringProvider.cs @@ -9,13 +9,13 @@ using Microsoft.CodeAnalysis.LanguageServices; using Microsoft.CodeAnalysis.Shared.Extensions; using Microsoft.CodeAnalysis.Text; -using Roslyn.Utilities; namespace Microsoft.CodeAnalysis.NameTupleElement { - abstract class AbstractNameTupleElementCodeRefactoringProvider : CodeRefactoringProvider + abstract class AbstractNameTupleElementCodeRefactoringProvider : CodeRefactoringProvider where TArgumentSyntax : SyntaxNode - where TTupleExpressionSyntax : SyntaxNode + where TExpresionSyntax : SyntaxNode + where TTupleExpressionSyntax : TExpresionSyntax { protected abstract bool IsCloseParenOrComma(SyntaxToken token); protected abstract TArgumentSyntax WithName(TArgumentSyntax argument, string argumentName); @@ -44,11 +44,10 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte return default; } - var helperService = document.GetLanguageService(); var syntaxFacts = document.GetLanguageService(); - - Func isTupleArgumentExpression = n => n.Parent != null && syntaxFacts.IsTupleExpression(n.Parent); - var argument = await document.TryGetSelectedNodeAsync(span, isTupleArgumentExpression, cancellationToken).ConfigureAwait(false); + var expressions = await document.TryGetRelevantNodesAsync(span, cancellationToken).ConfigureAwait(false); + var argument = expressions.FirstOrDefault( + n => n.Parent is TArgumentSyntax && n.Parent.Parent is TTupleExpressionSyntax)?.Parent as TArgumentSyntax; if (argument == null || !syntaxFacts.IsSimpleArgument(argument)) { return default; diff --git a/src/Features/Core/Portable/UseNamedArguments/AbstractUseNamedArgumentsCodeRefactoringProvider.cs b/src/Features/Core/Portable/UseNamedArguments/AbstractUseNamedArgumentsCodeRefactoringProvider.cs index 6d5539342e995d461336f28fdfd5ff2ee8e4515f..d8f67906ae5d8280b601f32fc2c93de00ee3fcef 100644 --- a/src/Features/Core/Portable/UseNamedArguments/AbstractUseNamedArgumentsCodeRefactoringProvider.cs +++ b/src/Features/Core/Portable/UseNamedArguments/AbstractUseNamedArgumentsCodeRefactoringProvider.cs @@ -20,17 +20,19 @@ protected interface IAnalyzer Task ComputeRefactoringsAsync(CodeRefactoringContext context, SyntaxNode root); } - protected abstract class Analyzer : IAnalyzer + protected abstract class Analyzer : IAnalyzer where TBaseArgumentSyntax : SyntaxNode where TSimpleArgumentSyntax : TBaseArgumentSyntax where TArgumentListSyntax : SyntaxNode + where TExpressionSyntax : SyntaxNode { public async Task ComputeRefactoringsAsync( CodeRefactoringContext context, SyntaxNode root) { var (document, textSpan, cancellationToken) = context; - var argument = await context.TryGetSelectedNodeAsync().ConfigureAwait(false); + var expressions = await context.TryGetSelectedNodesAsync().ConfigureAwait(false); + var argument = expressions.FirstOrDefault(n => n is TExpressionSyntax)?.Parent as TSimpleArgumentSyntax; if (argument == null) { return; diff --git a/src/Features/VisualBasic/Portable/CodeRefactorings/VisualBasicRefactoringHelpersService.vb b/src/Features/VisualBasic/Portable/CodeRefactorings/VisualBasicRefactoringHelpersService.vb index e33e0cf1c006dccefb4c3c4b9042592496955a6c..f02f9e396c40f0aff9a7f24c4b18f8940f6858ba 100644 --- a/src/Features/VisualBasic/Portable/CodeRefactorings/VisualBasicRefactoringHelpersService.vb +++ b/src/Features/VisualBasic/Portable/CodeRefactorings/VisualBasicRefactoringHelpersService.vb @@ -9,7 +9,7 @@ Imports Microsoft.CodeAnalysis.LanguageServices Namespace Microsoft.CodeAnalysis.VisualBasic.CodeRefactorings Friend Class VisualBasicRefactoringHelpersService - Inherits AbstractRefactoringHelpersService(Of ExpressionSyntax, ArgumentSyntax) + Inherits AbstractRefactoringHelpersService(Of ExpressionSyntax) Protected Overrides Iterator Function ExtractNodesSimple(node As SyntaxNode, syntaxFacts As ISyntaxFactsService) As IEnumerable(Of SyntaxNode) For Each baseExtraction In MyBase.ExtractNodesSimple(node, syntaxFacts) diff --git a/src/Features/VisualBasic/Portable/NameTupleElement/VisualBasicNameTupleElementCodeRefactoringProvider.vb b/src/Features/VisualBasic/Portable/NameTupleElement/VisualBasicNameTupleElementCodeRefactoringProvider.vb index e420b534f5ef836de038b89e4af8c7dc7813e08f..08eba57cf88f7f16c0464dd4175c59a7db54137d 100644 --- a/src/Features/VisualBasic/Portable/NameTupleElement/VisualBasicNameTupleElementCodeRefactoringProvider.vb +++ b/src/Features/VisualBasic/Portable/NameTupleElement/VisualBasicNameTupleElementCodeRefactoringProvider.vb @@ -9,7 +9,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.NameTupleElement Friend Class VisualBasicNameTupleElementCodeRefactoringProvider - Inherits AbstractNameTupleElementCodeRefactoringProvider(Of SimpleArgumentSyntax, TupleExpressionSyntax) + Inherits AbstractNameTupleElementCodeRefactoringProvider(Of SimpleArgumentSyntax, TupleExpressionSyntax, ExpressionSyntax) Public Sub New() diff --git a/src/Features/VisualBasic/Portable/UseNamedArguments/VisualBasicUseNamedArgumentsCodeRefactoringProvider.vb b/src/Features/VisualBasic/Portable/UseNamedArguments/VisualBasicUseNamedArgumentsCodeRefactoringProvider.vb index daf41d5f9a749286b7265c471767c33de0fb955d..73d51fd9bae0db92c2902a2e4a625812b35383ac 100644 --- a/src/Features/VisualBasic/Portable/UseNamedArguments/VisualBasicUseNamedArgumentsCodeRefactoringProvider.vb +++ b/src/Features/VisualBasic/Portable/UseNamedArguments/VisualBasicUseNamedArgumentsCodeRefactoringProvider.vb @@ -14,7 +14,7 @@ Namespace Microsoft.CodeAnalysis.VisualBasic.UseNamedArguments Inherits AbstractUseNamedArgumentsCodeRefactoringProvider Private Class ArgumentAnalyzer - Inherits Analyzer(Of ArgumentSyntax, SimpleArgumentSyntax, ArgumentListSyntax) + Inherits Analyzer(Of ArgumentSyntax, SimpleArgumentSyntax, ArgumentListSyntax, ExpressionSyntax) Protected Overrides Function IsPositionalArgument(argument As SimpleArgumentSyntax) As Boolean Return argument.NameColonEquals Is Nothing