From 17a30872a494a1782e6281b5ba28b566b297641c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Hou=C5=A1ka?= Date: Fri, 28 Jun 2019 14:28:37 -0700 Subject: [PATCH] LambdaSimplifier moved to helper + improved unwrapper. --- .../LambdaSimplifier/LambdaSimplifierTests.cs | 45 +++++++++++++++++++ ...LambdaSimplifierCodeRefactoringProvider.cs | 7 +-- .../AbstractRefactoringHelpersService.cs | 44 +++++++++++------- .../CSharpSyntaxFactsService.cs | 2 + .../SyntaxFactsService/ISyntaxFactsService.cs | 1 + .../VisualBasicSyntaxFactsService.vb | 9 ++++ 6 files changed, 88 insertions(+), 20 deletions(-) diff --git a/src/EditorFeatures/CSharpTest/CodeActions/LambdaSimplifier/LambdaSimplifierTests.cs b/src/EditorFeatures/CSharpTest/CodeActions/LambdaSimplifier/LambdaSimplifierTests.cs index 2ceea8f85ad..a7e9895ecc2 100644 --- a/src/EditorFeatures/CSharpTest/CodeActions/LambdaSimplifier/LambdaSimplifierTests.cs +++ b/src/EditorFeatures/CSharpTest/CodeActions/LambdaSimplifier/LambdaSimplifierTests.cs @@ -557,6 +557,51 @@ static void Main() }", @"using System; +class Program +{ + static void Main() + { + Action a = Console.WriteLine; + } +}"); + } + + [WorkItem(35180, "https://github.com/dotnet/roslyn/issues/35180")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsLambdaSimplifier)] + public async Task TestMissingCaretPositionInside() + { + await TestMissingInRegularAndScriptAsync( +@"using System; + +class Program +{ + static void Main() + { + Action a = () => { + Console.[||]WriteLine(); + }; + } +}"); + } + + [WorkItem(35180, "https://github.com/dotnet/roslyn/issues/35180")] + [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsLambdaSimplifier)] + public async Task TestCaretPosition() + { + await TestInRegularAndScriptAsync( +@"using System; + +class Program +{ + static void Main() + { + [|Action a = () => { + Console.WriteLine(); + };|] + } +}", +@"using System; + class Program { static void Main() diff --git a/src/Features/CSharp/Portable/CodeRefactorings/LambdaSimplifier/LambdaSimplifierCodeRefactoringProvider.cs b/src/Features/CSharp/Portable/CodeRefactorings/LambdaSimplifier/LambdaSimplifierCodeRefactoringProvider.cs index eb6604f1a4b..ad55e3e9cdc 100644 --- a/src/Features/CSharp/Portable/CodeRefactorings/LambdaSimplifier/LambdaSimplifierCodeRefactoringProvider.cs +++ b/src/Features/CSharp/Portable/CodeRefactorings/LambdaSimplifier/LambdaSimplifierCodeRefactoringProvider.cs @@ -14,6 +14,7 @@ namespace Microsoft.CodeAnalysis.CSharp.CodeRefactorings.LambdaSimplifier { + // Disabled due to: https://github.com/dotnet/roslyn/issues/5835 & https://github.com/dotnet/roslyn/pull/6642 // [ExportCodeRefactoringProvider(LanguageNames.CSharp, Name = PredefinedCodeRefactoringProviderNames.SimplifyLambda)] internal partial class LambdaSimplifierCodeRefactoringProvider : CodeRefactoringProvider { @@ -35,9 +36,9 @@ public override async Task ComputeRefactoringsAsync(CodeRefactoringContext conte var semanticDocument = await SemanticDocument.CreateAsync(document, cancellationToken).ConfigureAwait(false); - var lambda = semanticDocument.Root.FindToken(textSpan.Start).GetAncestor(n => - n is SimpleLambdaExpressionSyntax || n is ParenthesizedLambdaExpressionSyntax); - if (lambda == null || !lambda.Span.IntersectsWith(textSpan.Start)) + var refactoringHelperService = document.GetLanguageService(); + var lambda = await refactoringHelperService.TryGetSelectedNodeAsync(document, context.Span, cancellationToken).ConfigureAwait(false); + if (lambda == null) { return; } diff --git a/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs b/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs index 8302f7caf4d..24ea6d13ac2 100644 --- a/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs +++ b/src/Features/Core/Portable/CodeRefactorings/AbstractRefactoringHelpersService.cs @@ -30,7 +30,7 @@ internal abstract class AbstractRefactoringHelpersService : IRefactoringHelpersS /// - Whole node passing is selected. /// /// - /// Attempts extracting (and testing with a Node for each Node it consideres (see above). + /// Attempts extracting (and testing with a Node for each Node it considers (see above). /// By default extracts initializer expressions from declarations and assignments. /// /// @@ -56,7 +56,7 @@ protected Task TryGetSelectedNodeAsync(Document document, TextSpan s /// /// /// The enables testing with and potentially returning Nodes - /// that are under those that might be selected / considered (as described above). It is a that + /// that are under/above those that might be selected / considered (as described above). It is a that /// should always return either given Node or a Node somewhere below it that should be tested with and /// potentially returned instead of current Node. /// @@ -68,8 +68,8 @@ protected Task TryGetSelectedNodeAsync(Document document, TextSpan s /// protected async Task TryGetSelectedNodeAsync(Document document, TextSpan selection, Predicate predicate, Func extractNode, CancellationToken cancellationToken) { - // Given selection is trimmed first to enable overselection that spans multiple lines. Since trailing whitespace ends - // at newline boundary overselection to e.g. a line after LocalFunctionStatement would cause FindNode to find enclosing + // 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 // (whitespace) of following statement. @@ -77,23 +77,23 @@ protected async Task TryGetSelectedNodeAsync(Document document, Text var root = await document.GetSyntaxRootAsync(cancellationToken).ConfigureAwait(false); var selectionTrimmed = await CodeRefactoringHelpers.GetTrimmedTextSpan(document, selection, cancellationToken).ConfigureAwait(false); - // Everytime a Node is considered by following algorithm (and tested with predicate) and the predicate fails + // Every time a Node is considered by following algorithm (and tested with predicate) and the predicate fails // extractNode is called on the node and the result is tested with predicate again. If any of those succeed // a respective Node gets returned. // // That enables us to e.g. return node `b` when Node `var a = b;` is being considered without a complex (and potentially - // lang. & situation dependant) into Children descending code here. We can't just try extracted Node because we might + // 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;` // // See local function TryGetAcceptedNodeOrExtracted DefaultNodeExtractor for more info. // Handle selections: - // - The smallest node whose FullSpan inlcudes the whole (trimmed) selection + // - The smallest node whose FullSpan includes the whole (trimmed) selection // - Travels upwards through same-sized (FullSpan) nodes, extracting and testing predicate // - 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 seleted (e.g. `C [|Fun() {}|]` + // - Most/the whole wanted Node is selected (e.g. `C [|Fun() {}|]` var node = root.FindNode(selectionTrimmed, getInnermostNodeForTie: true); SyntaxNode prevNode; do @@ -120,10 +120,10 @@ protected async Task TryGetSelectedNodeAsync(Document document, Text // 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 resonable to expect that the user might want to + // 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 travelling upwards. The situation for right edge (`C methodName(){}[||]`) is analogical. + // 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) if (!selection.IsEmpty) @@ -215,11 +215,15 @@ static SyntaxNode TryGetAcceptedNodeOrExtracted(SyntaxNode node, Predicate /// - /// Extractor function for methods - /// that retrieves expressions from declarations and assignments. Otherwise returns unchanged . + /// Extractor function for method + /// that retrieves nodes that should also be considered as refactoring targets given is considered. + /// Can extract both nodes above and under given . /// /// - /// The rationale is that when user selects e.g. entire local delaration statement [|var a = b;|] it is reasonable + /// E.g. extracts expressions from assignment statement (`a = b;` -> `b`) or lambda node for its body. + /// + /// + /// The rationale is that when user selects e.g. entire local declaration statement [|var a = b;|] it is reasonable /// to provide refactoring for `b` node. Similarly for other types of refactorings. /// /// @@ -230,8 +234,8 @@ protected virtual SyntaxNode DefaultNodeExtractor(SyntaxNode node, ISyntaxFactsS // that were found to be relevant for refactorings that were moved to `TryGetSelectedNodeAsync`. // Feel free to extend it / refine current heuristics. - // var a = b; - // -> b + // `var a = b`; + // -> `b` if (syntaxFacts.IsLocalDeclarationStatement(node)) { var variables = syntaxFacts.GetVariablesOfLocalDeclarationStatement(node); @@ -251,14 +255,20 @@ protected virtual SyntaxNode DefaultNodeExtractor(SyntaxNode node, ISyntaxFactsS } } - // a = b; - // -> b + // `a = b;` + // -> `b` if (syntaxFacts.IsSimpleAssignmentStatement(node)) { syntaxFacts.GetPartsOfAssignmentExpressionOrStatement(node, out _, out _, out var rightSide); return rightSide; } + // a => `b`; -> `a => b;` + if (syntaxFacts.IsLambdaBody(node)) + { + return node.Parent; + } + return node; } } diff --git a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs index d9789624ffc..08442086857 100644 --- a/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs +++ b/src/Workspaces/CSharp/Portable/LanguageServices/CSharpSyntaxFactsService.cs @@ -1950,5 +1950,7 @@ public void GetPartsOfCastExpression(SyntaxNode node, out SyntaxNode type, out S return null; } + + public bool IsLambdaBody(SyntaxNode node) => node.Parent != null && node.Parent is LambdaExpressionSyntax lambdaNode && lambdaNode.Body == node; } } diff --git a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs index 9b12ae3272b..1cc1cb1d90b 100644 --- a/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs +++ b/src/Workspaces/Core/Portable/LanguageServices/SyntaxFactsService/ISyntaxFactsService.cs @@ -264,6 +264,7 @@ internal interface ISyntaxFactsService : ILanguageService bool IsVariableDeclarator(SyntaxNode node); bool IsDeconstructionAssignment(SyntaxNode node); bool IsDeconstructionForEachStatement(SyntaxNode node); + bool IsLambdaBody(SyntaxNode node); /// /// Returns true for nodes that represent the body of a method. diff --git a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb index d932bba4fa7..76c41843ce9 100644 --- a/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb +++ b/src/Workspaces/VisualBasic/Portable/LanguageServices/VisualBasicSyntaxFactsService.vb @@ -1944,5 +1944,14 @@ Namespace Microsoft.CodeAnalysis.VisualBasic Public Shadows Function SpansPreprocessorDirective(tokens As IEnumerable(Of SyntaxToken)) As Boolean Return MyBase.SpansPreprocessorDirective(tokens) End Function + + Public Function IsLambdaBody(node As SyntaxNode) As Boolean Implements ISyntaxFactsService.IsLambdaBody + If node.Parent IsNot Nothing And TypeOf node.Parent Is LambdaExpressionSyntax Then + Dim lambdaNode = CType(node.Parent, LambdaExpressionSyntax) + Dim lambdaBodies = lambdaNode.GetBodies() + Return lambdaBodies.Count() = 1 And lambdaBodies.First Is node + End If + Return False + End Function End Class End Namespace -- GitLab