From 43dfc376be16a6f410898c38b2723035ee1f2c67 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 01:32:25 +0200 Subject: [PATCH 1/6] Add refactorings that extract a selected expression into a let binding or a literal The selected expression is bound to a new name in front of the statement that uses it: above a statement that starts its line, or, when the body of a binding, match clause, branch or lambda shares the line with its keyword, on new lines under that keyword. A constant inside a module-level declaration can instead become a [] in front of that declaration. The shared selection and placement logic lives in Refactor/RefactoringHelpers.fs, and the refactoring test framework gains helpers that run a provider on a selected span. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../src/FSharp.Editor/FSharp.Editor.fsproj | 2 + .../src/FSharp.Editor/FSharp.Editor.resx | 6 + .../Refactor/ExtractLetBinding.fs | 84 +++ .../Refactor/RefactoringHelpers.fs | 507 ++++++++++++++++++ .../Telemetry/TelemetryReporter.fs | 3 + .../FSharp.Editor/xlf/FSharp.Editor.cs.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.de.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.es.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.fr.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.it.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.ja.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.ko.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.pl.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.ru.xlf | 10 + .../FSharp.Editor/xlf/FSharp.Editor.tr.xlf | 10 + .../xlf/FSharp.Editor.zh-Hans.xlf | 10 + .../xlf/FSharp.Editor.zh-Hant.xlf | 10 + .../FSharp.Editor.Tests.fsproj | 1 + .../Refactors/ExtractLetBindingTests.fs | 170 ++++++ .../Refactors/RefactorTestFramework.fs | 27 + 22 files changed, 931 insertions(+) create mode 100644 vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs create mode 100644 vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index ba03f663967..8041eeef17d 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -2,6 +2,7 @@ * Code-fixes for FS3888 (compiler-semantic attribute on the `.fs` but not the `.fsi`): copy the attribute into the `.fsi`, or remove it from the `.fs`. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880)) * Expand `` in IDE tooltips, completion, and signature help, inheriting XML documentation from base classes, interfaces, overridden members, and constructors. ([Issue #19175](https://github.com/dotnet/fsharp/issues/19175), [PR #19188](https://github.com/dotnet/fsharp/pull/19188)) +* **Extract to let binding** and **Extract to literal** refactorings for a selected expression. The value is bound in front of the statement that uses it, so it is computed before the code that preceded the selection in that statement; a constant in a module-level declaration becomes a `[]` in front of that declaration. ### Fixed diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index 319bdd5a264..4759355028a 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -101,10 +101,12 @@ + + diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx index 1f1f632d770..b645ff73aa8 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx @@ -368,4 +368,10 @@ Use live (unsaved) buffers for analysis Returns: + + Extract to let binding + + + Extract to literal + \ No newline at end of file diff --git a/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs b/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs new file mode 100644 index 00000000000..947e5c4871a --- /dev/null +++ b/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs @@ -0,0 +1,84 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +namespace Microsoft.VisualStudio.FSharp.Editor + +open System.Composition + +open Microsoft.CodeAnalysis.CodeActions +open Microsoft.CodeAnalysis.CodeRefactorings +open Microsoft.CodeAnalysis.Formatting +open Microsoft.CodeAnalysis.Text +open Microsoft.VisualStudio.FSharp.Editor.Telemetry + +open FSharp.Compiler.Syntax + +open RefactoringHelpers +open CancellableTasks + +[] +type internal FSharpExtractLetBindingRefactoring [] () = + inherit CodeRefactoringProvider() + + static let register + (context: CodeRefactoringContext) + (sourceText: SourceText) + (title: string) + (kind: string) + (changes: TextChange list) + = + let changedDocument = + cancellableTask { + TelemetryReporter.ReportSingleEvent( + TelemetryEvents.RefactoringActivated, + [| "name", box (nameof FSharpExtractLetBindingRefactoring); "kind", box kind |] + ) + + return context.Document.WithText(sourceText.WithChanges changes) + } + + context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) + + override _.ComputeRefactoringsAsync context = + cancellableTask { + let document = context.Document + + if not (context.Span.IsEmpty || document.IsFSharpSignatureFile) then + let! cancellationToken = CancellableTask.getCancellationToken () + let! sourceText = document.GetTextAsync cancellationToken + let! parseResults = document.GetFSharpParseResultsAsync(nameof FSharpExtractLetBindingRefactoring) + + match tryExtractionTarget sourceText parseResults.ParseTree context.Span with + | ValueNone -> () + | ValueSome target -> + let! options = document.GetOptionsAsync cancellationToken + + let indentSize = + options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) + + let names = usedNames parseResults.ParseTree + let literalLines = linesInsideLiterals parseResults.ParseTree + + match anchorsOf target.Expr target.Path with + | anchor :: _ -> + let name = uniqueName "extracted" names + + match tryDeclareInFront sourceText target anchor $"let {name}" name indentSize literalLines with + | ValueSome changes -> register context sourceText (SR.ExtractToLetBinding()) "let" changes + | ValueNone -> () + | [] -> () + + let constant = + match target.Expr with + | SynExpr.Paren(expr = inner) -> inner + | expr -> expr + + if isLiteralConstant constant then + let name = uniqueName "ExtractedConstant" names + + match + tryDeclareInFrontOfModuleLet sourceText target [ "[]" ] $"let {name}" name indentSize literalLines + with + | ValueSome changes -> register context sourceText (SR.ExtractToLiteral()) "literal" changes + | ValueNone -> () + } + |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs b/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs new file mode 100644 index 00000000000..f69c579a364 --- /dev/null +++ b/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs @@ -0,0 +1,507 @@ +// Copyright (c) Microsoft Corporation. All Rights Reserved. See License.txt in the project root for license information. + +module internal Microsoft.VisualStudio.FSharp.Editor.RefactoringHelpers + +open System +open System.Collections.Generic +open System.Text + +open Microsoft.CodeAnalysis.Text + +open FSharp.Compiler.Syntax +open FSharp.Compiler.SyntaxTrivia +open FSharp.Compiler.Text + +/// A place in front of which a declaration used by the selected expression can be inserted. +[] +type Anchor = + /// An expression that is a statement of its block. + | Statement of start: pos + /// The body of a binding, match clause, branch or lambda, after the keyword ending at keywordEnd. + | Clause of keywordEnd: pos * body: range + +[] +type ExtractionTarget = + { + Expr: SynExpr + Path: SyntaxVisitorPath + /// The selection without its parentheses when the selected expression is parenthesized. + Content: TextSpan + /// The text the new name replaces; the parentheses of a method call stay. + Replaced: TextSpan + } + +let private isSame (expr: SynExpr) (other: SynExpr) = obj.ReferenceEquals(expr, other) + +let private hasName (name: string) (ident: Ident) = + String.Equals(ident.idText, name, StringComparison.Ordinal) + +let textSpanOf (sourceText: SourceText) (m: range) = + RoslynHelpers.FSharpRangeToTextSpan(sourceText, m) + +let positionOf (sourceText: SourceText) (offset: int) = + let linePosition = sourceText.Lines.GetLinePosition offset + Position.mkPos (Line.fromZ linePosition.Line) linePosition.Character + +let leadingSpaces (sourceText: SourceText) (line: TextLine) = + let mutable position = line.Start + + while position < line.End && sourceText[position] = ' ' do + position <- position + 1 + + position - line.Start + +let lineBreakOf (sourceText: SourceText) = + sourceText.Lines + |> Seq.tryFind (fun line -> line.EndIncludingLineBreak > line.End) + |> Option.map (fun line -> sourceText.ToString(TextSpan.FromBounds(line.End, line.EndIncludingLineBreak))) + |> Option.defaultValue Environment.NewLine + +let isLineLeading (sourceText: SourceText) (position: pos) = + let line = sourceText.Lines[Line.toZ position.Line] + leadingSpaces sourceText line = position.Column + +/// Whether only closing brackets and a line comment follow the position on its line. +let restOfLineIsClosers (sourceText: SourceText) (position: pos) = + let line = sourceText.Lines[Line.toZ position.Line] + + let rest = + sourceText.ToString(TextSpan.FromBounds(line.Start + position.Column, line.End)) + + let code = + match rest.IndexOf("//", StringComparison.Ordinal) with + | -1 -> rest + | comment -> rest.Substring(0, comment) + + code + |> Seq.forall (fun c -> Char.IsWhiteSpace c || c = ')' || c = ']' || c = '}' || c = '|') + +let linesInsideLiterals (parseTree: ParsedInput) = + (HashSet(), parseTree) + ||> ParsedInput.fold (fun lines _ node -> + match node with + | SyntaxNode.SynExpr(SynExpr.Const(range = m)) + | SyntaxNode.SynExpr(SynExpr.InterpolatedString(range = m)) + | SyntaxNode.SynPat(SynPat.Const(range = m)) -> + for line in m.StartLine + 1 .. m.EndLine do + lines.Add(Line.toZ line) |> ignore + | _ -> () + + lines) + +let usedNames (parseTree: ParsedInput) = + (HashSet(StringComparer.Ordinal), parseTree) + ||> ParsedInput.fold (fun names _ node -> + match node with + | SyntaxNode.SynExpr(SynExpr.Ident ident) + | SyntaxNode.SynExpr(SynExpr.LongIdent(longDotId = SynLongIdent(id = ident :: _))) + | SyntaxNode.SynPat(SynPat.Named(ident = SynIdent(ident, _))) + | SyntaxNode.SynPat(SynPat.LongIdent(longDotId = SynLongIdent(id = ident :: _))) -> names.Add ident.idText |> ignore + | _ -> () + + names) + +let uniqueName (baseName: string) (used: HashSet) = + Seq.initInfinite (fun i -> if i = 0 then baseName else $"{baseName}{i}") + |> Seq.find (used.Contains >> not) + +/// The text of the span with every line after the first moved by as many columns as the first line moves to reach +/// column. Lines inside multi-line string literals are kept as they are; a line that would need a negative indentation +/// makes the whole text unavailable. +let tryIndentedText (sourceText: SourceText) (span: TextSpan) (column: int) (literalLines: HashSet) = + let lines = sourceText.Lines + let first = lines.GetLineFromPosition span.Start + let last = lines.GetLineFromPosition span.End + let shift = column - (span.Start - first.Start) + + let text = + StringBuilder(sourceText.ToString(TextSpan.FromBounds(span.Start, min first.End span.End))) + + let rec append lineNumber = + if lineNumber > last.LineNumber then + ValueSome(text.ToString()) + else + let previous = lines[lineNumber - 1] + let line = lines[lineNumber] + + let content = + sourceText.ToString(TextSpan.FromBounds(line.Start, min line.End span.End)) + + let moved = + match literalLines.Contains lineNumber with + | true -> ValueSome content + | false when String.IsNullOrWhiteSpace content -> ValueSome content + | false when shift >= 0 -> ValueSome(String(' ', shift) + content) + | false when leadingSpaces sourceText line >= -shift -> ValueSome(content.Substring(-shift)) + | false -> ValueNone + + match moved with + | ValueSome moved -> + text.Append(sourceText.ToString(TextSpan.FromBounds(previous.End, previous.EndIncludingLineBreak))).Append(moved) + |> ignore + + append (lineNumber + 1) + | ValueNone -> ValueNone + + append (first.LineNumber + 1) + +let private trimmed (sourceText: SourceText) (span: TextSpan) = + let mutable start = span.Start + let mutable finish = span.End + + while start < finish && Char.IsWhiteSpace sourceText[start] do + start <- start + 1 + + while finish > start && Char.IsWhiteSpace sourceText[finish - 1] do + finish <- finish - 1 + + TextSpan.FromBounds(start, finish) + +let private directiveLine (directive: ConditionalDirectiveTrivia) = + match directive with + | ConditionalDirectiveTrivia.If(range = m) + | ConditionalDirectiveTrivia.Elif(range = m) + | ConditionalDirectiveTrivia.Else(range = m) + | ConditionalDirectiveTrivia.EndIf(range = m) -> m.StartLine + +let private isOperator (name: string) (expr: SynExpr) = + match expr with + | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ operator ])) -> hasName name operator + | _ -> false + +let private isCall (path: SyntaxVisitorPath) = + match path with + | SyntaxNode.SynExpr(SynExpr.App(flag = ExprAtomicFlag.Atomic) | SynExpr.New _) :: _ -> true + | _ -> false + +let private isMethodArgument (path: SyntaxVisitorPath) = + match path with + | SyntaxNode.SynExpr(SynExpr.Paren _) :: call + | SyntaxNode.SynExpr(SynExpr.Tuple _) :: SyntaxNode.SynExpr(SynExpr.Paren _) :: call -> isCall call + | _ -> false + +let private isExtractableShape (expr: SynExpr) (path: SyntaxVisitorPath) = + match expr with + | SynExpr.App(isInfix = true) + | SynExpr.Ident _ + | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ _ ])) + | SynExpr.Const(SynConst.Unit, _) + | SynExpr.Paren(rightParenRange = None) + | SynExpr.ComputationExpr _ + | SynExpr.YieldOrReturn _ + | SynExpr.YieldOrReturnFrom _ + | SynExpr.DoBang _ + | SynExpr.MatchBang _ + | SynExpr.WhileBang _ + | SynExpr.ImplicitZero _ + | SynExpr.SequentialOrImplicitYield _ + | SynExpr.JoinIn _ + | SynExpr.ArbitraryAfterError _ + | SynExpr.FromParseError _ + | SynExpr.DiscardAfterMissingQualificationAfterDot _ + | SynExpr.Typar _ + | SynExpr.TraitCall _ + | SynExpr.IndexRange _ + | SynExpr.IndexFromEnd _ + | SynExpr.Fixed _ + | SynExpr.AddressOf _ + | SynExpr.Do _ + | SynExpr.Dynamic _ + | SynExpr.LongIdentSet _ + | SynExpr.Set _ + | SynExpr.DotSet _ + | SynExpr.DotIndexedSet _ + | SynExpr.NamedIndexedPropertySet _ + | SynExpr.DotNamedIndexedPropertySet _ -> false + | SynExpr.LetOrUse letOrUse -> not letOrUse.IsBang + | SynExpr.Tuple _ -> not (isMethodArgument path) + | SynExpr.App(isInfix = false; funcExpr = SynExpr.App(isInfix = true; funcExpr = equals; argExpr = SynExpr.Ident _)) when + isOperator "op_Equality" equals + -> + not (isMethodArgument path) + | _ -> true + +let private isInExcludedContext (expr: SynExpr) (path: SyntaxVisitorPath) = + let rec loop (child: SyntaxNode) (path: SyntaxVisitorPath) = + match path, child with + | [], _ -> false + | SyntaxNode.SynExpr(SynExpr.InterpolatedString _ | SynExpr.Quote _ | SynExpr.Lazy _) :: _, _ -> true + | SyntaxNode.SynExpr(SynExpr.While(whileExpr = condition) | SynExpr.WhileBang(whileExpr = condition)) :: _, SyntaxNode.SynExpr expr when + isSame condition expr + -> + true + | SyntaxNode.SynMatchClause(SynMatchClause(whenExpr = Some guard)) :: _, SyntaxNode.SynExpr expr when isSame guard expr -> true + | SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = SynExpr.App(isInfix = true; funcExpr = operator); argExpr = right)) :: _, + SyntaxNode.SynExpr expr when + isSame right expr + && (isOperator "op_BooleanAnd" operator || isOperator "op_BooleanOr" operator) + -> + true + | parent :: rest, _ -> loop parent rest + + loop (SyntaxNode.SynExpr expr) path + +/// Whether the expression contains a computation expression construct outside any computation expression of its own. +let private hasStatementOnlyConstruct (expr: SynExpr) = + let containers = ResizeArray() + let statements = ResizeArray() + + [ SyntaxNode.SynExpr expr ] + |> SyntaxNodes.fold + (fun () _ node -> + match node with + | SyntaxNode.SynExpr(SynExpr.ComputationExpr(range = m) | SynExpr.ArrayOrListComputed(range = m)) -> containers.Add m + | SyntaxNode.SynExpr(SynExpr.YieldOrReturn(range = m) | SynExpr.YieldOrReturnFrom(range = m) | SynExpr.DoBang(range = m) | SynExpr.MatchBang( + range = m) | SynExpr.WhileBang(range = m) | SynExpr.JoinIn(range = m)) -> statements.Add m + | SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse) when letOrUse.IsBang -> statements.Add letOrUse.Range + | _ -> ()) + () + + statements + |> Seq.exists (fun statement -> + not ( + containers + |> Seq.exists (fun container -> Range.rangeContainsRange container statement) + )) + +let tryExtractionTarget (sourceText: SourceText) (parseTree: ParsedInput) (selection: TextSpan) = + let span = trimmed sourceText selection + + match parseTree with + | ParsedInput.ImplFile file when not span.IsEmpty -> + let start = positionOf sourceText span.Start + let finish = positionOf sourceText span.End + + let crossesDirective = + file.Trivia.ConditionalDirectives + |> List.exists (fun directive -> + let line = directiveLine directive + line >= start.Line && line <= finish.Line) + + let exact = + (start, parseTree) + ||> ParsedInput.tryPickLast (fun path node -> + match node with + | SyntaxNode.SynExpr expr when Position.posEq expr.Range.Start start && Position.posEq expr.Range.End finish -> + Some(expr, path) + | _ -> None) + + match exact with + | Some(expr, path) when + not crossesDirective + && isExtractableShape expr path + && not (isInExcludedContext expr path) + && not (hasStatementOnlyConstruct expr) + -> + let content = + match expr with + | SynExpr.Paren(expr = inner) -> textSpanOf sourceText inner.Range + | _ -> span + + let replaced = + match expr with + | SynExpr.Paren _ when isCall path -> content + | _ -> span + + ValueSome + { + Expr = expr + Path = path + Content = content + Replaced = replaced + } + | _ -> ValueNone + | _ -> ValueNone + +let private isFunctionBinding (binding: SynBinding) = + match binding with + | SynBinding(headPat = SynPat.LongIdent(argPats = SynArgPats.Pats(_ :: _))) -> true + | _ -> false + +let private anchorOf (child: SyntaxNode) (parent: SyntaxNode) (grandparent: SyntaxNode voption) = + match parent, child with + | SyntaxNode.SynExpr(SynExpr.Sequential _), SyntaxNode.SynExpr expr -> ValueSome(Anchor.Statement expr.Range.Start) + | SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse), SyntaxNode.SynExpr expr when isSame letOrUse.Body expr -> + ValueSome(Anchor.Statement expr.Range.Start) + | SyntaxNode.SynExpr(SynExpr.For(doBody = body) | SynExpr.ForEach(bodyExpr = body) | SynExpr.While(doExpr = body) | SynExpr.TryWith( + tryExpr = body) | SynExpr.TryFinally(tryExpr = body) | SynExpr.ComputationExpr(expr = body)), + SyntaxNode.SynExpr expr when isSame body expr -> ValueSome(Anchor.Statement expr.Range.Start) + | SyntaxNode.SynModule(SynModuleDecl.Expr _), SyntaxNode.SynExpr expr -> ValueSome(Anchor.Statement expr.Range.Start) + | SyntaxNode.SynBinding(SynBinding(expr = rhs; trivia = trivia) as binding), SyntaxNode.SynExpr expr when isSame rhs expr -> + match grandparent, trivia.EqualsRange with + | ValueSome(SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse)), _ when not letOrUse.IsRecursive && not (isFunctionBinding binding) -> + ValueSome(Anchor.Statement letOrUse.Range.Start) + | _, Some equals -> ValueSome(Anchor.Clause(equals.End, expr.Range)) + | _ -> ValueNone + | SyntaxNode.SynMatchClause(SynMatchClause(resultExpr = result; trivia = trivia)), SyntaxNode.SynExpr expr when isSame result expr -> + match trivia.ArrowRange with + | Some arrow -> ValueSome(Anchor.Clause(arrow.End, expr.Range)) + | None -> ValueNone + | SyntaxNode.SynExpr(SynExpr.IfThenElse(thenExpr = thenExpr; elseExpr = elseExpr; trivia = trivia)), SyntaxNode.SynExpr expr -> + match elseExpr, trivia.ElseKeyword with + | _ when isSame thenExpr expr -> ValueSome(Anchor.Clause(trivia.ThenKeyword.End, expr.Range)) + | Some elseExpr, Some elseKeyword when isSame elseExpr expr -> ValueSome(Anchor.Clause(elseKeyword.End, expr.Range)) + | _ -> ValueNone + | SyntaxNode.SynExpr(SynExpr.Lambda(parsedData = Some(_, body); trivia = trivia)), SyntaxNode.SynExpr expr when isSame body expr -> + match trivia.ArrowRange with + | Some arrow -> ValueSome(Anchor.Clause(arrow.End, expr.Range)) + | None -> ValueNone + | _ -> ValueNone + +/// The places a declaration used by the expression can go, innermost first. +let anchorsOf (expr: SynExpr) (path: SyntaxVisitorPath) = + let rec loop (child: SyntaxNode) (path: SyntaxVisitorPath) = + match path with + | [] -> [] + | parent :: rest -> + let grandparent = + match rest with + | node :: _ -> ValueSome node + | [] -> ValueNone + + match anchorOf child parent grandparent with + | ValueSome anchor -> anchor :: loop parent rest + | ValueNone -> loop parent rest + + loop (SyntaxNode.SynExpr expr) path + +let isLiteralConstant (expr: SynExpr) = + match expr with + | SynExpr.Const((SynConst.Bool _ | SynConst.SByte _ | SynConst.Byte _ | SynConst.Int16 _ | SynConst.UInt16 _ | SynConst.Int32 _ | SynConst.UInt32 _ | SynConst.Int64 _ | SynConst.UInt64 _ | SynConst.IntPtr _ | SynConst.UIntPtr _ | SynConst.Single _ | SynConst.Double _ | SynConst.Char _ | SynConst.String _), + _) -> true + | _ -> false + +/// The module-level let declaration containing the path, with its first binding. +let tryEnclosingModuleLet (path: SyntaxVisitorPath) = + path + |> List.tryPick (function + | SyntaxNode.SynModule(SynModuleDecl.Let(bindings = binding :: _; range = m)) -> Some struct (binding, m) + | _ -> None) + +let private padding (width: int) = String(' ', width) + +/// `header = `, with a multi-line content starting on its own line at bodyColumn. +let private tryDeclaration (sourceText: SourceText) (content: TextSpan) (header: string) (bodyColumn: int) literalLines lineBreak = + tryIndentedText sourceText content bodyColumn literalLines + |> ValueOption.map (fun body -> + let lines = sourceText.Lines + + if lines.GetLineFromPosition(content.Start).LineNumber = lines.GetLineFromPosition(content.End).LineNumber then + $"{header} = {body}" + else + $"{header} ={lineBreak}{padding bodyColumn}{body}") + +/// Changes that declare `header = ` in front of the anchor and put replacement where the selection was. +/// A body that shares its line with its keyword moves to new lines under that keyword, when only closing brackets +/// and a comment follow it. +let tryDeclareInFront + (sourceText: SourceText) + (target: ExtractionTarget) + (anchor: Anchor) + (header: string) + (replacement: string) + (indentSize: int) + (literalLines: HashSet) + = + let lines = sourceText.Lines + let lineBreak = lineBreakOf sourceText + let replacementChange = TextChange(target.Replaced, replacement) + + let inFrontOfLine (start: pos) = + let line = lines[Line.toZ start.Line] + + tryDeclaration sourceText target.Content header (start.Column + indentSize) literalLines lineBreak + |> ValueOption.map (fun declaration -> + [ + TextChange(TextSpan(line.Start, 0), $"{padding start.Column}{declaration}{lineBreak}") + replacementChange + ]) + + match anchor with + | Anchor.Statement start when isLineLeading sourceText start -> inFrontOfLine start + | Anchor.Clause(_, body) when isLineLeading sourceText body.Start -> inFrontOfLine body.Start + | Anchor.Clause(keywordEnd, body) when keywordEnd.Line = body.StartLine && restOfLineIsClosers sourceText body.End -> + let keywordLine = lines[Line.toZ keywordEnd.Line] + let keywordEndOffset = keywordLine.Start + keywordEnd.Column + let bodySpan = textSpanOf sourceText body + let newIndent = leadingSpaces sourceText keywordLine + indentSize + let shift = newIndent - body.StartColumn + + let continuation = + [ + for lineNumber in Line.toZ body.StartLine + 1 .. Line.toZ body.EndLine do + let line = lines[lineNumber] + + let keep = + literalLines.Contains lineNumber + || String.IsNullOrWhiteSpace(line.ToString()) + || (line.Start >= target.Replaced.Start && line.Start <= target.Replaced.End) + + if not keep then + line + ] + + let movable = + continuation + |> List.forall (fun line -> shift >= 0 || leadingSpaces sourceText line >= -shift) + + let separatedByWhitespace = + String.IsNullOrWhiteSpace(sourceText.ToString(TextSpan.FromBounds(keywordEndOffset, bodySpan.Start))) + + if movable && separatedByWhitespace then + tryDeclaration sourceText target.Content header (newIndent + indentSize) literalLines lineBreak + |> ValueOption.map (fun declaration -> + [ + TextChange( + TextSpan.FromBounds(keywordEndOffset, bodySpan.Start), + $"{lineBreak}{padding newIndent}{declaration}{lineBreak}{padding newIndent}" + ) + + for line in continuation do + if shift > 0 then + TextChange(TextSpan(line.Start, 0), padding shift) + elif shift < 0 then + TextChange(TextSpan(line.Start, -shift), "") + + replacementChange + ] + |> List.sortBy _.Span.Start) + else + ValueNone + | _ -> ValueNone + +/// Changes that declare `header = `, preceded by the attribute lines, in front of the module-level let +/// containing the selection, and put replacement where the selection was. +let tryDeclareInFrontOfModuleLet + (sourceText: SourceText) + (target: ExtractionTarget) + (attributes: string list) + (header: string) + (replacement: string) + (indentSize: int) + (literalLines: HashSet) + = + match tryEnclosingModuleLet target.Path with + | Some(struct (SynBinding(xmlDoc = xmlDoc), declaration)) -> + let firstLine = + if xmlDoc.IsEmpty then + declaration.StartLine + else + min declaration.StartLine xmlDoc.Range.StartLine + + let line = sourceText.Lines[Line.toZ firstLine] + let indent = leadingSpaces sourceText line + let lineBreak = lineBreakOf sourceText + + tryDeclaration sourceText target.Content header (indent + indentSize) literalLines lineBreak + |> ValueOption.map (fun declaration -> + let attributeLines = + attributes + |> List.map (fun attribute -> $"{padding indent}{attribute}{lineBreak}") + |> String.concat "" + + [ + TextChange(TextSpan(line.Start, 0), $"{attributeLines}{padding indent}{declaration}{lineBreak}{lineBreak}") + TextChange(target.Replaced, replacement) + ]) + | None -> ValueNone diff --git a/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs b/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs index 230f4988438..a5bf5e6b4ef 100644 --- a/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs +++ b/vsintegration/src/FSharp.Editor/Telemetry/TelemetryReporter.fs @@ -18,6 +18,9 @@ module TelemetryEvents = [] let CodefixActivated = "codefixactivated" + [] + let RefactoringActivated = "refactoringactivated" + [] let Hints = "hints" diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf index cd8c46bf705..9b94eafd87e 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf @@ -105,6 +105,16 @@ Navrhnout názvy pro nerozpoznané identifikátory; Použít místo negace odčítání + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Uvolnitelné hodnoty jazyka F# (místní) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf index bce1941f0b1..8ce1754b360 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf @@ -105,6 +105,16 @@ Namen für nicht aufgelöste Bezeichner vorschlagen; Subtraktion anstelle von Negation verwenden + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Disposable-Werte in F# (lokal) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf index fa8cb62c422..501be2d5e7c 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf @@ -105,6 +105,16 @@ Sugerir nombres para identificadores sin resolver; Usar la resta en lugar de la negación + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Valores de F# descartables (locales) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf index e7ec71e839e..8b8d9a57638 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf @@ -105,6 +105,16 @@ Suggérer des noms pour les identificateurs non résolus ; Utiliser la soustraction à la place de la négation + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Valeurs F# pouvant être supprimées (variables locales) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf index 327a7ca362f..35e4cca8ad4 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf @@ -105,6 +105,16 @@ Suggerisci i nomi per gli identificatori non risolti; Usare la sottrazione invece della negazione + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Valori eliminabili F# (variabili locali) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf index d45234c011a..928004aa616 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf @@ -105,6 +105,16 @@ Suggest names for unresolved identifiers; 否定の代わりに減算を使用する + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) F# の破棄可能な値 (ローカル) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf index 3248e0641ea..c104880c6d9 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf @@ -105,6 +105,16 @@ Suggest names for unresolved identifiers; 부정 대신 빼기 사용 + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) F# 삭제 가능한 값(로컬) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf index abc39f15da5..4a27d371123 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf @@ -105,6 +105,16 @@ Sugeruj nazwy dla nierozpoznanych identyfikatorów; Użyj odejmowania zamiast negacji + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Wartości możliwe do likwidacji języka F# (lokalne) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf index dfde43120f5..ee0729dde99 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf @@ -105,6 +105,16 @@ Sugerir nomes para identificadores não resolvidos; Use a subtração em vez da negação + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Valores F# Descartáveis (locais) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf index 47cda215312..69476255afe 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf @@ -105,6 +105,16 @@ Suggest names for unresolved identifiers; Используйте вычитание вместо отрицания. + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) Освобождаемые значения F# (локальные) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf index 58aa5d54c43..802990076ba 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf @@ -105,6 +105,16 @@ Kullanılmayan değerleri analiz et ve bunlara düzeltmeler öner; Negatif yapma yerine çıkarmayı kullanın + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) F# Atılabilir Değerleri (yereller) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf index 4fa703776fb..ace8f6ec72e 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf @@ -105,6 +105,16 @@ Suggest names for unresolved identifiers; 使用减法代替求反 + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) F# 可释放值(局部值) diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf index fd46ef9919a..e2191749e38 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf @@ -105,6 +105,16 @@ Suggest names for unresolved identifiers; 使用減號代替否定 + + Extract to let binding + Extract to let binding + + + + Extract to literal + Extract to literal + + F# Disposable Values (locals) F# 可處置的值 (區域) diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index ecce1205b8c..67a671126da 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -74,6 +74,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs new file mode 100644 index 00000000000..e30e122c27b --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs @@ -0,0 +1,170 @@ +module FSharp.Editor.Tests.Refactors.ExtractLetBindingTests + +open System + +open Microsoft.CodeAnalysis.Text + +open Microsoft.VisualStudio.FSharp.Editor +open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks + +open Xunit + +open FSharp.Editor.Tests.Refactors.RefactorTestFramework + +let private extractToLetBinding = "Extract to let binding" +let private extractToLiteral = "Extract to literal" + +let private selectionOf (code: string) (selected: string) = + TextSpan(code.IndexOf(selected, StringComparison.Ordinal), selected.Length) + +let private extracted (title: string) (code: string) (selected: string) = + use context = TestContext.CreateWithCode code + + let document = + refactorSpan code (selectionOf code selected) title context (new FSharpExtractLetBindingRefactoring()) + + let parseResults = + document.GetFSharpParseResultsAsync "test" + |> CancellableTask.runSynchronouslyWithoutCancellation + + Assert.Empty(parseResults.Diagnostics) + (document.GetTextAsync() |> GetTaskResult).ToString() + +let private titlesFor (code: string) (selected: string) = + use context = TestContext.CreateWithCode code + + tryGetRefactoringActionsForSpan code (selectionOf code selected) context (new FSharpExtractLetBindingRefactoring()) + |> Seq.map _.Title + |> List.ofSeq + +[] +[] +[] +let ``Expression in a statement is bound in front of it`` (selected: string, body: string) = + let code = "module M\n\nlet area w h =\n printfn \"%d\" (w * h + 1)\n" + Assert.Equal($"module M\n\nlet area w h =\n{body}", extracted extractToLetBinding code selected) + +[] +let ``Parentheses of a method call stay`` () = + let code = + "module M\n\nlet append (sb: System.Text.StringBuilder) =\n sb.Append(1 + 2)\n" + + let expected = + "module M\n\nlet append (sb: System.Text.StringBuilder) =\n let extracted = 1 + 2\n sb.Append(extracted)\n" + + Assert.Equal(expected, extracted extractToLetBinding code "(1 + 2)") + +[] +let ``Multi-line right-hand side is bound in front of its let`` () = + let code = + "module M\n\nlet run items =\n let mutable acc = 0\n let total =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n total\n" + + let expected = + "module M\n\nlet run items =\n let mutable acc = 0\n let extracted =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n let total =\n extracted\n total\n" + + Assert.Equal(expected, extracted extractToLetBinding code "items\n |> List.filter (fun i -> i > acc)\n |> List.sum") + +[] +let ``Match clause body on the arrow line moves to its own lines`` () = + let code = + "module M\n\nlet f x offset =\n match x with\n | Some v -> v * 2 + offset\n | None -> 0\n" + + let expected = + "module M\n\nlet f x offset =\n match x with\n | Some v ->\n let extracted = v * 2 + offset\n extracted\n | None -> 0\n" + + Assert.Equal(expected, extracted extractToLetBinding code "v * 2 + offset") + +[] +let ``Right-hand side on the equals line keeps its trailing comment`` () = + let code = "module M\n\nlet r = compute a b // slow\n" + + let expected = + "module M\n\nlet r =\n let extracted = compute a b\n extracted // slow\n" + + Assert.Equal(expected, extracted extractToLetBinding code "compute a b") + +[] +let ``Expression in a computation expression is bound in front of its statement`` () = + let code = + "module M\n\nlet load id = async {\n let! raw = fetch id\n return parse raw |> List.length }\n" + + let expected = + "module M\n\nlet load id = async {\n let! raw = fetch id\n let extracted = parse raw |> List.length\n return extracted }\n" + + Assert.Equal(expected, extracted extractToLetBinding code "parse raw |> List.length") + +[] +let ``Lambda body on the arrow line moves to its own lines`` () = + let code = "module M\n\nlet r = xs |> List.map (fun x -> x + 1)\n" + + let expected = + "module M\n\nlet r = xs |> List.map (fun x ->\n let extracted = x + 1\n extracted)\n" + + Assert.Equal(expected, extracted extractToLetBinding code "x + 1") + +[] +let ``Name does not collide with an existing identifier`` () = + let code = + "module M\n\nlet extracted = 0\n\nlet f x =\n printfn \"%d\" (x + 1)\n" + + let expected = + "module M\n\nlet extracted = 0\n\nlet f x =\n let extracted1 = x + 1\n printfn \"%d\" (extracted1)\n" + + Assert.Equal(expected, extracted extractToLetBinding code "x + 1") + +[] +let ``Line breaks of the file are kept`` () = + let code = "module M\r\n\r\nlet f x =\r\n printfn \"%d\" (x + 1)\r\n" + + let expected = + "module M\r\n\r\nlet f x =\r\n let extracted = x + 1\r\n printfn \"%d\" (extracted)\r\n" + + Assert.Equal(expected, extracted extractToLetBinding code "x + 1") + +[] +let ``Lines inside a multi-line string are not re-indented`` () = + let code = + "module M\n\nlet f name =\n printfn \"%s\" (String.Format(\"\"\"Hello\n{0}\"\"\", name))\n" + + let expected = + "module M\n\nlet f name =\n let extracted =\n String.Format(\"\"\"Hello\n{0}\"\"\", name)\n printfn \"%s\" (extracted)\n" + + Assert.Equal(expected, extracted extractToLetBinding code "String.Format(\"\"\"Hello\n{0}\"\"\", name)") + +[] +let ``Constant in a module-level declaration becomes a literal in front of it`` () = + let code = "module M\n\nlet greet name =\n printfn \"Hello %s\" name\n" + + let expected = + "module M\n\n[]\nlet ExtractedConstant = \"Hello %s\"\n\nlet greet name =\n printfn ExtractedConstant name\n" + + Assert.Equal(expected, extracted extractToLiteral code "\"Hello %s\"") + +[] +let ``Negative number becomes a literal`` () = + let code = "module M\n\nlet f () = g -1\n" + + let expected = + "module M\n\n[]\nlet ExtractedConstant = -1\n\nlet f () = g ExtractedConstant\n" + + Assert.Equal(expected, extracted extractToLiteral code "-1") + +[] +let ``Literal is offered only for constants outside types`` () = + Assert.Equal([ extractToLetBinding; extractToLiteral ], titlesFor "module M\n\nlet f () = g 42\n" "42") + Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\ntype T() =\n member _.M() = 42\n" "42") + Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\nlet f x = g (x + 1)\n" "x + 1") + +[] +[] +[] +[] +[] +[ 0 -> v\n | _ -> 0\n", "v > 0")>] +[] +[] +[] +[] +[ 0\n", "s.Length > 0")>] +[] +let ``No action`` (code: string, selected: string) = Assert.Empty(titlesFor code selected) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/RefactorTestFramework.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/RefactorTestFramework.fs index 849da8c84ec..4aad2b130d1 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/RefactorTestFramework.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/RefactorTestFramework.fs @@ -87,3 +87,30 @@ let tryGetRefactoringActions (code: string) (cursorPosition) (context: TestConte } |> CancellableTask.startWithoutCancellation |> fun task -> task.Result + +let private refactoringActionsAt (code: string) (span: TextSpan) (context: TestContext) (refactorProvider: CodeRefactoringProvider) = + let actions = List() + let existingDocument = RoslynTestHelpers.GetLastDocument context.Solution + context.Solution <- context.Solution.WithDocumentText(existingDocument.Id, SourceText.From(code)) + let document = RoslynTestHelpers.GetLastDocument context.Solution + + let refactoringContext = + CodeRefactoringContext(document, span, (fun action -> actions.Add action), context.CancellationToken) + + refactorProvider.ComputeRefactoringsAsync(refactoringContext).GetAwaiter().GetResult() + actions + +let tryGetRefactoringActionsForSpan (code: string) (span: TextSpan) (context: TestContext) (refactorProvider: #CodeRefactoringProvider) = + refactoringActionsAt code span context refactorProvider + +let refactorSpan (code: string) (span: TextSpan) (title: string) (context: TestContext) (refactorProvider: #CodeRefactoringProvider) = + let action = + refactoringActionsAt code span context refactorProvider + |> Seq.find (fun action -> String.Equals(action.Title, title, StringComparison.Ordinal)) + + for operation in action.GetOperationsAsync(context.CancellationToken) |> GetTaskResult do + let applyChanges = operation :?> ApplyChangesOperation + applyChanges.Apply(context.Solution.Workspace, context.CancellationToken) + context.Solution <- applyChanges.ChangedSolution + + RoslynTestHelpers.GetLastDocument context.Solution From f9c69f5ef45a636a85c9bc817e449e7919d137a2 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 13:23:10 +0200 Subject: [PATCH 2/6] Offer Extract to literal on a caret inside a constant, as C# does C# offers Introduce Constant without a selection. With a caret alone only the literal is offered; a let binding still needs a selection. Co-Authored-By: Claude Opus 5 (1M context) --- .../Refactor/ExtractLetBinding.fs | 25 +++++---- .../Refactor/RefactoringHelpers.fs | 30 +++++++++++ .../Refactors/ExtractLetBindingTests.fs | 54 +++++++++++++++++-- 3 files changed, 96 insertions(+), 13 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs b/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs index 947e5c4871a..8233a79ae43 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs @@ -42,12 +42,18 @@ type internal FSharpExtractLetBindingRefactoring [] () = cancellableTask { let document = context.Document - if not (context.Span.IsEmpty || document.IsFSharpSignatureFile) then + if not document.IsFSharpSignatureFile then let! cancellationToken = CancellableTask.getCancellationToken () let! sourceText = document.GetTextAsync cancellationToken let! parseResults = document.GetFSharpParseResultsAsync(nameof FSharpExtractLetBindingRefactoring) - match tryExtractionTarget sourceText parseResults.ParseTree context.Span with + let target = + if context.Span.IsEmpty then + tryConstantAtCaret sourceText parseResults.ParseTree context.Span.Start + else + tryExtractionTarget sourceText parseResults.ParseTree context.Span + + match target with | ValueNone -> () | ValueSome target -> let! options = document.GetOptionsAsync cancellationToken @@ -58,14 +64,15 @@ type internal FSharpExtractLetBindingRefactoring [] () = let names = usedNames parseResults.ParseTree let literalLines = linesInsideLiterals parseResults.ParseTree - match anchorsOf target.Expr target.Path with - | anchor :: _ -> - let name = uniqueName "extracted" names + if not context.Span.IsEmpty then + match anchorsOf target.Expr target.Path with + | anchor :: _ -> + let name = uniqueName "extracted" names - match tryDeclareInFront sourceText target anchor $"let {name}" name indentSize literalLines with - | ValueSome changes -> register context sourceText (SR.ExtractToLetBinding()) "let" changes - | ValueNone -> () - | [] -> () + match tryDeclareInFront sourceText target anchor $"let {name}" name indentSize literalLines with + | ValueSome changes -> register context sourceText (SR.ExtractToLetBinding()) "let" changes + | ValueNone -> () + | [] -> () let constant = match target.Expr with diff --git a/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs b/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs index f69c579a364..525a4a555cb 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs @@ -371,6 +371,36 @@ let isLiteralConstant (expr: SynExpr) = _) -> true | _ -> false +/// The literal constant the caret is in or touches. +let tryConstantAtCaret (sourceText: SourceText) (parseTree: ParsedInput) (caret: int) = + let position = positionOf sourceText caret + + let constant = + (position, parseTree) + ||> ParsedInput.tryPickLast (fun path node -> + match node with + | SyntaxNode.SynExpr(SynExpr.Const(range = m) as expr) when + isLiteralConstant expr + && Position.posGeq position m.Start + && Position.posGeq m.End position + && not (isInExcludedContext expr path) + -> + Some(expr, path) + | _ -> None) + + match constant with + | Some(expr, path) -> + let span = textSpanOf sourceText expr.Range + + ValueSome + { + Expr = expr + Path = path + Content = span + Replaced = span + } + | None -> ValueNone + /// The module-level let declaration containing the path, with its first binding. let tryEnclosingModuleLet (path: SyntaxVisitorPath) = path diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs index e30e122c27b..bb356ad5fa5 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs @@ -17,11 +17,14 @@ let private extractToLiteral = "Extract to literal" let private selectionOf (code: string) (selected: string) = TextSpan(code.IndexOf(selected, StringComparison.Ordinal), selected.Length) -let private extracted (title: string) (code: string) (selected: string) = +let private caretAt (code: string) (marker: string) = + TextSpan(code.IndexOf(marker, StringComparison.Ordinal), 0) + +let private extractedAt (title: string) (code: string) (span: TextSpan) = use context = TestContext.CreateWithCode code let document = - refactorSpan code (selectionOf code selected) title context (new FSharpExtractLetBindingRefactoring()) + refactorSpan code span title context (new FSharpExtractLetBindingRefactoring()) let parseResults = document.GetFSharpParseResultsAsync "test" @@ -30,13 +33,19 @@ let private extracted (title: string) (code: string) (selected: string) = Assert.Empty(parseResults.Diagnostics) (document.GetTextAsync() |> GetTaskResult).ToString() -let private titlesFor (code: string) (selected: string) = +let private extracted (title: string) (code: string) (selected: string) = + extractedAt title code (selectionOf code selected) + +let private titlesAt (code: string) (span: TextSpan) = use context = TestContext.CreateWithCode code - tryGetRefactoringActionsForSpan code (selectionOf code selected) context (new FSharpExtractLetBindingRefactoring()) + tryGetRefactoringActionsForSpan code span context (new FSharpExtractLetBindingRefactoring()) |> Seq.map _.Title |> List.ofSeq +let private titlesFor (code: string) (selected: string) = + titlesAt code (selectionOf code selected) + [] [] [] @@ -64,6 +73,19 @@ let ``Multi-line right-hand side is bound in front of its let`` () = Assert.Equal(expected, extracted extractToLetBinding code "items\n |> List.filter (fun i -> i > acc)\n |> List.sum") +[] +let ``Whole lines selected with their indentation and line break are extracted`` () = + let code = + "module M\n\nlet run items =\n let mutable acc = 0\n\n let total =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n\n total\n" + + let expected = + "module M\n\nlet run items =\n let mutable acc = 0\n\n let extracted =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n let total =\n extracted\n\n total\n" + + Assert.Equal( + expected, + extracted extractToLetBinding code " items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n" + ) + [] let ``Match clause body on the arrow line moves to its own lines`` () = let code = @@ -155,6 +177,30 @@ let ``Literal is offered only for constants outside types`` () = Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\ntype T() =\n member _.M() = 42\n" "42") Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\nlet f x = g (x + 1)\n" "x + 1") +[] +[] +[] +[] +let ``Constant at the caret becomes a literal without a selection`` (marker: string) = + let code = "module M\n\nlet greet name =\n printfn \"Hello %s\" name\n" + + let expected = + "module M\n\n[]\nlet ExtractedConstant = \"Hello %s\"\n\nlet greet name =\n printfn ExtractedConstant name\n" + + Assert.Equal(expected, extractedAt extractToLiteral code (caretAt code marker)) + +[] +let ``Only the literal is offered without a selection`` () = + let code = "module M\n\nlet f () = g 42\n" + Assert.Equal([ extractToLiteral ], titlesAt code (caretAt code "2\n")) + +[] +[] +[] +[] +let ``No action without a selection`` (code: string, marker: string) = + Assert.Empty(titlesAt code (caretAt code marker)) + [] [] [] From c5df58ee75ea499b40710ce8736170c0e8c301b3 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 13:24:19 +0200 Subject: [PATCH 3/6] Mention the caret in the Extract to literal release note Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 8041eeef17d..3ff872ad743 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -2,7 +2,7 @@ * Code-fixes for FS3888 (compiler-semantic attribute on the `.fs` but not the `.fsi`): copy the attribute into the `.fsi`, or remove it from the `.fs`. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880)) * Expand `` in IDE tooltips, completion, and signature help, inheriting XML documentation from base classes, interfaces, overridden members, and constructors. ([Issue #19175](https://github.com/dotnet/fsharp/issues/19175), [PR #19188](https://github.com/dotnet/fsharp/pull/19188)) -* **Extract to let binding** and **Extract to literal** refactorings for a selected expression. The value is bound in front of the statement that uses it, so it is computed before the code that preceded the selection in that statement; a constant in a module-level declaration becomes a `[]` in front of that declaration. +* **Extract to let binding** and **Extract to literal** refactorings for a selected expression. The value is bound in front of the statement that uses it, so it is computed before the code that preceded the selection in that statement; a constant in a module-level declaration becomes a `[]` in front of that declaration, also with just the caret inside the constant. ### Fixed From e2d0daf858a18c103dbabdd181d5880c6768171b Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 13:33:14 +0200 Subject: [PATCH 4/6] Extract a parenthesized lambda from a caret in its fun ... -> header A caret between 'fun' and the end of '->' acts as a selection of the parenthesized lambda; in the body a caret offers nothing new. Co-Authored-By: Claude Opus 5 (1M context) --- .../Refactor/ExtractLetBinding.fs | 16 ++++++++++---- .../Refactor/RefactoringHelpers.fs | 20 +++++++++++++++++ .../Refactors/ExtractLetBindingTests.fs | 22 +++++++++++++++++++ 3 files changed, 54 insertions(+), 4 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs b/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs index 8233a79ae43..4bc595bf49d 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/ExtractLetBinding.fs @@ -47,11 +47,19 @@ type internal FSharpExtractLetBindingRefactoring [] () = let! sourceText = document.GetTextAsync cancellationToken let! parseResults = document.GetFSharpParseResultsAsync(nameof FSharpExtractLetBindingRefactoring) - let target = + let lambdaAtCaret = if context.Span.IsEmpty then - tryConstantAtCaret sourceText parseResults.ParseTree context.Span.Start + tryParenthesizedLambdaAtCaret sourceText parseResults.ParseTree context.Span.Start else - tryExtractionTarget sourceText parseResults.ParseTree context.Span + ValueNone + + let isSelected = not context.Span.IsEmpty || lambdaAtCaret.IsSome + + let target = + match lambdaAtCaret with + | ValueSome _ -> lambdaAtCaret + | ValueNone when context.Span.IsEmpty -> tryConstantAtCaret sourceText parseResults.ParseTree context.Span.Start + | ValueNone -> tryExtractionTarget sourceText parseResults.ParseTree context.Span match target with | ValueNone -> () @@ -64,7 +72,7 @@ type internal FSharpExtractLetBindingRefactoring [] () = let names = usedNames parseResults.ParseTree let literalLines = linesInsideLiterals parseResults.ParseTree - if not context.Span.IsEmpty then + if isSelected then match anchorsOf target.Expr target.Path with | anchor :: _ -> let name = uniqueName "extracted" names diff --git a/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs b/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs index 525a4a555cb..b6cd11425cf 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/RefactoringHelpers.fs @@ -401,6 +401,26 @@ let tryConstantAtCaret (sourceText: SourceText) (parseTree: ParsedInput) (caret: } | None -> ValueNone +/// The parenthesized lambda whose `fun ->` the caret is in, as if it were selected with its parentheses. +let tryParenthesizedLambdaAtCaret (sourceText: SourceText) (parseTree: ParsedInput) (caret: int) = + let position = positionOf sourceText caret + + let parenthesized = + (position, parseTree) + ||> ParsedInput.tryPickLast (fun _ node -> + match node with + | SyntaxNode.SynExpr(SynExpr.Paren( + expr = SynExpr.Lambda(parsedData = Some _; trivia = { ArrowRange = Some arrow }) as lambda; range = m)) when + Position.posGeq position lambda.Range.Start + && Position.posGeq arrow.End position + -> + Some m + | _ -> None) + + match parenthesized with + | Some m -> tryExtractionTarget sourceText parseTree (textSpanOf sourceText m) + | None -> ValueNone + /// The module-level let declaration containing the path, with its first binding. let tryEnclosingModuleLet (path: SyntaxVisitorPath) = path diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs index bb356ad5fa5..7974f2ba4b8 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs @@ -177,6 +177,28 @@ let ``Literal is offered only for constants outside types`` () = Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\ntype T() =\n member _.M() = 42\n" "42") Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\nlet f x = g (x + 1)\n" "x + 1") +[] +[] +[")>] +[ x")>] +[] +let ``Caret in the header of a parenthesized lambda extracts the lambda`` (marker: string) = + let code = "module M\n\nlet f xs =\n xs |> List.map (fun x -> x + 1)\n" + + let expected = + "module M\n\nlet f xs =\n let extracted = fun x -> x + 1\n xs |> List.map extracted\n" + + Assert.Equal([ extractToLetBinding ], titlesAt code (caretAt code marker)) + Assert.Equal(expected, extractedAt extractToLetBinding code (caretAt code marker)) + +[] +[ List.map (fun x -> x + 1)\n", "x + 1)")>] +[ List.map (fun x -> x + 1)\n", "+ 1)")>] +[ List.map (fun x -> x + 1)\n", "(fun")>] +[ x + 1\n", "fun")>] +let ``No action without a selection outside a parenthesized lambda header`` (code: string, marker: string) = + Assert.Empty(titlesAt code (caretAt code marker)) + [] [] [] From 4b02cc9b02ee52c6c74a9adc432f0b698b7602fa Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 15:30:50 +0200 Subject: [PATCH 5/6] Write the refactoring test code as multi-line strings Co-Authored-By: Claude Opus 5 (1M context) --- .../Refactors/ExtractLetBindingTests.fs | 452 +++++++++++++++--- 1 file changed, 377 insertions(+), 75 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs index 7974f2ba4b8..50078b08db0 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ExtractLetBindingTests.fs @@ -46,91 +46,246 @@ let private titlesAt (code: string) (span: TextSpan) = let private titlesFor (code: string) (selected: string) = titlesAt code (selectionOf code selected) +let private area = + """ +module M + +let area w h = + printfn "%d" (w * h + 1) +""" + [] -[] -[] -let ``Expression in a statement is bound in front of it`` (selected: string, body: string) = - let code = "module M\n\nlet area w h =\n printfn \"%d\" (w * h + 1)\n" - Assert.Equal($"module M\n\nlet area w h =\n{body}", extracted extractToLetBinding code selected) +[] +[] +let ``Expression in a statement is bound in front of it`` (selected: string, expected: string) = + Assert.Equal(expected, extracted extractToLetBinding area selected) [] let ``Parentheses of a method call stay`` () = let code = - "module M\n\nlet append (sb: System.Text.StringBuilder) =\n sb.Append(1 + 2)\n" + """ +module M + +let append (sb: System.Text.StringBuilder) = + sb.Append(1 + 2) +""" let expected = - "module M\n\nlet append (sb: System.Text.StringBuilder) =\n let extracted = 1 + 2\n sb.Append(extracted)\n" + """ +module M + +let append (sb: System.Text.StringBuilder) = + let extracted = 1 + 2 + sb.Append(extracted) +""" Assert.Equal(expected, extracted extractToLetBinding code "(1 + 2)") +let private run = + """ +module M + +let run items = + let mutable acc = 0 + let total = + items + |> List.filter (fun i -> i > acc) + |> List.sum + total +""" + [] let ``Multi-line right-hand side is bound in front of its let`` () = - let code = - "module M\n\nlet run items =\n let mutable acc = 0\n let total =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n total\n" - let expected = - "module M\n\nlet run items =\n let mutable acc = 0\n let extracted =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n let total =\n extracted\n total\n" + """ +module M - Assert.Equal(expected, extracted extractToLetBinding code "items\n |> List.filter (fun i -> i > acc)\n |> List.sum") +let run items = + let mutable acc = 0 + let extracted = + items + |> List.filter (fun i -> i > acc) + |> List.sum + let total = + extracted + total +""" + + let start = run.LastIndexOf("items", StringComparison.Ordinal) + + let finish = + run.IndexOf("|> List.sum", start, StringComparison.Ordinal) + + "|> List.sum".Length + + Assert.Equal(expected, extractedAt extractToLetBinding run (TextSpan.FromBounds(start, finish))) [] let ``Whole lines selected with their indentation and line break are extracted`` () = let code = - "module M\n\nlet run items =\n let mutable acc = 0\n\n let total =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n\n total\n" + """ +module M + +let run items = + let mutable acc = 0 + + let total = + items + |> List.filter (fun i -> i > acc) + |> List.sum + + total +""" let expected = - "module M\n\nlet run items =\n let mutable acc = 0\n\n let extracted =\n items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n let total =\n extracted\n\n total\n" + """ +module M + +let run items = + let mutable acc = 0 + + let extracted = + items + |> List.filter (fun i -> i > acc) + |> List.sum + let total = + extracted - Assert.Equal( - expected, - extracted extractToLetBinding code " items\n |> List.filter (fun i -> i > acc)\n |> List.sum\n" - ) + total +""" + + let lineStart = code.LastIndexOf(" items", StringComparison.Ordinal) + + let afterLineBreak = + code.IndexOf('\n', code.IndexOf("|> List.sum", lineStart, StringComparison.Ordinal)) + + 1 + + Assert.Equal(expected, extractedAt extractToLetBinding code (TextSpan.FromBounds(lineStart, afterLineBreak))) [] let ``Match clause body on the arrow line moves to its own lines`` () = let code = - "module M\n\nlet f x offset =\n match x with\n | Some v -> v * 2 + offset\n | None -> 0\n" + """ +module M + +let f x offset = + match x with + | Some v -> v * 2 + offset + | None -> 0 +""" let expected = - "module M\n\nlet f x offset =\n match x with\n | Some v ->\n let extracted = v * 2 + offset\n extracted\n | None -> 0\n" + """ +module M + +let f x offset = + match x with + | Some v -> + let extracted = v * 2 + offset + extracted + | None -> 0 +""" Assert.Equal(expected, extracted extractToLetBinding code "v * 2 + offset") [] let ``Right-hand side on the equals line keeps its trailing comment`` () = - let code = "module M\n\nlet r = compute a b // slow\n" + let code = + """ +module M + +let r = compute a b // slow +""" let expected = - "module M\n\nlet r =\n let extracted = compute a b\n extracted // slow\n" + """ +module M + +let r = + let extracted = compute a b + extracted // slow +""" Assert.Equal(expected, extracted extractToLetBinding code "compute a b") [] let ``Expression in a computation expression is bound in front of its statement`` () = let code = - "module M\n\nlet load id = async {\n let! raw = fetch id\n return parse raw |> List.length }\n" + """ +module M + +let load id = async { + let! raw = fetch id + return parse raw |> List.length } +""" let expected = - "module M\n\nlet load id = async {\n let! raw = fetch id\n let extracted = parse raw |> List.length\n return extracted }\n" + """ +module M + +let load id = async { + let! raw = fetch id + let extracted = parse raw |> List.length + return extracted } +""" Assert.Equal(expected, extracted extractToLetBinding code "parse raw |> List.length") [] let ``Lambda body on the arrow line moves to its own lines`` () = - let code = "module M\n\nlet r = xs |> List.map (fun x -> x + 1)\n" + let code = + """ +module M + +let r = xs |> List.map (fun x -> x + 1) +""" let expected = - "module M\n\nlet r = xs |> List.map (fun x ->\n let extracted = x + 1\n extracted)\n" + """ +module M + +let r = xs |> List.map (fun x -> + let extracted = x + 1 + extracted) +""" Assert.Equal(expected, extracted extractToLetBinding code "x + 1") [] let ``Name does not collide with an existing identifier`` () = let code = - "module M\n\nlet extracted = 0\n\nlet f x =\n printfn \"%d\" (x + 1)\n" + """ +module M + +let extracted = 0 + +let f x = + printfn "%d" (x + 1) +""" let expected = - "module M\n\nlet extracted = 0\n\nlet f x =\n let extracted1 = x + 1\n printfn \"%d\" (extracted1)\n" + """ +module M + +let extracted = 0 + +let f x = + let extracted1 = x + 1 + printfn "%d" (extracted1) +""" Assert.Equal(expected, extracted extractToLetBinding code "x + 1") @@ -145,6 +300,7 @@ let ``Line breaks of the file are kept`` () = [] let ``Lines inside a multi-line string are not re-indented`` () = + // The code contains a triple-quoted string, which a triple-quoted literal cannot hold. let code = "module M\n\nlet f name =\n printfn \"%s\" (String.Format(\"\"\"Hello\n{0}\"\"\", name))\n" @@ -153,29 +309,85 @@ let ``Lines inside a multi-line string are not re-indented`` () = Assert.Equal(expected, extracted extractToLetBinding code "String.Format(\"\"\"Hello\n{0}\"\"\", name)") -[] -let ``Constant in a module-level declaration becomes a literal in front of it`` () = - let code = "module M\n\nlet greet name =\n printfn \"Hello %s\" name\n" +let private greet = + """ +module M - let expected = - "module M\n\n[]\nlet ExtractedConstant = \"Hello %s\"\n\nlet greet name =\n printfn ExtractedConstant name\n" +let greet name = + printfn "Hello %s" name +""" - Assert.Equal(expected, extracted extractToLiteral code "\"Hello %s\"") +let private greetWithLiteral = + """ +module M + +[] +let ExtractedConstant = "Hello %s" + +let greet name = + printfn ExtractedConstant name +""" + +[] +let ``Constant in a module-level declaration becomes a literal in front of it`` () = + Assert.Equal(greetWithLiteral, extracted extractToLiteral greet "\"Hello %s\"") [] let ``Negative number becomes a literal`` () = - let code = "module M\n\nlet f () = g -1\n" + let code = + """ +module M + +let f () = g -1 +""" let expected = - "module M\n\n[]\nlet ExtractedConstant = -1\n\nlet f () = g ExtractedConstant\n" + """ +module M + +[] +let ExtractedConstant = -1 + +let f () = g ExtractedConstant +""" Assert.Equal(expected, extracted extractToLiteral code "-1") +let private moduleConstant = + """ +module M + +let f () = g 42 +""" + [] let ``Literal is offered only for constants outside types`` () = - Assert.Equal([ extractToLetBinding; extractToLiteral ], titlesFor "module M\n\nlet f () = g 42\n" "42") - Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\ntype T() =\n member _.M() = 42\n" "42") - Assert.Equal([ extractToLetBinding ], titlesFor "module M\n\nlet f x = g (x + 1)\n" "x + 1") + let memberConstant = + """ +module M + +type T() = + member _.M() = 42 +""" + + let expression = + """ +module M + +let f x = g (x + 1) +""" + + Assert.Equal([ extractToLetBinding; extractToLiteral ], titlesFor moduleConstant "42") + Assert.Equal([ extractToLetBinding ], titlesFor memberConstant "42") + Assert.Equal([ extractToLetBinding ], titlesFor expression "x + 1") + +let private mapIncrement = + """ +module M + +let f xs = + xs |> List.map (fun x -> x + 1) +""" [] [] @@ -183,56 +395,146 @@ let ``Literal is offered only for constants outside types`` () = [ x")>] [] let ``Caret in the header of a parenthesized lambda extracts the lambda`` (marker: string) = - let code = "module M\n\nlet f xs =\n xs |> List.map (fun x -> x + 1)\n" - let expected = - "module M\n\nlet f xs =\n let extracted = fun x -> x + 1\n xs |> List.map extracted\n" + """ +module M - Assert.Equal([ extractToLetBinding ], titlesAt code (caretAt code marker)) - Assert.Equal(expected, extractedAt extractToLetBinding code (caretAt code marker)) +let f xs = + let extracted = fun x -> x + 1 + xs |> List.map extracted +""" -[] -[ List.map (fun x -> x + 1)\n", "x + 1)")>] -[ List.map (fun x -> x + 1)\n", "+ 1)")>] -[ List.map (fun x -> x + 1)\n", "(fun")>] -[ x + 1\n", "fun")>] -let ``No action without a selection outside a parenthesized lambda header`` (code: string, marker: string) = - Assert.Empty(titlesAt code (caretAt code marker)) + Assert.Equal([ extractToLetBinding ], titlesAt mapIncrement (caretAt mapIncrement marker)) + Assert.Equal(expected, extractedAt extractToLetBinding mapIncrement (caretAt mapIncrement marker)) [] -[] -[] -[] -let ``Constant at the caret becomes a literal without a selection`` (marker: string) = - let code = "module M\n\nlet greet name =\n printfn \"Hello %s\" name\n" +[] +[] +[] +let ``No action without a selection outside a parenthesized lambda header`` (marker: string) = + Assert.Empty(titlesAt mapIncrement (caretAt mapIncrement marker)) - let expected = - "module M\n\n[]\nlet ExtractedConstant = \"Hello %s\"\n\nlet greet name =\n printfn ExtractedConstant name\n" +[] +let ``No action without a selection on a lambda without parentheses`` () = + let code = + """ +module M + +let f = fun x -> x + 1 +""" - Assert.Equal(expected, extractedAt extractToLiteral code (caretAt code marker)) + Assert.Empty(titlesAt code (caretAt code "fun")) + +[] +[] +[] +[] +let ``Constant at the caret becomes a literal without a selection`` (marker: string, offset: int) = + let caret = TextSpan(greet.IndexOf(marker, StringComparison.Ordinal) + offset, 0) + Assert.Equal(greetWithLiteral, extractedAt extractToLiteral greet caret) [] let ``Only the literal is offered without a selection`` () = - let code = "module M\n\nlet f () = g 42\n" - Assert.Equal([ extractToLiteral ], titlesAt code (caretAt code "2\n")) + let caret = TextSpan(moduleConstant.IndexOf("42", StringComparison.Ordinal) + 1, 0) + + Assert.Equal([ extractToLiteral ], titlesAt moduleConstant caret) [] -[] -[] -[] +[] +[] +[] let ``No action without a selection`` (code: string, marker: string) = Assert.Empty(titlesAt code (caretAt code marker)) [] -[] -[] -[] -[] -[ 0 -> v\n | _ -> 0\n", "v > 0")>] -[] -[] -[] -[] -[ 0\n", "s.Length > 0")>] -[] +[] +[] +[] +[] +[ 0 -> v + | _ -> 0 +""", + "v > 0")>] +[] +[] +[] +[] +[ 0 +""", + "s.Length > 0")>] +[] let ``No action`` (code: string, selected: string) = Assert.Empty(titlesFor code selected) From d1c6df716a02b7102dab290f0e4024a689600291 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 21:36:07 +0200 Subject: [PATCH 6/6] Link the release note to the pull request and move it to a random line of its section Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 3ff872ad743..4d63626deed 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -1,8 +1,8 @@ ### Added +* **Extract to let binding** and **Extract to literal** refactorings for a selected expression. The value is bound in front of the statement that uses it, so it is computed before the code that preceded the selection in that statement; a constant in a module-level declaration becomes a `[]` in front of that declaration, also with just the caret inside the constant. ([PR #20537](https://github.com/dotnet/fsharp/pull/20537)) * Code-fixes for FS3888 (compiler-semantic attribute on the `.fs` but not the `.fsi`): copy the attribute into the `.fsi`, or remove it from the `.fs`. ([Issue #19560](https://github.com/dotnet/fsharp/issues/19560), [PR #19880](https://github.com/dotnet/fsharp/pull/19880)) * Expand `` in IDE tooltips, completion, and signature help, inheriting XML documentation from base classes, interfaces, overridden members, and constructors. ([Issue #19175](https://github.com/dotnet/fsharp/issues/19175), [PR #19188](https://github.com/dotnet/fsharp/pull/19188)) -* **Extract to let binding** and **Extract to literal** refactorings for a selected expression. The value is bound in front of the statement that uses it, so it is computed before the code that preceded the selection in that statement; a constant in a module-level declaration becomes a `[]` in front of that declaration, also with just the caret inside the constant. ### Fixed