Repository navigation
Add a refactoring between fun x -> x.Prop and _.Prop
#20535
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
xperiandri
wants to merge
11
commits into
dotnet:main
Choose a base branch
from
xperiandri:feature/dot-lambda-refactoring
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+548
−37
Open
Changes from all commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
92a3eac
Add a refactoring between 'fun x -> x.Prop' and the '_.Prop' shorthand
xperiandri 3427c98
Write the refactoring test code as multi-line strings
xperiandri 0cbf251
Link the release note to the pull request and move it to a random lin…
xperiandri 064a97f
Share the _. body chain walk between pushUnaryArg and its inverse
xperiandri df3974c
Ask the compiler which lambda bodies _. can express, and count assign…
xperiandri b15bd37
Withhold the _. conversions that lose a comment, capture a value or b…
xperiandri e18b128
Remove the duplicated release note
xperiandri e47c07c
Name the dot-lambda conversion helpers instead of documenting them
xperiandri 12d14fd
Fold the standalone dot-lambda conversion tests into the tables
xperiandri b5496e8
Keep one compact Visual Studio release note for the dot-lambda refact…
xperiandri da62d5d
Merge branch 'main' into feature/dot-lambda-refactoring
abonie File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| module FSharp.Compiler.Service.Tests.SyntaxTreeOpsTests | ||
|
|
||
| open FSharp.Compiler.Service.Tests.Common | ||
| open FSharp.Compiler.Syntax | ||
| open FSharp.Compiler.SyntaxTreeOps | ||
| open Xunit | ||
|
|
||
| let private exprOf source = | ||
| match getParseResults source with | ||
| | ParsedInput.ImplFile(ParsedImplFileInput(contents = [ SynModuleOrNamespace(decls = [ SynModuleDecl.Expr(expr = expr) ]) ])) -> expr | ||
| | tree -> failwith $"Expected a single expression, got %A{tree}" | ||
|
|
||
| [<Theory>] | ||
| [<InlineData("x.P")>] | ||
| [<InlineData("x.A.B")>] | ||
| [<InlineData("x.M(y)")>] | ||
| [<InlineData("x.Xs[0]")>] | ||
| [<InlineData("x.Xs.[0]")>] | ||
| [<InlineData("x.M<int>()")>] | ||
| [<InlineData("x.M().P")>] | ||
| let ``tryPopUnaryArg splits the root off a chain that pushUnaryArg restores`` (source: string) = | ||
| let body = exprOf source | ||
|
|
||
| match tryPopUnaryArg body with | ||
| | ValueSome(struct (root, popped)) -> | ||
| Assert.Equal("x", root.idText) | ||
| Assert.Equal($"%A{body}", $"%A{pushUnaryArg popped root}") | ||
| | ValueNone -> Assert.Fail $"Expected {source} to have a root" | ||
|
|
||
| [<Theory>] | ||
| [<InlineData("f x.P")>] | ||
| [<InlineData("x.M y")>] | ||
| [<InlineData("x.P + 1")>] | ||
| [<InlineData("x")>] | ||
| [<InlineData("x[0]")>] | ||
| [<InlineData("(x.P)")>] | ||
| [<InlineData("x.")>] | ||
| let ``tryPopUnaryArg rejects what _. cannot express`` (source: string) = | ||
| Assert.True((tryPopUnaryArg (exprOf source)).IsNone) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
207 changes: 207 additions & 0 deletions
207
vsintegration/src/FSharp.Editor/Refactor/ConvertDotLambda.fs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,207 @@ | ||
| // Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. | ||
|
|
||
| namespace Microsoft.VisualStudio.FSharp.Editor | ||
|
|
||
| open System | ||
| open System.Composition | ||
|
|
||
| open Microsoft.CodeAnalysis.CodeActions | ||
| open Microsoft.CodeAnalysis.CodeRefactorings | ||
| open Microsoft.CodeAnalysis.Text | ||
| open Microsoft.VisualStudio.FSharp.Editor.Telemetry | ||
|
|
||
| open FSharp.Compiler.Syntax | ||
| open FSharp.Compiler.SyntaxTreeOps | ||
| open FSharp.Compiler.SyntaxTrivia | ||
| open FSharp.Compiler.Text | ||
|
|
||
| open CancellableTasks | ||
|
|
||
| [<RequireQualifiedAccess>] | ||
| module private DotLambdaConversion = | ||
|
|
||
| [<NoComparison; NoEquality>] | ||
| type Conversion = | ||
| { | ||
| Title: string | ||
| Direction: string | ||
| Change: TextChange | ||
| } | ||
|
|
||
| let hasName (name: string) (ident: Ident) = | ||
| String.Equals(ident.idText, name, StringComparison.Ordinal) | ||
|
|
||
| let identifiersIn (body: SynExpr) = | ||
| ([], [ SyntaxNode.SynExpr body ]) | ||
| ||> SyntaxNodes.fold (fun identifiers _ node -> | ||
| match node with | ||
| | SyntaxNode.SynExpr(SynExpr.Ident ident) | ||
| | SyntaxNode.SynExpr(SynExpr.LongIdent(longDotId = SynLongIdent(id = ident :: _))) | ||
| | SyntaxNode.SynExpr(SynExpr.LongIdentSet(longDotId = SynLongIdent(id = ident :: _))) | ||
| | SyntaxNode.SynExpr(SynExpr.NamedIndexedPropertySet(longDotId = SynLongIdent(id = ident :: _))) | ||
| | SyntaxNode.SynPat(SynPat.Named(ident = SynIdent(ident, _))) -> ident :: identifiers | ||
| | _ -> identifiers) | ||
|
|
||
| let parameterNameFor (body: SynExpr) = | ||
| let used = identifiersIn body | ||
|
|
||
| Seq.initInfinite (fun i -> if i = 0 then "x" else $"x{i}") | ||
| |> Seq.find (fun name -> not (List.exists (hasName name) used)) | ||
|
|
||
| let canBeCapturedByGeneratedArgument (ident: Ident) = | ||
| let prefix = "_arg" | ||
| let name = ident.idText | ||
|
|
||
| let rec isDigitsFrom i = | ||
| i = name.Length || (Char.IsDigit name[i] && isDigitsFrom (i + 1)) | ||
|
|
||
| name.Length > prefix.Length | ||
| && name.StartsWith(prefix, StringComparison.Ordinal) | ||
| && isDigitsFrom prefix.Length | ||
|
|
||
| let hasCommentIn (comments: CommentTrivia list) (m: range) = | ||
| comments | ||
| |> List.exists (fun (CommentTrivia.LineComment comment | CommentTrivia.BlockComment comment) -> Range.rangeContainsRange m comment) | ||
|
|
||
| let rec lastOffsideSensitiveLine (expr: SynExpr) = | ||
| match expr with | ||
| | SynExpr.App(flag = ExprAtomicFlag.Atomic; funcExpr = func) -> lastOffsideSensitiveLine func | ||
| | SynExpr.DotGet(longDotId = longDotId) -> longDotId.Range.EndLine | ||
| | SynExpr.DotIndexedGet(dotRange = dot) -> dot.EndLine | ||
| | _ -> expr.Range.EndLine | ||
|
|
||
| let isInQuotation (path: SyntaxVisitorPath) = | ||
| path | ||
| |> List.exists (function | ||
| | SyntaxNode.SynExpr(SynExpr.Quote _) -> true | ||
| | _ -> false) | ||
|
|
||
| let rec isAppliedDirectly (expr: SynExpr) (path: SyntaxVisitorPath) = | ||
| match path with | ||
| | SyntaxNode.SynExpr(SynExpr.Paren _ as paren) :: outer -> isAppliedDirectly paren outer | ||
| | SyntaxNode.SynExpr(SynExpr.App(funcExpr = func)) :: _ -> obj.ReferenceEquals(func, expr) | ||
| | _ -> false | ||
|
|
||
| let isBlankBetween (sourceText: SourceText) start finish = | ||
| start <= finish | ||
| && String.IsNullOrWhiteSpace(sourceText.ToString(TextSpan.FromBounds(start, finish))) | ||
|
|
||
| [<return: Struct>] | ||
| let (|SoleParenthesizedLambdaArgument|_|) (sourceText: SourceText) (lambda: SynExpr) (lambdaSpan: TextSpan) (path: SyntaxVisitorPath) = | ||
| match path with | ||
| | SyntaxNode.SynExpr(SynExpr.Paren(expr = inner; leftParenRange = leftParen; rightParenRange = Some rightParen; range = parenRange) as paren) :: SyntaxNode.SynExpr(SynExpr.App( | ||
| flag = ExprAtomicFlag.NonAtomic; isInfix = false; argExpr = arg)) :: _ when | ||
| obj.ReferenceEquals(inner, lambda) | ||
| && obj.ReferenceEquals(arg, paren) | ||
| && isBlankBetween sourceText (RoslynHelpers.FSharpRangeToTextSpan(sourceText, leftParen)).End lambdaSpan.Start | ||
| && isBlankBetween sourceText lambdaSpan.End (RoslynHelpers.FSharpRangeToTextSpan(sourceText, rightParen)).Start | ||
| -> | ||
| ValueSome(RoslynHelpers.FSharpRangeToTextSpan(sourceText, parenRange)) | ||
| | _ -> ValueNone | ||
|
|
||
| let toShorthand (sourceText: SourceText) (path: SyntaxVisitorPath) (lambda: SynExpr) (root: Ident) = | ||
| let lambdaSpan = RoslynHelpers.FSharpRangeToTextSpan(sourceText, lambda.Range) | ||
| let rootEnd = RoslynHelpers.FSharpRangeToTextSpan(sourceText, root.idRange).End | ||
|
|
||
| match path with | ||
| | SoleParenthesizedLambdaArgument sourceText lambda lambdaSpan parenSpan -> | ||
| let chain = sourceText.ToString(TextSpan.FromBounds(rootEnd, lambdaSpan.End)) | ||
| TextChange(parenSpan, $"_{chain}") | ||
| | _ -> TextChange(TextSpan.FromBounds(lambdaSpan.Start, rootEnd), "_") | ||
|
|
||
| let toLambda (sourceText: SourceText) (path: SyntaxVisitorPath) (body: SynExpr) (range: range) (trivia: SynExprDotLambdaTrivia) = | ||
| let name = parameterNameFor body | ||
| let lambda = $"fun {name} -> {name}" | ||
|
|
||
| let underscore = | ||
| RoslynHelpers.FSharpRangeToTextSpan(sourceText, trivia.UnderscoreRange) | ||
|
|
||
| match path with | ||
| | SyntaxNode.SynExpr(SynExpr.Paren _) :: _ | ||
| | SyntaxNode.SynBinding _ :: _ -> TextChange(underscore, lambda) | ||
| | _ -> | ||
| let span = RoslynHelpers.FSharpRangeToTextSpan(sourceText, range) | ||
| let chain = sourceText.ToString(TextSpan.FromBounds(underscore.End, span.End)) | ||
| TextChange(span, $"({lambda}{chain})") | ||
|
|
||
|
abonie marked this conversation as resolved.
|
||
| let tryFind (sourceText: SourceText) (position: pos) (parseTree: ParsedInput) = | ||
| let comments = | ||
| match parseTree with | ||
| | ParsedInput.ImplFile file -> file.Trivia.CodeComments | ||
| | ParsedInput.SigFile file -> file.Trivia.CodeComments | ||
|
|
||
| (position, parseTree) | ||
| ||> ParsedInput.tryPickLast (fun path node -> | ||
| match node with | ||
| | SyntaxNode.SynExpr(SynExpr.Lambda( | ||
| fromMethod = false | ||
| parsedData = Some([ SynPat.Named(ident = SynIdent(parameter, _); isThisVal = false; accessibility = None) ], body)) as lambda) when | ||
| not (isInQuotation path) && not (isAppliedDirectly lambda path) | ||
| -> | ||
| let identifiers = identifiersIn body | ||
|
|
||
| match tryPopUnaryArg body with | ||
| | ValueSome(struct (root, _)) when | ||
| hasName parameter.idText root | ||
| && (identifiers |> List.filter (hasName parameter.idText) |> List.length) = 1 | ||
| && not (List.exists canBeCapturedByGeneratedArgument identifiers) | ||
| && not (hasCommentIn comments (Range.withEnd root.idRange.End lambda.Range)) | ||
| -> | ||
|
abonie marked this conversation as resolved.
|
||
| Some | ||
| { | ||
| Title = SR.ConvertToShorthandLambda() | ||
| Direction = "shorthand" | ||
| Change = toShorthand sourceText path lambda root | ||
|
abonie marked this conversation as resolved.
|
||
| } | ||
| | _ -> None | ||
|
|
||
| | SyntaxNode.SynExpr(SynExpr.DotLambda(expr = body; range = range; trivia = trivia)) when | ||
| not body.IsArbExprAndThusAlreadyReportedError | ||
|
abonie marked this conversation as resolved.
|
||
| && not (isInQuotation path) | ||
| && lastOffsideSensitiveLine body = range.StartLine | ||
| -> | ||
| Some | ||
| { | ||
| Title = SR.ConvertToFunLambda() | ||
| Direction = "lambda" | ||
| Change = toLambda sourceText path body range trivia | ||
| } | ||
|
|
||
| | _ -> None) | ||
|
|
||
| [<ExportCodeRefactoringProvider(FSharpConstants.FSharpLanguageName, Name = "ConvertDotLambda"); Shared>] | ||
| type internal FSharpConvertDotLambdaRefactoring [<ImportingConstructor>] () = | ||
| inherit CodeRefactoringProvider() | ||
|
|
||
| override _.ComputeRefactoringsAsync context = | ||
| cancellableTask { | ||
| let document = context.Document | ||
|
|
||
| if not document.IsFSharpSignatureFile then | ||
| let! cancellationToken = CancellableTask.getCancellationToken () | ||
| let! sourceText = document.GetTextAsync cancellationToken | ||
|
|
||
| let! parseResults = document.GetFSharpParseResultsAsync(nameof FSharpConvertDotLambdaRefactoring) | ||
|
|
||
| let caret = sourceText.Lines.GetLinePosition context.Span.Start | ||
| let position = Position.fromZ caret.Line caret.Character | ||
|
|
||
| match DotLambdaConversion.tryFind sourceText position parseResults.ParseTree with | ||
| | Some conversion -> | ||
| let changedDocument = | ||
| cancellableTask { | ||
| TelemetryReporter.ReportSingleEvent( | ||
| TelemetryEvents.RefactoringActivated, | ||
| [| | ||
| "name", box (nameof FSharpConvertDotLambdaRefactoring) | ||
| "direction", box conversion.Direction | ||
| |] | ||
| ) | ||
|
|
||
| return document.WithText(sourceText.WithChanges conversion.Change) | ||
| } | ||
|
|
||
| context.RegisterRefactoring(CodeAction.Create(conversion.Title, changedDocument, conversion.Title)) | ||
| | None -> () | ||
| } | ||
| |> CancellableTask.startAsTask context.CancellationToken | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.