diff --git a/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs b/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs index a63016862cd6f9f3d18a41fe314b1fd8dbc0c183..e5ee5ff9cba4c8e48d2d23d4ca8accf33ecc2f0c 100644 --- a/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs @@ -40,85 +40,79 @@ internal abstract class AbstractRefactoringHelpersService.Empty; } - var relevantNodesBuilder = ArrayBuilder.GetInstance(); - try + using var relevantNodesBuilderDisposer = ArrayBuilder.GetInstance(out var relevantNodesBuilder); + + // 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: + // - Most/the whole wanted Node is selected (e.g. `C [|Fun() {}|]` + // - 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 + // - Token with wanted Node as direct parent is selected (e.g. IdentifierToken for LocalFunctionStatement: `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) { - // Every time a Node is considered an extractNodes method is called to add all nodes around the original one - // that should also be considered. + 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. // - // 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: - // - Most/the whole wanted Node is selected (e.g. `C [|Fun() {}|]` - // - 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 - // - Token with wanted Node as direct parent is selected (e.g. IdentifierToken for LocalFunctionStatement: `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) + // 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. + // We want to treat more types like expressions, e.g.: ArgumentSyntax should still trigger even if deep-in. + if (IsWantedTypeExpressionLike()) { - AddRelevantNodesForSelection(syntaxFacts, root, selectionTrimmed, relevantNodesBuilder, cancellationToken); + // Reason to treat Arguments (and potentially others) as Expression-like: + // https://github.com/dotnet/roslyn/pull/37295#issuecomment-516145904 + await AddNodesDeepIn(document, location, relevantNodesBuilder, cancellationToken).ConfigureAwait(false); } - 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. - // We want to treat more types like expressions, e.g.: ArgumentSyntax should still trigger even if deep-in. - if (IsWantedTypeExpressionLike()) - { - // Reason to treat Arguments (and potentially others) as Expression-like: - // https://github.com/dotnet/roslyn/pull/37295#issuecomment-516145904 - await AddNodesDeepIn(document, location, relevantNodesBuilder, cancellationToken).ConfigureAwait(false); - } - } - - return relevantNodesBuilder.ToImmutable(); - } - finally - { - relevantNodesBuilder.Free(); } + + return relevantNodesBuilder.ToImmutable(); } private static bool IsWantedTypeExpressionLike() where TSyntaxNode : SyntaxNode