From b50c1035153afb28f69b80cc5c8c47b3985e8cc5 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 15:02:22 +0200 Subject: [PATCH 1/9] Add a refactoring between F# and .NET-compatible optional member parameters ?x: T is optional for F# callers and gives the body an option; [] x: T is optional for C# and VB callers too and gives the body the value. The refactoring moves the default between defaultArg x c in the body and the attribute, adds the InteropServices open when it is missing, and leaves call sites alone: F# accepts both forms the same way. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../src/FSharp.Editor/FSharp.Editor.fsproj | 1 + .../src/FSharp.Editor/FSharp.Editor.resx | 9 +- .../ConvertOptionalParameterDefaultValue.fs | 460 ++++++++++++++++++ .../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 + ...nvertOptionalParameterDefaultValueTests.fs | 132 +++++ 19 files changed, 730 insertions(+), 4 deletions(-) create mode 100644 vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs create mode 100644 vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 2357fbcdff4..6b54fb576d2 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -6,6 +6,7 @@ * 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)) * Added a **Tools > Options > F# Tools > Compiler** option, **"Use the .NET SDK F# compiler for builds"**, on by default. Visual Studio uses the .NET SDK F# compiler when the project supplies SDK paths, matching `dotnet build`. Otherwise, it uses the bundled .NET Framework compiler. Turn the option off to use the bundled compiler for all projects. Changes are saved on Apply or OK; Cancel discards unsaved changes. Command-line builds are unaffected. ([Issue #20484](https://github.com/dotnet/fsharp/issues/20484), [PR #20485](https://github.com/dotnet/fsharp/pull/20485)) * F# types, modules, members and values now appear in the GitHub Copilot Chat `#` mention picker, and attach their declaration source as context. ([PR #20409](https://github.com/dotnet/fsharp/pull/20409)) +* Refactoring to switch a member's optional parameter between F# `?x: T` and the .NET-compatible `[] x: T`, moving the default between `defaultArg x c` in the body and the attribute; F# call sites keep working unchanged. ### Fixed diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj index f5d6a8e0b04..e0a96d15e46 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.fsproj @@ -109,6 +109,7 @@ + diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx index 8d1a5854269..074eda7aa43 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx @@ -368,9 +368,10 @@ Use live (unsaved) buffers for analysis Returns: - - {0} in {1} -{2} - Tooltip of an F# declaration in the Copilot Chat "#" picker, as Copilot shows a C# one. {0} is the kind of declaration (Class, Method, Field...), {1} the file name, {2} the declaration's name. + + Use [<Optional; DefaultParameterValue>] for optional parameter + + + Use F# '?' optional parameter \ No newline at end of file diff --git a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs new file mode 100644 index 00000000000..65c0bdf547c --- /dev/null +++ b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs @@ -0,0 +1,460 @@ +// 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 +open Microsoft.CodeAnalysis.CodeActions +open Microsoft.CodeAnalysis.CodeRefactorings +open Microsoft.CodeAnalysis.Formatting +open Microsoft.CodeAnalysis.Text + +open FSharp.Compiler.CodeAnalysis +open FSharp.Compiler.Symbols +open FSharp.Compiler.Syntax +open FSharp.Compiler.Text + +open CancellableTasks + +[] +module private OptionalParameterDefaultValueConversion = + + [] + type Form = + /// `?x: T` + | FSharp of optionalValRange: range + /// `[] x: T`, with the attribute lists holding only those attributes. + | DotNet of lists: SynAttributeList list * defaultValue: SynExpr voption + + [] + type Parameter = + { + Ident: Ident + TypeName: SynType + Form: Form + MemberName: Ident + MemberBinding: SynBinding + } + + [] + type private Search = + | Searching + | Found of Parameter voption + + let private spanOf (sourceText: SourceText) (m: range) = + RoslynHelpers.FSharpRangeToTextSpan(sourceText, m) + + let private hasText (text: string) (ident: Ident) = + String.Equals(ident.idText, text, StringComparison.Ordinal) + + let private isSame (expr: SynExpr) (other: SynExpr) = obj.ReferenceEquals(expr, other) + + [] + let private (|SingleIdent|_|) (expr: SynExpr) = + match expr with + | SynExpr.Ident ident + | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ ident ])) -> ValueSome ident + | _ -> ValueNone + + let private isAttribute (names: string list) (attribute: SynAttribute) = + attribute.Target.IsNone + && match List.tryLast attribute.TypeName.LongIdent with + | Some name -> names |> List.exists (fun candidate -> hasText candidate name) + | None -> false + + let private isOptional = isAttribute [ "Optional"; "OptionalAttribute" ] + + let private isDefaultParameterValue = + isAttribute [ "DefaultParameterValue"; "DefaultParameterValueAttribute" ] + + let private isStruct = isAttribute [ "Struct"; "StructAttribute" ] + + /// The annotated parameter the pattern declares, with the attributes around it and the range of `?x`, if any. + let rec private tryShape (attributes: SynAttributes) (pat: SynPat) = + match pat with + | SynPat.Paren(pat = inner) -> tryShape attributes inner + | SynPat.Attrib(pat = inner; attributes = more) -> tryShape [ yield! attributes; yield! more ] inner + | SynPat.Typed(pat = SynPat.OptionalVal(ident, m); targetType = typeName) -> + ValueSome(struct (ident, typeName, attributes, ValueSome m)) + | SynPat.Typed(pat = SynPat.Named(ident = SynIdent(ident, _); isThisVal = false); targetType = typeName) -> + ValueSome(struct (ident, typeName, attributes, ValueNone)) + | _ -> ValueNone + + let private tryForm (attributes: SynAttributes) (optionalValRange: range voption) = + match optionalValRange with + | ValueSome _ when attributes |> List.exists (fun list -> List.exists isStruct list.Attributes) -> ValueNone + | ValueSome m -> ValueSome(Form.FSharp m) + | ValueNone -> + let isOptionalAttribute attribute = + isOptional attribute || isDefaultParameterValue attribute + + let lists = + attributes + |> List.filter (fun list -> List.exists isOptionalAttribute list.Attributes) + + let listed = lists |> List.collect _.Attributes + + if not (List.exists isOptional listed && List.forall isOptionalAttribute listed) then + ValueNone + else + match List.tryFind isDefaultParameterValue listed with + | None -> ValueSome(Form.DotNet(lists, ValueNone)) + | Some attribute -> + match attribute.ArgExpr with + | SynExpr.Paren(expr = SynExpr.Const _ as constant) -> ValueSome(Form.DotNet(lists, ValueSome constant)) + | _ -> ValueNone + + let rec private tryMember (path: SyntaxVisitorPath) = + match path with + | SyntaxNode.SynBinding(SynBinding(headPat = SynPat.LongIdent(longDotId = SynLongIdent(id = ids))) as binding) :: SyntaxNode.SynMemberDefn(SynMemberDefn.Member _) :: _ -> + match List.tryLast ids with + | Some name -> ValueSome(struct (name, binding)) + | None -> ValueNone + | SyntaxNode.SynPat _ :: rest -> tryMember rest + | _ -> ValueNone + + let tryParameter (caret: pos) (parseTree: ParsedInput) = + let search = + (Search.Searching, parseTree) + ||> ParsedInput.fold (fun search path node -> + match search, node with + | Search.Searching, SyntaxNode.SynPat pat when Position.posGeq caret pat.Range.Start && Position.posGeq pat.Range.End caret -> + match tryShape [] pat, tryMember path with + | ValueSome(struct (ident, typeName, attributes, optionalValRange)), ValueSome(struct (memberName, binding)) -> + tryForm attributes optionalValRange + |> ValueOption.map (fun form -> + { + Ident = ident + TypeName = typeName + Form = form + MemberName = memberName + MemberBinding = binding + }) + |> Search.Found + | _ -> Search.Searching + | _ -> search) + + match search with + | Search.Found parameter -> parameter + | Search.Searching -> ValueNone + + /// Whether the constant can be the `DefaultParameterValue` of a parameter of the named type. + let private isConstantOfType (typeText: string) (constant: SynConst) = + match typeText, constant with + | ("int" | "int32"), SynConst.Int32 _ + | "int64", SynConst.Int64 _ + | "int16", SynConst.Int16 _ + | "sbyte", SynConst.SByte _ + | "byte", SynConst.Byte _ + | "uint16", SynConst.UInt16 _ + | "uint32", SynConst.UInt32 _ + | "uint64", SynConst.UInt64 _ + | ("float" | "double"), SynConst.Double _ + | ("float32" | "single"), SynConst.Single _ + | "bool", SynConst.Bool _ + | "char", SynConst.Char _ + | "string", SynConst.String _ -> true + | _ -> false + + let private tryUseNode (parseTree: ParsedInput) (useRange: range) = + (useRange.Start, parseTree) + ||> ParsedInput.tryPickLast (fun path node -> + match node with + | SyntaxNode.SynExpr(SynExpr.Ident ident as expr) + | SyntaxNode.SynExpr(SynExpr.LongIdent(longDotId = SynLongIdent(id = ident :: _)) as expr) when + Position.posEq ident.idRange.Start useRange.Start + && Position.posEq ident.idRange.End useRange.End + -> + Some(expr, path) + | _ -> None) + + /// `defaultArg x c` around a use of the parameter, with its constant and what contains it. + let private tryDefaultArgUse (parseTree: ParsedInput) (useRange: range) = + match tryUseNode parseTree useRange with + | Some(node, + SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = SingleIdent func; argExpr = arg) as inner) :: SyntaxNode.SynExpr(SynExpr.App( + isInfix = false; funcExpr = funcExpr; argExpr = (SynExpr.Const _ as defaultValue)) as application) :: rest) when + isSame arg node && hasText "defaultArg" func && isSame funcExpr inner + -> + ValueSome(struct (application, defaultValue, rest)) + | _ -> ValueNone + + /// The whole line of `let x = defaultArg x c` when that line does nothing but rebind the parameter. + let private tryShadowingLine (sourceText: SourceText) (name: string) (application: SynExpr) (path: SyntaxVisitorPath) = + match path with + | SyntaxNode.SynBinding(SynBinding(headPat = SynPat.Named(ident = SynIdent(ident, _)); expr = rhs; trivia = trivia)) :: SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse) :: _ when + isSame rhs application + && hasText name ident + && not letOrUse.IsRecursive + && not letOrUse.IsBang + && letOrUse.Bindings.Length = 1 + -> + let keyword = trivia.LeadingKeyword.Range + let line = sourceText.Lines[Line.toZ keyword.StartLine] + let lineText = line.ToString() + + if + application.Range.EndLine = keyword.StartLine + && String.IsNullOrWhiteSpace(lineText.Substring(0, keyword.StartColumn)) + && String.IsNullOrWhiteSpace(lineText.Substring application.Range.EndColumn) + then + ValueSome(TextSpan.FromBounds(line.Start, line.EndIncludingLineBreak)) + else + ValueNone + | _ -> ValueNone + + /// `?x: T` with every use being `defaultArg x c` becomes `[] x: T`. + let tryToDotNetChanges + (sourceText: SourceText) + (parseTree: ParsedInput) + (parameter: Parameter) + (optionalValRange: range) + (uses: range list) + = + let found = + (ValueSome [], uses) + ||> List.fold (fun found useRange -> + match found, tryDefaultArgUse parseTree useRange with + | ValueSome found, ValueSome defaultArgUse -> ValueSome(defaultArgUse :: found) + | _ -> ValueNone) + + let textOf (expr: SynExpr) = + sourceText.ToString(spanOf sourceText expr.Range) + + let typeText = + sourceText.ToString(spanOf sourceText parameter.TypeName.Range).Trim() + + let attributeText = + match found with + | ValueSome [] -> ValueSome "[] " + | ValueSome(struct (_, (SynExpr.Const(constant, _) as defaultValue), _) :: others) when + isConstantOfType typeText constant + && others + |> List.forall (fun struct (_, other, _) -> String.Equals(textOf other, textOf defaultValue, StringComparison.Ordinal)) + -> + ValueSome $"[] " + | _ -> ValueNone + + match found, attributeText with + | ValueSome found, ValueSome attributeText -> + [ + TextChange(TextSpan((spanOf sourceText optionalValRange).Start, 1), attributeText) + + for struct (application, _, path) in found do + match tryShadowingLine sourceText parameter.Ident.idText application path with + | ValueSome line -> TextChange(line, "") + | ValueNone -> TextChange(spanOf sourceText application.Range, parameter.Ident.idText) + ] + |> List.sortBy _.Span.Start + |> ValueSome + | _ -> ValueNone + + let private interopServices = [ "System"; "Runtime"; "InteropServices" ] + + let private hasInteropServicesOpen (parseTree: ParsedInput) = + (false, parseTree) + ||> ParsedInput.fold (fun found _ node -> + found + || match node with + | SyntaxNode.SynModule(SynModuleDecl.Open(target = SynOpenDeclTarget.ModuleOrNamespace(longId = SynLongIdent(id = ids)))) -> + (ids |> List.map _.idText) = interopServices + | _ -> false) + + let withInteropServicesOpen (parseTree: ParsedInput) (memberName: Ident) (text: SourceText) = + if hasInteropServicesOpen parseTree then + text + else + let insertionContext = + FSharp.Compiler.EditorServices.ParsedInput.FindNearestPointToInsertOpenDeclaration + memberName.idRange.StartLine + parseTree + (List.toArray interopServices) + FSharp.Compiler.EditorServices.OpenStatementInsertionPoint.TopLevel + + OpenDeclarationHelper.insertOpenDeclaration text insertionContext (String.Join(".", interopServices)) + |> fst + + let private leadingSpaces (line: TextLine) = + let text = line.ToString() + text.Length - text.TrimStart(' ').Length + + let private lineBreakOf (sourceText: SourceText) (line: TextLine) = + match line.EndIncludingLineBreak - line.End with + | 0 -> Environment.NewLine + | length -> sourceText.ToString(TextSpan(line.End, length)) + + let private listRemoval (sourceText: SourceText) (list: SynAttributeList) = + let listSpan = spanOf sourceText list.Range + let line = sourceText.Lines.GetLineFromPosition listSpan.End + let after = sourceText.ToString(TextSpan.FromBounds(listSpan.End, line.End)) + TextSpan(listSpan.Start, listSpan.Length + after.Length - after.TrimStart().Length) + + /// `[] x: T` becomes `?x: T` with `let x = defaultArg x c` starting the body. + let tryToFSharpChanges + (sourceText: SourceText) + (indentSize: int) + (parameter: Parameter) + (lists: SynAttributeList list) + (defaultValue: SynExpr voption) + = + let name = parameter.Ident.idText + + let defaultText = + match defaultValue with + | ValueSome value -> sourceText.ToString(spanOf sourceText value.Range) + | ValueNone -> "Unchecked.defaultof<_>" + + let declaration = $"let {name} = defaultArg {name} {defaultText}" + + let (SynBinding(expr = body; returnInfo = returnInfo; trivia = trivia)) = + parameter.MemberBinding + + let body = + match returnInfo, body with + | Some _, SynExpr.Typed(expr = inner) -> inner + | _ -> body + + let memberLine = sourceText.Lines[Line.toZ trivia.LeadingKeyword.Range.StartLine] + let lineBreak = lineBreakOf sourceText memberLine + + let bodyChange = + match trivia.EqualsRange with + | Some equals when body.Range.StartLine > equals.EndLine -> + let line = sourceText.Lines[Line.toZ body.Range.StartLine] + ValueSome(TextChange(TextSpan(line.Start, 0), $"{String(' ', body.Range.StartColumn)}{declaration}{lineBreak}")) + | Some equals when body.Range.StartLine = body.Range.EndLine -> + let indent = String(' ', leadingSpaces memberLine + indentSize) + + ValueSome( + TextChange( + TextSpan.FromBounds((spanOf sourceText equals).End, (spanOf sourceText body.Range).Start), + $"{lineBreak}{indent}{declaration}{lineBreak}{indent}" + ) + ) + | _ -> ValueNone + + bodyChange + |> ValueOption.map (fun bodyChange -> + [ + for list in lists do + TextChange(listRemoval sourceText list, "") + + TextChange(TextSpan((spanOf sourceText parameter.Ident.idRange).Start, 0), "?") + bodyChange + ] + |> List.sortBy _.Span.Start) + +[] +type internal FSharpConvertOptionalParameterDefaultValueRefactoring [] () = + inherit CodeRefactoringProvider() + + static let hasSignatureFile (document: Document) = + let signaturePath = document.FilePath + "i" + + document.Project.Documents + |> Seq.exists (fun d -> String.Equals(d.FilePath, signaturePath, StringComparison.OrdinalIgnoreCase)) + + static let tryGetSymbolUse (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (ident: Ident) = + let line = sourceText.Lines[Line.toZ ident.idRange.EndLine].ToString() + checkResults.GetSymbolUseAtLocation(ident.idRange.EndLine, ident.idRange.EndColumn, line, [ ident.idText ]) + + static let isConvertibleMember (symbol: FSharpSymbol) = + match symbol with + | :? FSharpMemberOrFunctionOrValue as mfv -> + not ( + mfv.IsOverrideOrExplicitInterfaceImplementation + || mfv.IsDispatchSlot + || mfv.IsConstructor + || mfv.IsExtensionMember + ) + | _ -> false + + override _.ComputeRefactoringsAsync context = + cancellableTask { + let document = context.Document + + if not (document.IsFSharpSignatureFile || hasSignatureFile document) then + let! cancellationToken = CancellableTask.getCancellationToken () + let! sourceText = document.GetTextAsync cancellationToken + let! parseResults = document.GetFSharpParseResultsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) + + let caret = + let linePosition = sourceText.Lines.GetLinePosition context.Span.Start + Position.mkPos (Line.fromZ linePosition.Line) linePosition.Character + + match OptionalParameterDefaultValueConversion.tryParameter caret parseResults.ParseTree with + | ValueNone -> () + | ValueSome parameter -> + let! _, checkResults = + document.GetFSharpParseAndCheckResultsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) + + match tryGetSymbolUse checkResults sourceText parameter.MemberName with + | Some memberUse when isConvertibleMember memberUse.Symbol -> + match parameter.Form with + | OptionalParameterDefaultValueConversion.Form.FSharp optionalValRange -> + match tryGetSymbolUse checkResults sourceText parameter.Ident with + | Some parameterUse -> + let memberRange = parameter.MemberBinding.RangeOfBindingWithRhs + + let uses = + checkResults.GetUsesOfSymbolInFile(parameterUse.Symbol, cancellationToken = cancellationToken) + // Named arguments at call sites are uses of the parameter too; only the body matters here. + |> Seq.filter (fun symbolUse -> + not symbolUse.IsFromDefinition + && Position.posGeq symbolUse.Range.Start memberRange.Start + && Position.posGeq memberRange.End symbolUse.Range.End) + |> Seq.map _.Range + |> List.ofSeq + + match + OptionalParameterDefaultValueConversion.tryToDotNetChanges + sourceText + parseResults.ParseTree + parameter + optionalValRange + uses + with + | ValueSome changes -> + let title = SR.UseDotNetOptionalParameter() + + let changedDocument = + cancellableTask { + let changed = + sourceText.WithChanges changes + |> OptionalParameterDefaultValueConversion.withInteropServicesOpen + parseResults.ParseTree + parameter.MemberName + + return document.WithText changed + } + + context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) + | ValueNone -> () + | None -> () + | OptionalParameterDefaultValueConversion.Form.DotNet(lists, defaultValue) -> + let! options = document.GetOptionsAsync cancellationToken + + let indentSize = + options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) + + match + OptionalParameterDefaultValueConversion.tryToFSharpChanges + sourceText + indentSize + parameter + lists + defaultValue + with + | ValueSome changes -> + let title = SR.UseFSharpOptionalParameter() + + let changedDocument = + cancellableTask { return document.WithText(sourceText.WithChanges changes) } + + context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) + | ValueNone -> () + | _ -> () + } + |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf index 5af8dfaab66..7043f918a5c 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf @@ -395,11 +395,21 @@ Zobrazit poznámky v Rychlých informacích Formátování + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Použít syntaxi lambda jazyka F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Pokud chcete změnit hodnotu, použijte <-. diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf index e7cf1e34711..2945a9b1ae7 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf @@ -395,11 +395,21 @@ Hinweise in QuickInfo anzeigen Formatierung + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax F#-Lambdasyntax verwenden + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value "<-" zum Ändern des Werts verwenden diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf index a763e64aca8..0a5ba42c4e2 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf @@ -395,11 +395,21 @@ Mostrar comentarios en Información rápida Formato + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Usar la sintaxis lambda de F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Usar "<-" para mutar el valor diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf index 6fd805187e5..1f094b4be3f 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf @@ -395,11 +395,21 @@ Afficher les notes dans Info express Mise en forme + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Utiliser la syntaxe lambda F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Utiliser '<-' pour muter la valeur diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf index 766483153f0..abe292a166a 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf @@ -395,11 +395,21 @@ Mostra i commenti in Informazioni rapide Formattazione + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Usa la sintassi lambda di F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Usare '<-' per modificare il valore diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf index 30834f89e84..952a49c0700 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf @@ -395,11 +395,21 @@ F# 構文規則に準拠した改行を追加して、署名を指定された 書式設定 + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax F# のラムダ構文を使用する + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value '<-' を使用して値を変換する diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf index 1ef22e3047d..ca8314d0795 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf @@ -395,11 +395,21 @@ F# 구문 규칙에 맞는 줄 바꿈을 추가하여 지정된 너비에 시그 서식 + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax F# 람다 구문 사용 + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value '<-'를 사용하여 값 변경 diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf index 1091913daa2..6323a9ff4c9 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf @@ -395,11 +395,21 @@ Pokaż uwagi w szybkich informacjach Formatowanie + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Użyj składni wyrażenia lambda języka F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Użyj znaku „<-” w celu zmodyfikowania wartości 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 28e5ff703d2..14cc2835582 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf @@ -395,11 +395,21 @@ Mostrar os comentários nas Informações Rápidas Formatação + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Usar a sintaxe lambda F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Usar '<-' para modificar o valor diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf index 5787d9818f3..656f7b84604 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf @@ -395,11 +395,21 @@ Show remarks in Quick Info Форматирование + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax Использовать синтаксис лямбда F# + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Используйте "<-", чтобы изменить значение diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf index 933e42779d4..1068d05a8c7 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf @@ -395,11 +395,21 @@ Açıklamaları Hızlı Bilgide göster Biçimlendirme + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax F# lambda söz dizimini kullan + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value Değeri değiştirmek için '<-' kullan 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 69381cc377d..c2afd4357f6 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf @@ -395,11 +395,21 @@ Show remarks in Quick Info 正在格式化 + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax 使用 F# lambda 语法 + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value 使用 "<-" 来更改值 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 4bdafbf0121..7d74625d3e6 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf @@ -395,11 +395,21 @@ Show remarks in Quick Info 格式化 + + Use [<Optional; DefaultParameterValue>] for optional parameter + Use [<Optional; DefaultParameterValue>] for optional parameter + + Use F# lambda syntax 使用 F# lambda 語法 + + Use F# '?' optional parameter + Use F# '?' optional parameter + + Use '<-' to mutate value 使用 '<-' 來變動值 diff --git a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj index d5d1a09c79a..c2ad3f8d305 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj +++ b/vsintegration/tests/FSharp.Editor.Tests/FSharp.Editor.Tests.fsproj @@ -79,6 +79,7 @@ + diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs new file mode 100644 index 00000000000..792ca51f0fc --- /dev/null +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs @@ -0,0 +1,132 @@ +module FSharp.Editor.Tests.Refactors.ConvertOptionalParameterDefaultValueTests + +open System +open System.Threading + +open Microsoft.CodeAnalysis.CodeActions +open Microsoft.CodeAnalysis.CodeRefactorings +open Microsoft.CodeAnalysis.Text + +open Microsoft.VisualStudio.FSharp.Editor +open Microsoft.VisualStudio.FSharp.Editor.CancellableTasks + +open Xunit + +open FSharp.Compiler.Diagnostics + +open FSharp.Editor.Tests.Helpers +open FSharp.Editor.Tests.Refactors.RefactorTestFramework + +let private caretAt (code: string) (marker: string) = + code.IndexOf(marker, StringComparison.Ordinal) + +let private refactored (code: string) (marker: string) = + use context = TestContext.CreateWithCode code + + let document = + tryRefactor code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) + + let _, checkResults = + document.GetFSharpParseAndCheckResultsAsync "test" + |> CancellableTask.runSynchronouslyWithoutCancellation + + Assert.Empty( + checkResults.Diagnostics + |> Array.filter (fun diagnostic -> diagnostic.Severity = FSharpDiagnosticSeverity.Error) + ) + + (document.GetTextAsync() |> GetTaskResult).ToString() + +let private actionsAt (code: string) (marker: string) = + use context = TestContext.CreateWithCode code + tryGetRefactoringActions code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) + +[] +let ``Optional parameter with a default value becomes a .NET optional parameter and the open is added`` () = + let before = + "module M\n\ntype Greeter() =\n member _.Greet(name: string, ?greeting: string) =\n let greeting = defaultArg greeting \"Hello\"\n $\"{greeting}, {name}\"\n\nlet a = Greeter().Greet(\"Ada\")\nlet b = Greeter().Greet(\"Ada\", greeting = \"Hi\")\n" + + let after = + "module M\n\nopen System.Runtime.InteropServices\n\ntype Greeter() =\n member _.Greet(name: string, [] greeting: string) =\n $\"{greeting}, {name}\"\n\nlet a = Greeter().Greet(\"Ada\")\nlet b = Greeter().Greet(\"Ada\", greeting = \"Hi\")\n" + + Assert.Equal(after, refactored before "?greeting") + +[] +[] step: int) =\n step + 1\n\nlet n = Counter.Next()\n")>] +[] step: float) =\n float value + step\n")>] +let ``Shadowing default converts both ways`` (fsharpForm: string, dotNetForm: string) = + Assert.Equal(dotNetForm, refactored fsharpForm "?step") + Assert.Equal(fsharpForm, refactored dotNetForm "step:") + +[] +let ``Inline defaults are replaced by the parameter`` () = + let before = + "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, ?step: int) = value + defaultArg step 1\n" + + let after = + "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, [] step: int) = value + step\n" + + Assert.Equal(after, refactored before "?step") + +[] +let ``Body on the member line moves below it when converting back`` () = + let before = + "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, [] step: int) = value + step\n" + + let after = + "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, ?step: int) =\n let step = defaultArg step 1\n value + step\n" + + Assert.Equal(after, refactored before "step:") + +[] +[] flag: bool) = 1\n")>] +[] step: int) =\n step + 1\n", + "step:", + "module M\n\nopen System.Runtime.InteropServices\n\ntype C() =\n static member M(?step: int) =\n let step = defaultArg step Unchecked.defaultof<_>\n step + 1\n")>] +let ``Optional without a default value`` (before: string, marker: string, after: string) = + Assert.Equal(after, refactored before marker) + +[] +let ``Title names the target form`` () = + let fsharpForm = + "module M\n\ntype C() =\n static member M(?x: int) = defaultArg x 0\n" + + let dotNetForm = + "module M\n\nopen System.Runtime.InteropServices\n\ntype C() =\n static member M([] x: int) = x\n" + + Assert.Equal("Use [] for optional parameter", (actionsAt fsharpForm "?x" |> Seq.exactlyOne).Title) + Assert.Equal("Use F# '?' optional parameter", (actionsAt dotNetForm "x:" |> Seq.exactlyOne).Title) + +[] +[] +[] +[] +[] +[] +[] ?x: int) = defaultValueArg x 0\n", "?x")>] +[] x: int) = x\n", "x:")>] +[ int\n default _.M(?x: int) = defaultArg x 0\n", "?x: int)")>] +[] +let ``No action`` (code: string, marker: string) = Assert.Empty(actionsAt code marker) + +[] +let ``No action when the file has a signature`` () = + let code = "module M\n\ntype C() =\n static member M(?x: int) = defaultArg x 0\n" + + let signature = + "module M\n\ntype C =\n new: unit -> C\n static member M: ?x: int -> int\n" + + let document = RoslynTestHelpers.GetFsiAndFsDocuments signature code |> Seq.last + let actions = ResizeArray() + + let context = + CodeRefactoringContext(document, TextSpan(caretAt code "?x", 1), (fun action -> actions.Add action), CancellationToken.None) + + (new FSharpConvertOptionalParameterDefaultValueRefactoring()).ComputeRefactoringsAsync(context).GetAwaiter().GetResult() + + Assert.False(document.IsFSharpSignatureFile) + Assert.Empty(actions) From 73e2103bfa7239d8b7ed42fba139a76db5d7be13 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 15:42:13 +0200 Subject: [PATCH 2/9] Write the .NET optional parameter test code as multi-line strings Co-Authored-By: Claude Opus 5 (1M context) --- ...nvertOptionalParameterDefaultValueTests.fs | 259 +++++++++++++++--- 1 file changed, 227 insertions(+), 32 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs index 792ca51f0fc..93f7e6c5793 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs @@ -44,18 +44,77 @@ let private actionsAt (code: string) (marker: string) = [] let ``Optional parameter with a default value becomes a .NET optional parameter and the open is added`` () = let before = - "module M\n\ntype Greeter() =\n member _.Greet(name: string, ?greeting: string) =\n let greeting = defaultArg greeting \"Hello\"\n $\"{greeting}, {name}\"\n\nlet a = Greeter().Greet(\"Ada\")\nlet b = Greeter().Greet(\"Ada\", greeting = \"Hi\")\n" + """ +module M + +type Greeter() = + member _.Greet(name: string, ?greeting: string) = + let greeting = defaultArg greeting "Hello" + $"{greeting}, {name}" + +let a = Greeter().Greet("Ada") +let b = Greeter().Greet("Ada", greeting = "Hi") +""" let after = - "module M\n\nopen System.Runtime.InteropServices\n\ntype Greeter() =\n member _.Greet(name: string, [] greeting: string) =\n $\"{greeting}, {name}\"\n\nlet a = Greeter().Greet(\"Ada\")\nlet b = Greeter().Greet(\"Ada\", greeting = \"Hi\")\n" + """ +module M + +open System.Runtime.InteropServices + +type Greeter() = + member _.Greet(name: string, [] greeting: string) = + $"{greeting}, {name}" + +let a = Greeter().Greet("Ada") +let b = Greeter().Greet("Ada", greeting = "Hi") +""" Assert.Equal(after, refactored before "?greeting") [] -[] step: int) =\n step + 1\n\nlet n = Counter.Next()\n")>] -[] step: float) =\n float value + step\n")>] +[] step: int) = + step + 1 + +let n = Counter.Next() +""")>] +[] step: float) = + float value + step +""")>] let ``Shadowing default converts both ways`` (fsharpForm: string, dotNetForm: string) = Assert.Equal(dotNetForm, refactored fsharpForm "?step") Assert.Equal(fsharpForm, refactored dotNetForm "step:") @@ -63,68 +122,204 @@ let ``Shadowing default converts both ways`` (fsharpForm: string, dotNetForm: st [] let ``Inline defaults are replaced by the parameter`` () = let before = - "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, ?step: int) = value + defaultArg step 1\n" + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next(value: int, ?step: int) = value + defaultArg step 1 +""" let after = - "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, [] step: int) = value + step\n" + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next(value: int, [] step: int) = value + step +""" Assert.Equal(after, refactored before "?step") [] let ``Body on the member line moves below it when converting back`` () = let before = - "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, [] step: int) = value + step\n" + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next(value: int, [] step: int) = value + step +""" let after = - "module M\n\nopen System.Runtime.InteropServices\n\ntype Counter() =\n static member Next(value: int, ?step: int) =\n let step = defaultArg step 1\n value + step\n" + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next(value: int, ?step: int) = + let step = defaultArg step 1 + value + step +""" Assert.Equal(after, refactored before "step:") [] -[] flag: bool) = 1\n")>] -[] step: int) =\n step + 1\n", + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M([] flag: bool) = 1 +""")>] +[] step: int) = + step + 1 +""", "step:", - "module M\n\nopen System.Runtime.InteropServices\n\ntype C() =\n static member M(?step: int) =\n let step = defaultArg step Unchecked.defaultof<_>\n step + 1\n")>] + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M(?step: int) = + let step = defaultArg step Unchecked.defaultof<_> + step + 1 +""")>] let ``Optional without a default value`` (before: string, marker: string, after: string) = Assert.Equal(after, refactored before marker) +let private fsharpForm = + """ +module M + +type C() = + static member M(?x: int) = defaultArg x 0 +""" + [] let ``Title names the target form`` () = - let fsharpForm = - "module M\n\ntype C() =\n static member M(?x: int) = defaultArg x 0\n" - let dotNetForm = - "module M\n\nopen System.Runtime.InteropServices\n\ntype C() =\n static member M([] x: int) = x\n" + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M([] x: int) = x +""" Assert.Equal("Use [] for optional parameter", (actionsAt fsharpForm "?x" |> Seq.exactlyOne).Title) Assert.Equal("Use F# '?' optional parameter", (actionsAt dotNetForm "x:" |> Seq.exactlyOne).Title) [] -[] -[] -[] -[] -[] -[] ?x: int) = defaultValueArg x 0\n", "?x")>] -[] x: int) = x\n", "x:")>] -[ int\n default _.M(?x: int) = defaultArg x 0\n", "?x: int)")>] -[] +[] +[] +[] +[] +[] +[] ?x: int) = defaultValueArg x 0 +""", + "?x")>] +[] x: int) = x +""", + "x:")>] +[ int + default _.M(?x: int) = defaultArg x 0 +""", + "?x: int)")>] +[] let ``No action`` (code: string, marker: string) = Assert.Empty(actionsAt code marker) [] let ``No action when the file has a signature`` () = - let code = "module M\n\ntype C() =\n static member M(?x: int) = defaultArg x 0\n" - let signature = - "module M\n\ntype C =\n new: unit -> C\n static member M: ?x: int -> int\n" + """ +module M + +type C = + new: unit -> C + static member M: ?x: int -> int +""" + + let document = + RoslynTestHelpers.GetFsiAndFsDocuments signature fsharpForm |> Seq.last - let document = RoslynTestHelpers.GetFsiAndFsDocuments signature code |> Seq.last let actions = ResizeArray() let context = - CodeRefactoringContext(document, TextSpan(caretAt code "?x", 1), (fun action -> actions.Add action), CancellationToken.None) + CodeRefactoringContext(document, TextSpan(caretAt fsharpForm "?x", 1), (fun action -> actions.Add action), CancellationToken.None) (new FSharpConvertOptionalParameterDefaultValueRefactoring()).ComputeRefactoringsAsync(context).GetAwaiter().GetResult() From 21ff5aa790bda5ce33fc6b6bdcb5abb2230e1ff1 Mon Sep 17 00:00:00 2001 From: Andrii Chebukin Date: Mon, 14 Sep 2026 20:03:04 +0200 Subject: [PATCH 3/9] Offer a struct optional parameter when converting back from the .NET form [] x: T now converts back to either ?x: T with let x = defaultArg x c, or, from F# 10, [] ?x: T with let x = defaultValueArg x c. Co-Authored-By: Claude Opus 5 (1M context) --- docs/release-notes/.VisualStudio/18.vNext.md | 1 + .../src/FSharp.Editor/FSharp.Editor.resx | 3 + .../ConvertOptionalParameterDefaultValue.fs | 55 ++++++++---- .../FSharp.Editor/xlf/FSharp.Editor.cs.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.de.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.es.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.fr.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.it.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.ja.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.ko.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.pl.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.ru.xlf | 5 ++ .../FSharp.Editor/xlf/FSharp.Editor.tr.xlf | 5 ++ .../xlf/FSharp.Editor.zh-Hans.xlf | 5 ++ .../xlf/FSharp.Editor.zh-Hant.xlf | 5 ++ ...nvertOptionalParameterDefaultValueTests.fs | 87 +++++++++++++++++-- 17 files changed, 184 insertions(+), 27 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 6b54fb576d2..029a56a901d 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -7,6 +7,7 @@ * Added a **Tools > Options > F# Tools > Compiler** option, **"Use the .NET SDK F# compiler for builds"**, on by default. Visual Studio uses the .NET SDK F# compiler when the project supplies SDK paths, matching `dotnet build`. Otherwise, it uses the bundled .NET Framework compiler. Turn the option off to use the bundled compiler for all projects. Changes are saved on Apply or OK; Cancel discards unsaved changes. Command-line builds are unaffected. ([Issue #20484](https://github.com/dotnet/fsharp/issues/20484), [PR #20485](https://github.com/dotnet/fsharp/pull/20485)) * F# types, modules, members and values now appear in the GitHub Copilot Chat `#` mention picker, and attach their declaration source as context. ([PR #20409](https://github.com/dotnet/fsharp/pull/20409)) * Refactoring to switch a member's optional parameter between F# `?x: T` and the .NET-compatible `[] x: T`, moving the default between `defaultArg x c` in the body and the attribute; F# call sites keep working unchanged. +* Refactoring to switch a member's optional parameter between F# `?x: T` and the .NET-compatible `[] x: T`, moving the default between `defaultArg x c` in the body and the attribute; F# call sites keep working unchanged. From F# 10 the .NET form also converts back to `[] ?x: T` with `defaultValueArg x c`. ### Fixed diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx index 074eda7aa43..26722fc9e22 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx @@ -374,4 +374,7 @@ Use live (unsaved) buffers for analysis Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + \ No newline at end of file diff --git a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs index 65c0bdf547c..0b8d85cf6db 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs @@ -12,6 +12,7 @@ open Microsoft.CodeAnalysis.Formatting open Microsoft.CodeAnalysis.Text open FSharp.Compiler.CodeAnalysis +open FSharp.Compiler.Features open FSharp.Compiler.Symbols open FSharp.Compiler.Syntax open FSharp.Compiler.Text @@ -291,13 +292,15 @@ module private OptionalParameterDefaultValueConversion = let after = sourceText.ToString(TextSpan.FromBounds(listSpan.End, line.End)) TextSpan(listSpan.Start, listSpan.Length + after.Length - after.TrimStart().Length) - /// `[] x: T` becomes `?x: T` with `let x = defaultArg x c` starting the body. + /// `[] x: T` becomes `?x: T` with `let x = defaultArg x c` starting the body, + /// or `[] ?x: T` with `let x = defaultValueArg x c`. let tryToFSharpChanges (sourceText: SourceText) (indentSize: int) (parameter: Parameter) (lists: SynAttributeList list) (defaultValue: SynExpr voption) + (asStruct: bool) = let name = parameter.Ident.idText @@ -306,7 +309,13 @@ module private OptionalParameterDefaultValueConversion = | ValueSome value -> sourceText.ToString(spanOf sourceText value.Range) | ValueNone -> "Unchecked.defaultof<_>" - let declaration = $"let {name} = defaultArg {name} {defaultText}" + let struct (prefix, defaultFunction) = + if asStruct then + struct ("[] ?", "defaultValueArg") + else + struct ("?", "defaultArg") + + let declaration = $"let {name} = {defaultFunction} {name} {defaultText}" let (SynBinding(expr = body; returnInfo = returnInfo; trivia = trivia)) = parameter.MemberBinding @@ -341,7 +350,7 @@ module private OptionalParameterDefaultValueConversion = for list in lists do TextChange(listRemoval sourceText list, "") - TextChange(TextSpan((spanOf sourceText parameter.Ident.idRange).Start, 0), "?") + TextChange(TextSpan((spanOf sourceText parameter.Ident.idRange).Start, 0), prefix) bodyChange ] |> List.sortBy _.Span.Start) @@ -436,25 +445,33 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ let! options = document.GetOptionsAsync cancellationToken + let! _, langVersion = + document.GetFsharpParsingOptionsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) + let indentSize = options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) - match - OptionalParameterDefaultValueConversion.tryToFSharpChanges - sourceText - indentSize - parameter - lists - defaultValue - with - | ValueSome changes -> - let title = SR.UseFSharpOptionalParameter() - - let changedDocument = - cancellableTask { return document.WithText(sourceText.WithChanges changes) } - - context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) - | ValueNone -> () + let register (asStruct: bool) (title: string) = + match + OptionalParameterDefaultValueConversion.tryToFSharpChanges + sourceText + indentSize + parameter + lists + defaultValue + asStruct + with + | ValueSome changes -> + let changedDocument = + cancellableTask { return document.WithText(sourceText.WithChanges changes) } + + context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) + | ValueNone -> () + + register false (SR.UseFSharpOptionalParameter()) + + if LanguageVersion(langVersion).SupportsFeature LanguageFeature.SupportValueOptionsAsOptionalParameters then + register true (SR.UseFSharpStructOptionalParameter()) | _ -> () } |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf index 7043f918a5c..9634f409ee1 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.cs.xlf @@ -410,6 +410,11 @@ Zobrazit poznámky v Rychlých informacích Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Pokud chcete změnit hodnotu, použijte <-. diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf index 2945a9b1ae7..bc07a9ef789 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.de.xlf @@ -410,6 +410,11 @@ Hinweise in QuickInfo anzeigen Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value "<-" zum Ändern des Werts verwenden diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf index 0a5ba42c4e2..453057a78ab 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.es.xlf @@ -410,6 +410,11 @@ Mostrar comentarios en Información rápida Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Usar "<-" para mutar el valor diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf index 1f094b4be3f..8aae6fd49b1 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.fr.xlf @@ -410,6 +410,11 @@ Afficher les notes dans Info express Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Utiliser '<-' pour muter la valeur diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf index abe292a166a..2f598b6f7fe 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.it.xlf @@ -410,6 +410,11 @@ Mostra i commenti in Informazioni rapide Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Usare '<-' per modificare il valore diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf index 952a49c0700..ae6f7e188d7 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ja.xlf @@ -410,6 +410,11 @@ F# 構文規則に準拠した改行を追加して、署名を指定された Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value '<-' を使用して値を変換する diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf index ca8314d0795..55ce5cd5a48 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ko.xlf @@ -410,6 +410,11 @@ F# 구문 규칙에 맞는 줄 바꿈을 추가하여 지정된 너비에 시그 Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value '<-'를 사용하여 값 변경 diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf index 6323a9ff4c9..d37776029ff 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pl.xlf @@ -410,6 +410,11 @@ Pokaż uwagi w szybkich informacjach Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Użyj znaku „<-” w celu zmodyfikowania wartości 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 14cc2835582..41ecf3ae180 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.pt-BR.xlf @@ -410,6 +410,11 @@ Mostrar os comentários nas Informações Rápidas Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Usar '<-' para modificar o valor diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf index 656f7b84604..c9dc8382b3c 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.ru.xlf @@ -410,6 +410,11 @@ Show remarks in Quick Info Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Используйте "<-", чтобы изменить значение diff --git a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf index 1068d05a8c7..66a407e60c9 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.tr.xlf @@ -410,6 +410,11 @@ Açıklamaları Hızlı Bilgide göster Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value Değeri değiştirmek için '<-' kullan 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 c2afd4357f6..49fc20347c5 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hans.xlf @@ -410,6 +410,11 @@ Show remarks in Quick Info Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value 使用 "<-" 来更改值 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 7d74625d3e6..63d37f746f6 100644 --- a/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf +++ b/vsintegration/src/FSharp.Editor/xlf/FSharp.Editor.zh-Hant.xlf @@ -410,6 +410,11 @@ Show remarks in Quick Info Use F# '?' optional parameter + + Use F# '[<Struct>] ?' optional parameter + Use F# '[<Struct>] ?' optional parameter + + Use '<-' to mutate value 使用 '<-' 來變動值 diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs index 93f7e6c5793..9219dc541fe 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs @@ -20,11 +20,22 @@ open FSharp.Editor.Tests.Refactors.RefactorTestFramework let private caretAt (code: string) (marker: string) = code.IndexOf(marker, StringComparison.Ordinal) -let private refactored (code: string) (marker: string) = +let private fsharpTitle = "Use F# '?' optional parameter" +let private structTitle = "Use F# '[] ?' optional parameter" + +let private refactoredBy (pick: CodeAction seq -> CodeAction) (code: string) (marker: string) = use context = TestContext.CreateWithCode code - let document = - tryRefactor code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) + let action = + tryGetRefactoringActions code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) + |> pick + + for operation in action.GetOperationsAsync CancellationToken.None |> GetTaskResult do + let applyChanges = operation :?> ApplyChangesOperation + applyChanges.Apply(context.Solution.Workspace, CancellationToken.None) + context.Solution <- applyChanges.ChangedSolution + + let document = RoslynTestHelpers.GetLastDocument context.Solution let _, checkResults = document.GetFSharpParseAndCheckResultsAsync "test" @@ -37,6 +48,11 @@ let private refactored (code: string) (marker: string) = (document.GetTextAsync() |> GetTaskResult).ToString() +let private refactored (code: string) (marker: string) = refactoredBy Seq.head code marker + +let private refactoredWith (title: string) (code: string) (marker: string) = + refactoredBy (Seq.find (fun action -> String.Equals(action.Title, title, StringComparison.Ordinal))) code marker + let private actionsAt (code: string) (marker: string) = use context = TestContext.CreateWithCode code tryGetRefactoringActions code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) @@ -218,10 +234,8 @@ type C() = static member M(?x: int) = defaultArg x 0 """ -[] -let ``Title names the target form`` () = - let dotNetForm = - """ +let private dotNetForm = + """ module M open System.Runtime.InteropServices @@ -230,8 +244,65 @@ type C() = static member M([] x: int) = x """ +let private titlesOf (actions: CodeAction seq) = + actions |> Seq.map _.Title |> List.ofSeq + +[] +let ``Title names the target form`` () = Assert.Equal("Use [] for optional parameter", (actionsAt fsharpForm "?x" |> Seq.exactlyOne).Title) - Assert.Equal("Use F# '?' optional parameter", (actionsAt dotNetForm "x:" |> Seq.exactlyOne).Title) + Assert.Equal([ fsharpTitle; structTitle ], titlesOf (actionsAt dotNetForm "x:")) + +[] +[] step: int) = + value + step +""", + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next(value: int, [] ?step: int) = + let step = defaultValueArg step 1 + value + step +""")>] +[] step: int) = + step + 1 +""", + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M([] ?step: int) = + let step = defaultValueArg step Unchecked.defaultof<_> + step + 1 +""")>] +let ``Converting back to a struct optional parameter uses defaultValueArg`` (dotNetForm: string, structForm: string) = + Assert.Equal(structForm, refactoredWith structTitle dotNetForm "step:") + +[] +let ``Struct optional parameter is not offered before F# 10`` () = + use context = + new TestContext(RoslynTestHelpers.CreateSolution(dotNetForm, extraFSharpProjectOtherOptions = [| "--langversion:9.0" |])) + + let actions = + tryGetRefactoringActions dotNetForm (caretAt dotNetForm "x:") context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) + + Assert.Equal([ fsharpTitle ], titlesOf actions) [] [ Date: Mon, 14 Sep 2026 21:36:22 +0200 Subject: [PATCH 4/9] 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 | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 029a56a901d..44946fbf285 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -2,6 +2,7 @@ * Suggest active-pattern cases from unopened modules and offer valid Add Open and qualification edits. ([PR #20719](https://github.com/dotnet/fsharp/pull/20719)) * Add maintainer-requested F# Apex Visual Studio integration tests on DartLab-1ES CloudTest, using PR pipeline definitions pinned to an explicitly requested commit SHA. ([PR #20625](https://github.com/dotnet/fsharp/pull/20625)) +* Refactoring to switch a member's optional parameter between F# `?x: T` and the .NET-compatible `[] x: T`, moving the default between `defaultArg x c` in the body and the attribute; F# call sites keep working unchanged. From F# 10 the .NET form also converts back to `[] ?x: T` with `defaultValueArg x c`. ([PR #20547](https://github.com/dotnet/fsharp/pull/20547)) * 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)) * Added a **Tools > Options > F# Tools > Compiler** option, **"Use the .NET SDK F# compiler for builds"**, on by default. Visual Studio uses the .NET SDK F# compiler when the project supplies SDK paths, matching `dotnet build`. Otherwise, it uses the bundled .NET Framework compiler. Turn the option off to use the bundled compiler for all projects. Changes are saved on Apply or OK; Cancel discards unsaved changes. Command-line builds are unaffected. ([Issue #20484](https://github.com/dotnet/fsharp/issues/20484), [PR #20485](https://github.com/dotnet/fsharp/pull/20485)) From eb58154dddb66a923ab430dcfc119c911b8b77a3 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Tue, 29 Sep 2026 19:21:09 +0200 Subject: [PATCH 5/9] Check symbols, scope and callers before converting optional parameters Resolve defaultArg by symbol, not spelling. Do not offer the F# form for an [] obj without a default, whose omitted value is Missing.Value, nor the .NET form after an earlier ? parameter (FS1212). Keep a mutable shadowing binding, look for the InteropServices open only in modules that enclose the member, and rewrite ?x = option arguments when converting to a struct optional parameter. Co-Authored-By: Claude Opus 5.5 --- .../ConvertOptionalParameterDefaultValue.fs | 275 ++++++++++++++++-- ...nvertOptionalParameterDefaultValueTests.fs | 152 ++++++++++ 2 files changed, 404 insertions(+), 23 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs index 0b8d85cf6db..7cc7ce03db1 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs @@ -4,6 +4,8 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System open System.Composition +open System.Threading +open System.Threading.Tasks open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.CodeActions @@ -52,6 +54,36 @@ module private OptionalParameterDefaultValueConversion = let private isSame (expr: SynExpr) (other: SynExpr) = obj.ReferenceEquals(expr, other) + /// Whether the long identifier resolves to a value or union case of the FSharp.Core entity with the compiled full name. + let isFSharpCoreSymbol + (checkResults: FSharpCheckFileResults) + (sourceText: SourceText) + (entityFullName: string) + (longIdent: Ident list) + = + match List.tryLast longIdent with + | None -> false + | Some ident -> + let line = sourceText.Lines[Line.toZ ident.idRange.EndLine].ToString() + let names = longIdent |> List.map _.idText + + match checkResults.GetSymbolUseAtLocation(ident.idRange.EndLine, ident.idRange.EndColumn, line, names) with + | None -> false + | Some symbolUse -> + let declaringEntityName = + match symbolUse.Symbol with + | :? FSharpUnionCase as case -> case.DeclaringEntity.TryFullName + | :? FSharpMemberOrFunctionOrValue as value -> value.DeclaringEntity |> Option.bind _.TryFullName + | _ -> None + + match declaringEntityName with + | Some name -> + String.Equals(name, entityFullName, StringComparison.Ordinal) + && String.Equals(symbolUse.Symbol.Assembly.SimpleName, "FSharp.Core", StringComparison.Ordinal) + | None -> false + + let private operators = "Microsoft.FSharp.Core.Operators" + [] let private (|SingleIdent|_|) (expr: SynExpr) = match expr with @@ -172,20 +204,24 @@ module private OptionalParameterDefaultValueConversion = | _ -> None) /// `defaultArg x c` around a use of the parameter, with its constant and what contains it. - let private tryDefaultArgUse (parseTree: ParsedInput) (useRange: range) = + let private tryDefaultArgUse (isFSharpCore: string -> Ident list -> bool) (parseTree: ParsedInput) (useRange: range) = match tryUseNode parseTree useRange with | Some(node, SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = SingleIdent func; argExpr = arg) as inner) :: SyntaxNode.SynExpr(SynExpr.App( isInfix = false; funcExpr = funcExpr; argExpr = (SynExpr.Const _ as defaultValue)) as application) :: rest) when - isSame arg node && hasText "defaultArg" func && isSame funcExpr inner + isSame arg node + && hasText "defaultArg" func + && isSame funcExpr inner + && isFSharpCore operators [ func ] -> ValueSome(struct (application, defaultValue, rest)) | _ -> ValueNone - /// The whole line of `let x = defaultArg x c` when that line does nothing but rebind the parameter. + /// The whole line of `let x = defaultArg x c` when that line does nothing but rebind the parameter immutably. let private tryShadowingLine (sourceText: SourceText) (name: string) (application: SynExpr) (path: SyntaxVisitorPath) = match path with - | SyntaxNode.SynBinding(SynBinding(headPat = SynPat.Named(ident = SynIdent(ident, _)); expr = rhs; trivia = trivia)) :: SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse) :: _ when + | SyntaxNode.SynBinding(SynBinding( + isMutable = false; headPat = SynPat.Named(ident = SynIdent(ident, _)); expr = rhs; trivia = trivia)) :: SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse) :: _ when isSame rhs application && hasText name ident && not letOrUse.IsRecursive @@ -206,18 +242,32 @@ module private OptionalParameterDefaultValueConversion = ValueNone | _ -> ValueNone - /// `?x: T` with every use being `defaultArg x c` becomes `[] x: T`. + let rec private hasOptionalValBefore (position: pos) (pat: SynPat) = + match pat with + | SynPat.OptionalVal(range = m) -> Position.posLt m.Start position + | SynPat.Paren(pat = inner) + | SynPat.Typed(pat = inner) + | SynPat.Attrib(pat = inner) -> hasOptionalValBefore position inner + | SynPat.Tuple(elementPats = pats) + | SynPat.LongIdent(argPats = SynArgPats.Pats pats) -> pats |> List.exists (hasOptionalValBefore position) + | _ -> false + + /// `?x: T` with every use being `defaultArg x c` becomes `[] x: T`, + /// unless a `?` parameter before it would then precede a non-optional one (FS1212). let tryToDotNetChanges + (isFSharpCore: string -> Ident list -> bool) (sourceText: SourceText) (parseTree: ParsedInput) (parameter: Parameter) (optionalValRange: range) (uses: range list) = + let (SynBinding(headPat = headPat)) = parameter.MemberBinding + let found = (ValueSome [], uses) ||> List.fold (fun found useRange -> - match found, tryDefaultArgUse parseTree useRange with + match found, tryDefaultArgUse isFSharpCore parseTree useRange with | ValueSome found, ValueSome defaultArgUse -> ValueSome(defaultArgUse :: found) | _ -> ValueNone) @@ -239,7 +289,7 @@ module private OptionalParameterDefaultValueConversion = | _ -> ValueNone match found, attributeText with - | ValueSome found, ValueSome attributeText -> + | ValueSome found, ValueSome attributeText when not (hasOptionalValBefore optionalValRange.Start headPat) -> [ TextChange(TextSpan((spanOf sourceText optionalValRange).Start, 1), attributeText) @@ -254,17 +304,34 @@ module private OptionalParameterDefaultValueConversion = let private interopServices = [ "System"; "Runtime"; "InteropServices" ] - let private hasInteropServicesOpen (parseTree: ParsedInput) = + let rec private tryContainerRange (path: SyntaxVisitorPath) = + match path with + | SyntaxNode.SynModuleOrNamespace moduleOrNamespace :: _ -> ValueSome moduleOrNamespace.Range + | SyntaxNode.SynModule(SynModuleDecl.NestedModule(range = m)) :: _ -> ValueSome m + | _ :: rest -> tryContainerRange rest + | [] -> ValueNone + + /// Whether an `open System.Runtime.InteropServices` in a module or namespace enclosing the position precedes it. + let private hasInteropServicesOpenAt (position: pos) (parseTree: ParsedInput) = (false, parseTree) - ||> ParsedInput.fold (fun found _ node -> + ||> ParsedInput.fold (fun found path node -> found || match node with - | SyntaxNode.SynModule(SynModuleDecl.Open(target = SynOpenDeclTarget.ModuleOrNamespace(longId = SynLongIdent(id = ids)))) -> - (ids |> List.map _.idText) = interopServices + | SyntaxNode.SynModule(SynModuleDecl.Open( + target = SynOpenDeclTarget.ModuleOrNamespace(longId = SynLongIdent(id = ids)); range = m)) when + Position.posLt m.End position + && ids.Length = interopServices.Length + && List.forall2 hasText interopServices ids + -> + match tryContainerRange path with + | ValueSome container -> + Position.posGeq position container.Start + && Position.posGeq container.End position + | ValueNone -> false | _ -> false) let withInteropServicesOpen (parseTree: ParsedInput) (memberName: Ident) (text: SourceText) = - if hasInteropServicesOpen parseTree then + if hasInteropServicesOpenAt memberName.idRange.Start parseTree then text else let insertionContext = @@ -355,6 +422,85 @@ module private OptionalParameterDefaultValueConversion = ] |> List.sortBy _.Span.Start) + let private isOperator (name: string) (expr: SynExpr) = + match expr with + | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ operator ])) -> hasText name operator + | _ -> false + + let rec private functionOf (expr: SynExpr) = + match expr with + | SynExpr.App(isInfix = false; funcExpr = funcExpr) -> functionOf funcExpr + | _ -> expr + + let private isAtomic (expr: SynExpr) = + match expr with + | SynExpr.Ident _ + | SynExpr.LongIdent _ + | SynExpr.Paren _ + | SynExpr.Const _ -> true + | _ -> false + + /// The values passed as `?name = value` in the arguments of a call. + let private optionalArgumentValues (name: string) (arguments: SynExpr) = + let arguments = + match arguments with + | SynExpr.Paren(expr = SynExpr.Tuple(exprs = exprs)) -> exprs + | SynExpr.Paren(expr = single) -> [ single ] + | _ -> [] + + arguments + |> List.choose (function + | SynExpr.App( + isInfix = false + funcExpr = SynExpr.App( + isInfix = true + funcExpr = equals + argExpr = SynExpr.LongIdent(isOptional = true; longDotId = SynLongIdent(id = [ argumentName ]))) + argExpr = value) when isOperator "op_Equality" equals && hasText name argumentName -> Some value + | _ -> None) + + /// `Some v` becomes `ValueSome v`, `None` becomes `ValueNone`, and any other option goes through `ValueOption.ofOption`. + let private valueOptionChanges (isFSharpCore: string -> Ident list -> bool) (sourceText: SourceText) (value: SynExpr) = + let valueSpan = spanOf sourceText value.Range + + match functionOf value with + | SingleIdent head when + (hasText "Some" head || hasText "None" head) + && isFSharpCore "Microsoft.FSharp.Core.FSharpOption`1" [ head ] + -> + [ TextChange(TextSpan((spanOf sourceText head.idRange).Start, 0), "Value") ] + | _ when isAtomic value -> [ TextChange(TextSpan(valueSpan.Start, 0), "ValueOption.ofOption ") ] + | _ -> + [ + TextChange(TextSpan(valueSpan.Start, 0), "ValueOption.ofOption (") + TextChange(TextSpan(valueSpan.End, 0), ")") + ] + + /// Changes to the `?name = value` arguments of the call whose function ends at the use of the member, + /// for the parameter becoming a `[] ?name`. + let callSiteChanges + (isFSharpCore: string -> Ident list -> bool) + (sourceText: SourceText) + (parseTree: ParsedInput) + (name: string) + (useRange: range) + = + let arguments = + (useRange.Start, parseTree) + ||> ParsedInput.tryPickLast (fun _ node -> + match node with + | SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = func; argExpr = arguments)) when + Position.posEq func.Range.End useRange.End + -> + Some arguments + | _ -> None) + + match arguments with + | Some arguments -> + optionalArgumentValues name arguments + |> List.collect (valueOptionChanges isFSharpCore sourceText) + | None -> [] + [] type internal FSharpConvertOptionalParameterDefaultValueRefactoring [] () = inherit CodeRefactoringProvider() @@ -380,6 +526,18 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ false + /// `[]` alone passes `Missing.Value` for an omitted `obj`, not the `Unchecked.defaultof` that `?` would give. + static let isObject (symbol: FSharpSymbol) = + match symbol with + | :? FSharpMemberOrFunctionOrValue as parameter -> + let parameterType = parameter.FullType.StripAbbreviations() + + parameterType.HasTypeDefinition + && match parameterType.TypeDefinition.TryFullName with + | Some name -> String.Equals(name, "System.Object", StringComparison.Ordinal) + | None -> false + | _ -> false + override _.ComputeRefactoringsAsync context = cancellableTask { let document = context.Document @@ -419,6 +577,7 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ () | None -> () | OptionalParameterDefaultValueConversion.Form.DotNet(lists, defaultValue) -> - let! options = document.GetOptionsAsync cancellationToken + let isOmittedObject = + defaultValue.IsNone + && match tryGetSymbolUse checkResults sourceText parameter.Ident with + | Some parameterUse -> isObject parameterUse.Symbol + | None -> true - let! _, langVersion = - document.GetFsharpParsingOptionsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) + if not isOmittedObject then + let! options = document.GetOptionsAsync cancellationToken - let indentSize = - options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) + let! _, langVersion = + document.GetFsharpParsingOptionsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) - let register (asStruct: bool) (title: string) = - match + let indentSize = + options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) + + let tryChanges (asStruct: bool) = OptionalParameterDefaultValueConversion.tryToFSharpChanges sourceText indentSize @@ -460,18 +625,82 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ + let title = SR.UseFSharpOptionalParameter() + let changedDocument = cancellableTask { return document.WithText(sourceText.WithChanges changes) } context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) | ValueNone -> () - register false (SR.UseFSharpOptionalParameter()) + match tryChanges true with + | ValueSome definitionChanges when + LanguageVersion(langVersion).SupportsFeature LanguageFeature.SupportValueOptionsAsOptionalParameters + -> + let title = SR.UseFSharpStructOptionalParameter() + + // `?x = Some 1` passes an option to `[] x`; a struct `?x` takes a value option instead. + let changedSolution = + cancellableTask { + let! cancellationToken = CancellableTask.getCancellationToken () + let! memberUses = SymbolHelpers.getSymbolUses memberUse document checkResults + + let usesByDocument = + memberUses + |> Seq.groupBy (fun (useDocument: Document, _) -> useDocument.Id) + |> Seq.toArray + + let mutable solution = document.Project.Solution + + for documentId, documentUses in usesByDocument do + let useDocument = solution.GetDocument documentId + let! text = useDocument.GetTextAsync cancellationToken + + let! useParseResults, useCheckResults = + useDocument.GetFSharpParseAndCheckResultsAsync( + nameof FSharpConvertOptionalParameterDefaultValueRefactoring + ) + + let changes = + [ + if documentId = document.Id then + yield! definitionChanges + + for _, useRange in documentUses do + yield! + OptionalParameterDefaultValueConversion.callSiteChanges + (OptionalParameterDefaultValueConversion.isFSharpCoreSymbol + useCheckResults + text) + text + useParseResults.ParseTree + parameter.Ident.idText + useRange + ] + |> List.distinctBy _.Span + |> List.sortBy _.Span.Start + + solution <- solution.WithDocumentText(documentId, text.WithChanges changes) + + if not (usesByDocument |> Array.exists (fun (documentId, _) -> documentId = document.Id)) then + solution <- solution.WithDocumentText(document.Id, sourceText.WithChanges definitionChanges) + + return solution + } + + let action = + CodeAction.Create( + title, + Func>(fun cancellationToken -> + CancellableTask.start cancellationToken changedSolution), + title + ) - if LanguageVersion(langVersion).SupportsFeature LanguageFeature.SupportValueOptionsAsOptionalParameters then - register true (SR.UseFSharpStructOptionalParameter()) + context.RegisterRefactoring action + | _ -> () | _ -> () } |> CancellableTask.startAsTask context.CancellationToken diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs index 9219dc541fe..46d511b42d5 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs @@ -371,8 +371,160 @@ type C() = static member M(x: int) = x """, "x:")>] +[] +[] x: obj) = x +""", + "x:")>] +[] let ``No action`` (code: string, marker: string) = Assert.Empty(actionsAt code marker) +[] +let ``Mutable shadowing keeps its binding`` () = + let before = + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M(?x: int) = + let mutable x = defaultArg x 0 + x <- x + 1 + x +""" + + let after = + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M([] x: int) = + let mutable x = x + x <- x + 1 + x +""" + + Assert.Equal(after, refactored before "?x") + +[] +let ``Open in another module does not count as in scope`` () = + let before = + """ +module M + +module Other = + open System.Runtime.InteropServices + + let o = 1 + +type C() = + static member M(?x: int) = defaultArg x 0 +""" + + let after = + """ +module M + +open System.Runtime.InteropServices + +module Other = + open System.Runtime.InteropServices + + let o = 1 + +type C() = + static member M([] x: int) = x +""" + + Assert.Equal(after, refactored before "?x") + +[] +let ``Open in an enclosing module counts as in scope`` () = + let before = + """ +module M + +module Inner = + open System.Runtime.InteropServices + + type C() = + static member M(?x: int) = defaultArg x 0 +""" + + let after = + """ +module M + +module Inner = + open System.Runtime.InteropServices + + type C() = + static member M([] x: int) = x +""" + + Assert.Equal(after, refactored before "?x") + +[] +let ``Converting to a struct optional parameter converts optional arguments to value options`` () = + let before = + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next([] step: int) = + step + 1 + +let a = Counter.Next() +let b = Counter.Next(?step = Some 2) +let c = Counter.Next(?step = None) +let d (s: int option) = Counter.Next(?step = s) +let e (s: int list) = Counter.Next(?step = List.tryHead s) +""" + + let after = + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next([] ?step: int) = + let step = defaultValueArg step 1 + step + 1 + +let a = Counter.Next() +let b = Counter.Next(?step = ValueSome 2) +let c = Counter.Next(?step = ValueNone) +let d (s: int option) = Counter.Next(?step = ValueOption.ofOption s) +let e (s: int list) = Counter.Next(?step = ValueOption.ofOption (List.tryHead s)) +""" + + Assert.Equal(after, refactoredWith structTitle before "step:") + [] let ``No action when the file has a signature`` () = let signature = From 89665720f6405cf4b6e0835da53f4fef1e2160cc Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Sun, 11 Oct 2026 09:35:38 +0200 Subject: [PATCH 6/9] Read optional parameter attributes from symbols and qualify generated names Converting `[] x: T` to `?x: T`: - Read the attributes from the parameter's symbol: they must be the `OptionalAttribute` and at most the `DefaultParameterValueAttribute` of `System.Runtime.InteropServices`, and the parameter may have no other attribute, in whichever attribute list it is written. - Require the default constant to be of the parameter's type, as the compiler does before it passes one, and take that type from the symbol in both directions. - Do not offer it for a byref or byref-like parameter type, nor when a parameter that is not a `?` one follows (FS1212). - Write `defaultArg` and `defaultValueArg` with their full names where the short name resolves to something else, and recognise a qualified `defaultArg` on the way back. - Keep a comment between `=` and the body, and the backticks of an escaped parameter name. Converting to `[] ?x: T`: - Offer it only when every F# project that depends on the member's project takes value options for optional parameters. - Replace the whole `Some` or `None` identifier at call sites, with the full name of the value option case where the short one is shadowed. Converting `?x: T` to the .NET form: - Write the attributes with their namespace instead of inserting `open System.Runtime.InteropServices`, unless it is open at the member. Co-Authored-By: Claude Opus 5.5 --- .../ConvertOptionalParameterDefaultValue.fs | 406 +++++++++----- ...nvertOptionalParameterDefaultValueTests.fs | 510 ++++++++++++++++-- 2 files changed, 747 insertions(+), 169 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs index 7cc7ce03db1..47dbf8741cd 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs @@ -28,7 +28,7 @@ module private OptionalParameterDefaultValueConversion = type Form = /// `?x: T` | FSharp of optionalValRange: range - /// `[] x: T`, with the attribute lists holding only those attributes. + /// `[] x: T`, the parameter having no other attribute. | DotNet of lists: SynAttributeList list * defaultValue: SynExpr voption [] @@ -84,6 +84,44 @@ module private OptionalParameterDefaultValueConversion = let private operators = "Microsoft.FSharp.Core.Operators" + /// How to write a value or union case of FSharp.Core at the position: by its name where that resolves to it, + /// and with its namespace and its module or type where the name means something else. + let private coreName + (checkResults: FSharpCheckFileResults) + (compiledContainer: string) + (container: string) + (name: string) + (position: pos) + = + let isNamed (symbolName: string) = + String.Equals(symbolName, name, StringComparison.Ordinal) + + let isInScope (symbol: FSharpSymbol) = + checkResults.IsRelativeNameResolvableFromSymbol(position, [], symbol) + + let entity = + checkResults.ProjectContext.GetReferencedAssemblies() + |> List.tryPick (fun assembly -> + if String.Equals(assembly.SimpleName, "FSharp.Core", StringComparison.Ordinal) then + assembly.Contents.FindEntityByPath [ "Microsoft"; "FSharp"; "Core"; compiledContainer ] + else + None) + + let resolves = + match entity with + | Some entity when entity.IsFSharpUnion -> + entity.UnionCases + |> Seq.exists (fun case -> isNamed case.Name && isInScope case) + | Some entity -> + entity.MembersFunctionsAndValues + |> Seq.exists (fun value -> isNamed value.DisplayName && isInScope value) + | None -> false + + if resolves then + name + else + $"Microsoft.FSharp.Core.{container}.{name}" + [] let private (|SingleIdent|_|) (expr: SynExpr) = match expr with @@ -91,6 +129,17 @@ module private OptionalParameterDefaultValueConversion = | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ ident ])) -> ValueSome ident | _ -> ValueNone + /// The identifiers of a name, qualified or not, and the last of them. + [] + let private (|Name|_|) (expr: SynExpr) = + match expr with + | SynExpr.Ident ident -> ValueSome(struct ([ ident ], ident)) + | SynExpr.LongIdent(longDotId = SynLongIdent(id = ids)) -> + match List.tryLast ids with + | Some last -> ValueSome(struct (ids, last)) + | None -> ValueNone + | _ -> ValueNone + let private isAttribute (names: string list) (attribute: SynAttribute) = attribute.Target.IsNone && match List.tryLast attribute.TypeName.LongIdent with @@ -123,20 +172,16 @@ module private OptionalParameterDefaultValueConversion = let isOptionalAttribute attribute = isOptional attribute || isDefaultParameterValue attribute - let lists = - attributes - |> List.filter (fun list -> List.exists isOptionalAttribute list.Attributes) - - let listed = lists |> List.collect _.Attributes + let listed = attributes |> List.collect _.Attributes if not (List.exists isOptional listed && List.forall isOptionalAttribute listed) then ValueNone else match List.tryFind isDefaultParameterValue listed with - | None -> ValueSome(Form.DotNet(lists, ValueNone)) + | None -> ValueSome(Form.DotNet(attributes, ValueNone)) | Some attribute -> match attribute.ArgExpr with - | SynExpr.Paren(expr = SynExpr.Const _ as constant) -> ValueSome(Form.DotNet(lists, ValueSome constant)) + | SynExpr.Paren(expr = SynExpr.Const _ as constant) -> ValueSome(Form.DotNet(attributes, ValueSome constant)) | _ -> ValueNone let rec private tryMember (path: SyntaxVisitorPath) = @@ -173,22 +218,22 @@ module private OptionalParameterDefaultValueConversion = | Search.Found parameter -> parameter | Search.Searching -> ValueNone - /// Whether the constant can be the `DefaultParameterValue` of a parameter of the named type. - let private isConstantOfType (typeText: string) (constant: SynConst) = - match typeText, constant with - | ("int" | "int32"), SynConst.Int32 _ - | "int64", SynConst.Int64 _ - | "int16", SynConst.Int16 _ - | "sbyte", SynConst.SByte _ - | "byte", SynConst.Byte _ - | "uint16", SynConst.UInt16 _ - | "uint32", SynConst.UInt32 _ - | "uint64", SynConst.UInt64 _ - | ("float" | "double"), SynConst.Double _ - | ("float32" | "single"), SynConst.Single _ - | "bool", SynConst.Bool _ - | "char", SynConst.Char _ - | "string", SynConst.String _ -> true + /// Whether the constant can be the `DefaultParameterValue` of a parameter of the type with the full name. + let isConstantOfType (typeFullName: string option) (constant: SynConst) = + match typeFullName, constant with + | Some "System.Int32", SynConst.Int32 _ + | Some "System.Int64", SynConst.Int64 _ + | Some "System.Int16", SynConst.Int16 _ + | Some "System.SByte", SynConst.SByte _ + | Some "System.Byte", SynConst.Byte _ + | Some "System.UInt16", SynConst.UInt16 _ + | Some "System.UInt32", SynConst.UInt32 _ + | Some "System.UInt64", SynConst.UInt64 _ + | Some "System.Double", SynConst.Double _ + | Some "System.Single", SynConst.Single _ + | Some "System.Boolean", SynConst.Bool _ + | Some "System.Char", SynConst.Char _ + | Some "System.String", SynConst.String _ -> true | _ -> false let private tryUseNode (parseTree: ParsedInput) (useRange: range) = @@ -207,12 +252,12 @@ module private OptionalParameterDefaultValueConversion = let private tryDefaultArgUse (isFSharpCore: string -> Ident list -> bool) (parseTree: ParsedInput) (useRange: range) = match tryUseNode parseTree useRange with | Some(node, - SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = SingleIdent func; argExpr = arg) as inner) :: SyntaxNode.SynExpr(SynExpr.App( + SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = Name(func, name); argExpr = arg) as inner) :: SyntaxNode.SynExpr(SynExpr.App( isInfix = false; funcExpr = funcExpr; argExpr = (SynExpr.Const _ as defaultValue)) as application) :: rest) when isSame arg node - && hasText "defaultArg" func + && hasText "defaultArg" name && isSame funcExpr inner - && isFSharpCore operators [ func ] + && isFSharpCore operators func -> ValueSome(struct (application, defaultValue, rest)) | _ -> ValueNone @@ -252,13 +297,43 @@ module private OptionalParameterDefaultValueConversion = | SynPat.LongIdent(argPats = SynArgPats.Pats pats) -> pats |> List.exists (hasOptionalValBefore position) | _ -> false + let private interopServices = [ "System"; "Runtime"; "InteropServices" ] + + let rec private tryContainerRange (path: SyntaxVisitorPath) = + match path with + | SyntaxNode.SynModuleOrNamespace moduleOrNamespace :: _ -> ValueSome moduleOrNamespace.Range + | SyntaxNode.SynModule(SynModuleDecl.NestedModule(range = m)) :: _ -> ValueSome m + | _ :: rest -> tryContainerRange rest + | [] -> ValueNone + + /// Whether an `open System.Runtime.InteropServices` in a module or namespace enclosing the position precedes it. + let private hasInteropServicesOpenAt (position: pos) (parseTree: ParsedInput) = + (false, parseTree) + ||> ParsedInput.fold (fun found path node -> + found + || match node with + | SyntaxNode.SynModule(SynModuleDecl.Open( + target = SynOpenDeclTarget.ModuleOrNamespace(longId = SynLongIdent(id = ids)); range = m)) when + Position.posLt m.End position + && ids.Length = interopServices.Length + && List.forall2 hasText interopServices ids + -> + match tryContainerRange path with + | ValueSome container -> + Position.posGeq position container.Start + && Position.posGeq container.End position + | ValueNone -> false + | _ -> false) + /// `?x: T` with every use being `defaultArg x c` becomes `[] x: T`, /// unless a `?` parameter before it would then precede a non-optional one (FS1212). + /// The attributes carry their namespace where no `open` of it is in scope. let tryToDotNetChanges (isFSharpCore: string -> Ident list -> bool) (sourceText: SourceText) (parseTree: ParsedInput) (parameter: Parameter) + (parameterTypeName: string option) (optionalValRange: range) (uses: range list) = @@ -274,76 +349,44 @@ module private OptionalParameterDefaultValueConversion = let textOf (expr: SynExpr) = sourceText.ToString(spanOf sourceText expr.Range) - let typeText = - sourceText.ToString(spanOf sourceText parameter.TypeName.Range).Trim() - - let attributeText = + let attributes = match found with - | ValueSome [] -> ValueSome "[] " + | ValueSome [] -> ValueSome [ "Optional" ] | ValueSome(struct (_, (SynExpr.Const(constant, _) as defaultValue), _) :: others) when - isConstantOfType typeText constant + isConstantOfType parameterTypeName constant && others |> List.forall (fun struct (_, other, _) -> String.Equals(textOf other, textOf defaultValue, StringComparison.Ordinal)) -> - ValueSome $"[] " + ValueSome [ "Optional"; $"DefaultParameterValue({textOf defaultValue})" ] | _ -> ValueNone - match found, attributeText with - | ValueSome found, ValueSome attributeText when not (hasOptionalValBefore optionalValRange.Start headPat) -> + match found, attributes with + | ValueSome found, ValueSome attributes when not (hasOptionalValBefore optionalValRange.Start headPat) -> + let qualifier = + if hasInteropServicesOpenAt parameter.MemberName.idRange.Start parseTree then + "" + else + String.Join(".", interopServices) + "." + + let attributeText = + attributes + |> Seq.map (fun attribute -> qualifier + attribute) + |> String.concat "; " + + let name = sourceText.ToString(spanOf sourceText parameter.Ident.idRange) + [ - TextChange(TextSpan((spanOf sourceText optionalValRange).Start, 1), attributeText) + TextChange(TextSpan((spanOf sourceText optionalValRange).Start, 1), $"[<{attributeText}>] ") for struct (application, _, path) in found do match tryShadowingLine sourceText parameter.Ident.idText application path with | ValueSome line -> TextChange(line, "") - | ValueNone -> TextChange(spanOf sourceText application.Range, parameter.Ident.idText) + | ValueNone -> TextChange(spanOf sourceText application.Range, name) ] |> List.sortBy _.Span.Start |> ValueSome | _ -> ValueNone - let private interopServices = [ "System"; "Runtime"; "InteropServices" ] - - let rec private tryContainerRange (path: SyntaxVisitorPath) = - match path with - | SyntaxNode.SynModuleOrNamespace moduleOrNamespace :: _ -> ValueSome moduleOrNamespace.Range - | SyntaxNode.SynModule(SynModuleDecl.NestedModule(range = m)) :: _ -> ValueSome m - | _ :: rest -> tryContainerRange rest - | [] -> ValueNone - - /// Whether an `open System.Runtime.InteropServices` in a module or namespace enclosing the position precedes it. - let private hasInteropServicesOpenAt (position: pos) (parseTree: ParsedInput) = - (false, parseTree) - ||> ParsedInput.fold (fun found path node -> - found - || match node with - | SyntaxNode.SynModule(SynModuleDecl.Open( - target = SynOpenDeclTarget.ModuleOrNamespace(longId = SynLongIdent(id = ids)); range = m)) when - Position.posLt m.End position - && ids.Length = interopServices.Length - && List.forall2 hasText interopServices ids - -> - match tryContainerRange path with - | ValueSome container -> - Position.posGeq position container.Start - && Position.posGeq container.End position - | ValueNone -> false - | _ -> false) - - let withInteropServicesOpen (parseTree: ParsedInput) (memberName: Ident) (text: SourceText) = - if hasInteropServicesOpenAt memberName.idRange.Start parseTree then - text - else - let insertionContext = - FSharp.Compiler.EditorServices.ParsedInput.FindNearestPointToInsertOpenDeclaration - memberName.idRange.StartLine - parseTree - (List.toArray interopServices) - FSharp.Compiler.EditorServices.OpenStatementInsertionPoint.TopLevel - - OpenDeclarationHelper.insertOpenDeclaration text insertionContext (String.Join(".", interopServices)) - |> fst - let private leadingSpaces (line: TextLine) = let text = line.ToString() text.Length - text.TrimStart(' ').Length @@ -359,9 +402,33 @@ module private OptionalParameterDefaultValueConversion = let after = sourceText.ToString(TextSpan.FromBounds(listSpan.End, line.End)) TextSpan(listSpan.Start, listSpan.Length + after.Length - after.TrimStart().Length) + let rec private startOfWhiteSpaceBefore (sourceText: SourceText) (position: int) = + if position > 0 && Char.IsWhiteSpace sourceText[position - 1] then + startOfWhiteSpaceBefore sourceText (position - 1) + else + position + + let rec private isOptionalVal (pat: SynPat) = + match pat with + | SynPat.OptionalVal _ -> true + | SynPat.Paren(pat = inner) + | SynPat.Typed(pat = inner) + | SynPat.Attrib(pat = inner) -> isOptionalVal inner + | _ -> false + + let rec private hasRequiredAfter (position: pos) (pat: SynPat) = + match pat with + | SynPat.Paren(pat = inner) -> hasRequiredAfter position inner + | SynPat.Tuple(elementPats = pats) -> + pats + |> List.exists (fun pat -> Position.posGt pat.Range.Start position && not (isOptionalVal pat)) + | SynPat.LongIdent(argPats = SynArgPats.Pats pats) -> pats |> List.exists (hasRequiredAfter position) + | _ -> false + /// `[] x: T` becomes `?x: T` with `let x = defaultArg x c` starting the body, - /// or `[] ?x: T` with `let x = defaultValueArg x c`. + /// or `[] ?x: T` with `let x = defaultValueArg x c`, unless a non-optional parameter follows it (FS1212). let tryToFSharpChanges + (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (indentSize: int) (parameter: Parameter) @@ -369,7 +436,7 @@ module private OptionalParameterDefaultValueConversion = (defaultValue: SynExpr voption) (asStruct: bool) = - let name = parameter.Ident.idText + let name = sourceText.ToString(spanOf sourceText parameter.Ident.idRange) let defaultText = match defaultValue with @@ -382,9 +449,7 @@ module private OptionalParameterDefaultValueConversion = else struct ("?", "defaultArg") - let declaration = $"let {name} = {defaultFunction} {name} {defaultText}" - - let (SynBinding(expr = body; returnInfo = returnInfo; trivia = trivia)) = + let (SynBinding(headPat = headPat; expr = body; returnInfo = returnInfo; trivia = trivia)) = parameter.MemberBinding let body = @@ -392,6 +457,10 @@ module private OptionalParameterDefaultValueConversion = | Some _, SynExpr.Typed(expr = inner) -> inner | _ -> body + let defaultFunction = + coreName checkResults "Operators" "Operators" defaultFunction body.Range.Start + + let declaration = $"let {name} = {defaultFunction} {name} {defaultText}" let memberLine = sourceText.Lines[Line.toZ trivia.LeadingKeyword.Range.StartLine] let lineBreak = lineBreakOf sourceText memberLine @@ -400,19 +469,21 @@ module private OptionalParameterDefaultValueConversion = | Some equals when body.Range.StartLine > equals.EndLine -> let line = sourceText.Lines[Line.toZ body.Range.StartLine] ValueSome(TextChange(TextSpan(line.Start, 0), $"{String(' ', body.Range.StartColumn)}{declaration}{lineBreak}")) - | Some equals when body.Range.StartLine = body.Range.EndLine -> + | Some _ when body.Range.StartLine = body.Range.EndLine -> let indent = String(' ', leadingSpaces memberLine + indentSize) + let bodyStart = (spanOf sourceText body.Range).Start + // A comment between `=` and the body stays on the member line. ValueSome( TextChange( - TextSpan.FromBounds((spanOf sourceText equals).End, (spanOf sourceText body.Range).Start), + TextSpan.FromBounds(startOfWhiteSpaceBefore sourceText bodyStart, bodyStart), $"{lineBreak}{indent}{declaration}{lineBreak}{indent}" ) ) | _ -> ValueNone - bodyChange - |> ValueOption.map (fun bodyChange -> + match bodyChange with + | ValueSome bodyChange when not (hasRequiredAfter parameter.Ident.idRange.End headPat) -> [ for list in lists do TextChange(listRemoval sourceText list, "") @@ -420,7 +491,9 @@ module private OptionalParameterDefaultValueConversion = TextChange(TextSpan((spanOf sourceText parameter.Ident.idRange).Start, 0), prefix) bodyChange ] - |> List.sortBy _.Span.Start) + |> List.sortBy _.Span.Start + |> ValueSome + | _ -> ValueNone let private isOperator (name: string) (expr: SynExpr) = match expr with @@ -460,15 +533,22 @@ module private OptionalParameterDefaultValueConversion = | _ -> None) /// `Some v` becomes `ValueSome v`, `None` becomes `ValueNone`, and any other option goes through `ValueOption.ofOption`. - let private valueOptionChanges (isFSharpCore: string -> Ident list -> bool) (sourceText: SourceText) (value: SynExpr) = + let private valueOptionChanges (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (value: SynExpr) = let valueSpan = spanOf sourceText value.Range match functionOf value with | SingleIdent head when (hasText "Some" head || hasText "None" head) - && isFSharpCore "Microsoft.FSharp.Core.FSharpOption`1" [ head ] + && isFSharpCoreSymbol checkResults sourceText "Microsoft.FSharp.Core.FSharpOption`1" [ head ] -> - [ TextChange(TextSpan((spanOf sourceText head.idRange).Start, 0), "Value") ] + let case = if hasText "Some" head then "ValueSome" else "ValueNone" + + [ + TextChange( + spanOf sourceText head.idRange, + coreName checkResults "FSharpValueOption`1" "ValueOption" case head.idRange.Start + ) + ] | _ when isAtomic value -> [ TextChange(TextSpan(valueSpan.Start, 0), "ValueOption.ofOption ") ] | _ -> [ @@ -479,7 +559,7 @@ module private OptionalParameterDefaultValueConversion = /// Changes to the `?name = value` arguments of the call whose function ends at the use of the member, /// for the parameter becoming a `[] ?name`. let callSiteChanges - (isFSharpCore: string -> Ident list -> bool) + (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (parseTree: ParsedInput) (name: string) @@ -498,7 +578,7 @@ module private OptionalParameterDefaultValueConversion = match arguments with | Some arguments -> optionalArgumentValues name arguments - |> List.collect (valueOptionChanges isFSharpCore sourceText) + |> List.collect (valueOptionChanges checkResults sourceText) | None -> [] [] @@ -526,17 +606,102 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ false - /// `[]` alone passes `Missing.Value` for an omitted `obj`, not the `Unchecked.defaultof` that `?` would give. - static let isObject (symbol: FSharpSymbol) = + static let hasFullName (fullName: string) (entity: FSharpEntity) = + match entity.TryFullName with + | Some name -> String.Equals(name, fullName, StringComparison.Ordinal) + | None -> false + + static let isAttribute (fullName: string) (attribute: FSharpAttribute) = + hasFullName fullName attribute.AttributeType + + static let isInteropOptional = + isAttribute "System.Runtime.InteropServices.OptionalAttribute" + + static let isInteropDefaultParameterValue = + isAttribute "System.Runtime.InteropServices.DefaultParameterValueAttribute" + + static let isByRefLike (definition: FSharpEntity) = + definition.IsByRef + || definition.Attributes + |> Seq.exists (isAttribute "System.Runtime.CompilerServices.IsByRefLikeAttribute") + + static let tryTypeDefinition (fsharpType: FSharpType) = + let stripped = fsharpType.StripAbbreviations() + + if stripped.HasTypeDefinition then + ValueSome stripped.TypeDefinition + else + ValueNone + + /// The full name of `T` for the symbol of a `?x: T` parameter. + static let tryOptionalValueTypeName (symbol: FSharpSymbol) = match symbol with | :? FSharpMemberOrFunctionOrValue as parameter -> - let parameterType = parameter.FullType.StripAbbreviations() + let arguments = parameter.FullType.StripAbbreviations().GenericArguments - parameterType.HasTypeDefinition - && match parameterType.TypeDefinition.TryFullName with - | Some name -> String.Equals(name, "System.Object", StringComparison.Ordinal) - | None -> false - | _ -> false + match arguments |> Seq.tryHeadV |> ValueOption.bind tryTypeDefinition with + | ValueSome definition -> definition.TryFullName + | ValueNone -> None + | _ -> None + + static let tryParameterSymbol (memberSymbol: FSharpSymbol) (name: string) = + match memberSymbol with + | :? FSharpMemberOrFunctionOrValue as mfv -> + mfv.CurriedParameterGroups + |> Seq.concat + |> Seq.tryFindV (fun candidate -> + match candidate.Name with + | Some candidateName -> String.Equals(candidateName, name, StringComparison.Ordinal) + | None -> false) + | _ -> ValueNone + + /// Whether `?x` can replace the attributes of the parameter: they are the `OptionalAttribute` and at most the + /// `DefaultParameterValueAttribute` of `System.Runtime.InteropServices`, the default value is one the compiler + /// passes for a parameter of this type, and an option of the type can exist. + static let isDotNetOptional (parameter: FSharpParameter) (defaultValue: SynExpr voption) = + let attributes = parameter.Attributes + + attributes |> Seq.exists isInteropOptional + && attributes + |> Seq.forall (fun attribute -> isInteropOptional attribute || isInteropDefaultParameterValue attribute) + && match tryTypeDefinition parameter.Type, defaultValue with + | ValueSome definition, _ when isByRefLike definition -> false + // `[]` alone passes `Missing.Value` for an omitted `obj`, not the `Unchecked.defaultof` that `?` would give. + | ValueSome definition, ValueNone -> not (hasFullName "System.Object" definition) + | ValueSome definition, ValueSome(SynExpr.Const(constant, _)) -> + OptionalParameterDefaultValueConversion.isConstantOfType definition.TryFullName constant + | ValueNone, ValueNone -> true + | _ -> false + + /// Whether the project and the F# projects that depend on it take a value option for a `?` parameter, + /// which a caller compiled for an earlier language version cannot pass. + static let supportsStructOptionalParameters (project: Project) = + cancellableTask { + let solution = project.Solution + + let projects = + [ + project + + for id in solution.GetProjectDependencyGraph().GetProjectsThatTransitivelyDependOnThisProject project.Id do + let dependent = solution.GetProject id + + if dependent.IsFSharp then + dependent + ] + + let mutable isSupported = true + + for candidate in projects do + if isSupported then + let! _, _, parsingOptions, _ = candidate.GetFSharpCompilationOptionsAsync() + + isSupported <- + LanguageVersion(parsingOptions.LangVersionText).SupportsFeature + LanguageFeature.SupportValueOptionsAsOptionalParameters + + return isSupported + } override _.ComputeRefactoringsAsync context = cancellableTask { @@ -581,6 +746,7 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ OptionalParameterDefaultValueConversion.withInteropServicesOpen - parseResults.ParseTree - parameter.MemberName - - return document.WithText changed - } + cancellableTask { return document.WithText(sourceText.WithChanges changes) } context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) | ValueNone -> () | None -> () | OptionalParameterDefaultValueConversion.Form.DotNet(lists, defaultValue) -> - let isOmittedObject = - defaultValue.IsNone - && match tryGetSymbolUse checkResults sourceText parameter.Ident with - | Some parameterUse -> isObject parameterUse.Symbol - | None -> true + let isConvertible = + match tryParameterSymbol memberUse.Symbol parameter.Ident.idText with + | ValueSome parameterSymbol -> isDotNetOptional parameterSymbol defaultValue + | ValueNone -> false - if not isOmittedObject then + if isConvertible then let! options = document.GetOptionsAsync cancellationToken - - let! _, langVersion = - document.GetFsharpParsingOptionsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) + let! isStructSupported = supportsStructOptionalParameters document.Project let indentSize = options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) let tryChanges (asStruct: bool) = OptionalParameterDefaultValueConversion.tryToFSharpChanges + checkResults sourceText indentSize parameter @@ -637,9 +793,7 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ () match tryChanges true with - | ValueSome definitionChanges when - LanguageVersion(langVersion).SupportsFeature LanguageFeature.SupportValueOptionsAsOptionalParameters - -> + | ValueSome definitionChanges when isStructSupported -> let title = SR.UseFSharpStructOptionalParameter() // `?x = Some 1` passes an option to `[] x`; a struct `?x` takes a value option instead. @@ -672,9 +826,7 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ GetTaskResult).ToString() + +let private assertNoErrors (document: Document) = + let _, checkResults = + document.GetFSharpParseAndCheckResultsAsync "test" + |> CancellableTask.runSynchronouslyWithoutCancellation + + Assert.Empty( + checkResults.Diagnostics + |> Array.filter (fun diagnostic -> diagnostic.Severity = FSharpDiagnosticSeverity.Error) + ) + let private refactoredBy (pick: CodeAction seq -> CodeAction) (code: string) (marker: string) = use context = TestContext.CreateWithCode code @@ -36,31 +54,45 @@ let private refactoredBy (pick: CodeAction seq -> CodeAction) (code: string) (ma context.Solution <- applyChanges.ChangedSolution let document = RoslynTestHelpers.GetLastDocument context.Solution - - let _, checkResults = - document.GetFSharpParseAndCheckResultsAsync "test" - |> CancellableTask.runSynchronouslyWithoutCancellation - - Assert.Empty( - checkResults.Diagnostics - |> Array.filter (fun diagnostic -> diagnostic.Severity = FSharpDiagnosticSeverity.Error) - ) - - (document.GetTextAsync() |> GetTaskResult).ToString() + assertNoErrors document + textOf document let private refactored (code: string) (marker: string) = refactoredBy Seq.head code marker let private refactoredWith (title: string) (code: string) (marker: string) = - refactoredBy (Seq.find (fun action -> String.Equals(action.Title, title, StringComparison.Ordinal))) code marker + refactoredBy (Seq.find (hasTitle title)) code marker let private actionsAt (code: string) (marker: string) = use context = TestContext.CreateWithCode code tryGetRefactoringActions code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) -[] -let ``Optional parameter with a default value becomes a .NET optional parameter and the open is added`` () = - let before = - """ +let private actionsIn (document: Document) (marker: string) = + let actions = ResizeArray() + + let context = + CodeRefactoringContext( + document, + TextSpan(caretAt (textOf document) marker, 1), + (fun action -> actions.Add action), + CancellationToken.None + ) + + (new FSharpConvertOptionalParameterDefaultValueRefactoring()).ComputeRefactoringsAsync(context).GetAwaiter().GetResult() + + actions + +let private documentOf (solution: Solution) (project: SyntheticProject) (fileId: string) = + solution.GetDocumentIdsWithFilePath(project.GetFilePath fileId) + |> Seq.exactlyOne + |> solution.GetDocument + +let private fileOf (fileId: string) (source: string) = + { sourceFile fileId [] with + Source = source + } + +[] +[] greeting: string) = + member _.Greet(name: string, [] greeting: string) = $"{greeting}, {name}" let a = Greeter().Greet("Ada") let b = Greeter().Greet("Ada", greeting = "Hi") -""" +""")>] +[] greeting: string) = greeting + +let a = Greeter().Greet() +""")>] +let ``Attributes are fully qualified where System.Runtime.InteropServices is not open`` (before: string, after: string) = Assert.Equal(after, refactored before "?greeting") [] @@ -131,6 +181,52 @@ type Counter() = static member Next(value: int, [] step: float) = float value + step """)>] +[] step: Step) = + step + 1 +""")>] +[] step: int) = + step + 1 +""")>] let ``Shadowing default converts both ways`` (fsharpForm: string, dotNetForm: string) = Assert.Equal(dotNetForm, refactored fsharpForm "?step") Assert.Equal(fsharpForm, refactored dotNetForm "step:") @@ -223,6 +319,25 @@ type C() = let step = defaultArg step Unchecked.defaultof<_> step + 1 """)>] +[] step: int) = (* keep this *) step + 1 +""", + "step:", + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M(?step: int) = (* keep this *) + let step = defaultArg step Unchecked.defaultof<_> + step + 1 +""")>] let ``Optional without a default value`` (before: string, marker: string, after: string) = Assert.Equal(after, refactored before marker) @@ -291,6 +406,29 @@ type C() = let step = defaultValueArg step Unchecked.defaultof<_> step + 1 """)>] +[] step: int) = + step + 1 +""", + """ +module M + +open System.Runtime.InteropServices + +let defaultValueArg (_: int voption) (fallback: int) = fallback + 1 + +type C() = + static member M([] ?step: int) = + let step = Microsoft.FSharp.Core.Operators.defaultValueArg step 1 + step + 1 +""")>] let ``Converting back to a struct optional parameter uses defaultValueArg`` (dotNetForm: string, structForm: string) = Assert.Equal(structForm, refactoredWith structTitle dotNetForm "step:") @@ -396,6 +534,79 @@ type C() = static member M(?x: int, ?y: int) = defaultArg x 0 + defaultArg y 1 """, "?y")>] +[] x: int) = x +""", + "x:")>] +[] x: int) = x +""", + "x:")>] +[] x: string) = x +""", + "x:")>] +[] x: int, y: int) = x + y +""", + "x:")>] +[] x: outref) = x <- true +""", + "x:")>] +[] +type S = + val Value: int + +type C() = + static member M([] x: S) = x.Value +""", + "x:")>] +[] [] x: int[]) = x.Length +""", + "x:")>] let ``No action`` (code: string, marker: string) = Assert.Empty(actionsAt code marker) [] @@ -447,15 +658,13 @@ type C() = """ module M -open System.Runtime.InteropServices - module Other = open System.Runtime.InteropServices let o = 1 type C() = - static member M([] x: int) = x + static member M([] x: int) = x """ Assert.Equal(after, refactored before "?x") @@ -486,10 +695,8 @@ module Inner = Assert.Equal(after, refactored before "?x") -[] -let ``Converting to a struct optional parameter converts optional arguments to value options`` () = - let before = - """ +[] +[] +[([] step: int) = + step + sizeof<'T> + +let a = Counter.Next(?step = Some 2) +""", + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next<'T>([] ?step: int) = + let step = defaultValueArg step 1 + step + sizeof<'T> + +let a = Counter.Next(?step = ValueSome 2) +""")>] +[] step: int) = + step + 1 + +let a = Counter.Next(?step = ``Some`` 2) +let b = Counter.Next(?step = ``None``) +""", + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next([] ?step: int) = + let step = defaultValueArg step 1 + step + 1 + +let a = Counter.Next(?step = ValueSome 2) +let b = Counter.Next(?step = ValueNone) +""")>] +[] step: int) = + step + 1 +type Shadow = + | ValueSome of int + | ValueNone + +let a = Counter.Next(?step = Some 2) +let b = Counter.Next(?step = None) +""", + """ +module M + +open System.Runtime.InteropServices + +type Counter() = + static member Next([] ?step: int) = + let step = defaultValueArg step 1 + step + 1 + +type Shadow = + | ValueSome of int + | ValueNone + +let a = Counter.Next(?step = Microsoft.FSharp.Core.ValueOption.ValueSome 2) +let b = Counter.Next(?step = Microsoft.FSharp.Core.ValueOption.ValueNone) +""")>] +let ``Converting to a struct optional parameter converts optional arguments to value options`` (before: string, after: string) = Assert.Equal(after, refactoredWith structTitle before "step:") +[] +let ``Escaped parameter name keeps its backticks`` () = + let fsharpForm = + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M(?``type``: int) = defaultArg ``type`` 7 +""" + + let dotNetForm = + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M([] ``type``: int) = ``type`` +""" + + let shadowingForm = + """ +module M + +open System.Runtime.InteropServices + +type C() = + static member M(?``type``: int) = + let ``type`` = defaultArg ``type`` 7 + ``type`` +""" + + Assert.Equal(dotNetForm, refactored fsharpForm "?``type``") + Assert.Equal(shadowingForm, refactored dotNetForm "``type``:") + [] let ``No action when the file has a signature`` () = let signature = @@ -539,12 +862,115 @@ type C = let document = RoslynTestHelpers.GetFsiAndFsDocuments signature fsharpForm |> Seq.last - let actions = ResizeArray() + Assert.False(document.IsFSharpSignatureFile) + Assert.Empty(actionsIn document "?x") - let context = - CodeRefactoringContext(document, TextSpan(caretAt fsharpForm "?x", 1), (fun action -> actions.Add action), CancellationToken.None) +let private counterDefinition = + """ +module Definition - (new FSharpConvertOptionalParameterDefaultValueRefactoring()).ComputeRefactoringsAsync(context).GetAwaiter().GetResult() +open System.Runtime.InteropServices - Assert.False(document.IsFSharpSignatureFile) - Assert.Empty(actions) +type Counter() = + static member Next([] step: int) = + step + 1 +""" + +let private counterCallers = + """ +module Callers + +let a = Definition.Counter.Next() +let b = Definition.Counter.Next(?step = Some 2) +""" + +let private inOneProject (definition: string) (callers: string) = + { SyntheticProject.Create(fileOf "Definition" definition, fileOf "Callers" callers) with + AutoAddModules = false + } + +[] +let ``Converting to a struct optional parameter converts optional arguments in another file`` () = + let project = inOneProject counterDefinition counterCallers + let solution, _ = RoslynTestHelpers.CreateSolution project + use _ = new TestContext(solution) + + let action = + actionsIn (documentOf solution project "Definition") "step:" + |> Seq.find (hasTitle structTitle) + + let operation = + action.GetOperationsAsync CancellationToken.None + |> GetTaskResult + |> Seq.exactlyOne + + let changed = (operation :?> ApplyChangesOperation).ChangedSolution + + let definition = textOf (documentOf changed project "Definition") + let callers = textOf (documentOf changed project "Callers") + + Assert.Equal( + """ +module Definition + +open System.Runtime.InteropServices + +type Counter() = + static member Next([] ?step: int) = + let step = defaultValueArg step 1 + step + 1 +""", + definition + ) + + Assert.Equal( + """ +module Callers + +let a = Definition.Counter.Next() +let b = Definition.Counter.Next(?step = ValueSome 2) +""", + callers + ) + + // The checker reads the other file of a synthetic project from disk, so the result is checked as a project of its own. + let checkedSolution, _ = + RoslynTestHelpers.CreateSolution(inOneProject definition callers) + + use _ = new TestContext(checkedSolution) + + for document in (Seq.exactlyOne checkedSolution.Projects).Documents do + assertNoErrors document + +[] +[] +[] +let ``Struct optional parameter is offered only when every dependent F# project supports it`` + (dependentLangVersion: string, isOffered: bool) + = + let library = + { SyntheticProject.Create("Library", fileOf "Definition" counterDefinition) with + AutoAddModules = false + } + + let app = + { SyntheticProject.Create("App", fileOf "Callers" counterCallers) with + AutoAddModules = false + DependsOn = [ library ] + OtherOptions = [ dependentLangVersion ] + } + + let struct (solution, _) = RoslynTestHelpers.CreateMultiProjectSolution app + use _ = new TestContext(solution) + + let titles = + actionsIn (documentOf solution library "Definition") "step:" |> titlesOf + + Assert.Equal( + [ + fsharpTitle + if isOffered then + structTitle + ], + titles + ) From b2e1b5debc203424b266f6002567738fe7873ba9 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 15 Oct 2026 03:38:12 +0100 Subject: [PATCH 7/9] Name the optional parameter default value conversion steps instead of commenting them The doc comments of ConvertOptionalParameterDefaultValue.fs move into names: the Form cases FSharpOptional and DotNetOptional, fsharpCoreNameAt, IdentsAndLast, tryAnnotatedParameter, hasOnlyOptionalAttributes, tryShadowingLineSpan, whiteSpaceBeforeBody, passesMissingValueWhenOmitted, isUseInMemberBody and projectAndDependentsSupportStructOptionals. Only the license header stays. ComputeRefactoringsAsync nested nine levels of control flow. The actions are now built by actionsFor and the solution by changedSolution, so the provider nests three. tryForm and tryParameter go from four levels to two through tryDotNetOptional and searchResultAt. Repeated pieces are merged: tryGetSymbolUse serves the provider and isFSharpCoreSymbol, documentAction builds the two document actions, Range.rangeContainsPos replaces hand-written position comparisons and SingleIdent replaces isOperator. tryGetSymbolUse, isInFSharpCore and isFSharpCoreSymbol are the same text as in ConvertOptionalParameterStruct.fs of the optional parameter struct refactoring (PR #20546); the two pull requests keep their own copies. Co-Authored-By: Claude Sonnet 5.5 --- .../ConvertOptionalParameterDefaultValue.fs | 490 +++++++++--------- 1 file changed, 238 insertions(+), 252 deletions(-) diff --git a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs index 47dbf8741cd..3504a465042 100644 --- a/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs +++ b/vsintegration/src/FSharp.Editor/Refactor/ConvertOptionalParameterDefaultValue.fs @@ -4,8 +4,6 @@ namespace Microsoft.VisualStudio.FSharp.Editor open System open System.Composition -open System.Threading -open System.Threading.Tasks open Microsoft.CodeAnalysis open Microsoft.CodeAnalysis.CodeActions @@ -26,10 +24,8 @@ module private OptionalParameterDefaultValueConversion = [] type Form = - /// `?x: T` - | FSharp of optionalValRange: range - /// `[] x: T`, the parameter having no other attribute. - | DotNet of lists: SynAttributeList list * defaultValue: SynExpr voption + | FSharpOptional of optionalValRange: range + | DotNetOptional of lists: SynAttributeList list * defaultValue: SynExpr voption [] type Parameter = @@ -54,39 +50,39 @@ module private OptionalParameterDefaultValueConversion = let private isSame (expr: SynExpr) (other: SynExpr) = obj.ReferenceEquals(expr, other) - /// Whether the long identifier resolves to a value or union case of the FSharp.Core entity with the compiled full name. - let isFSharpCoreSymbol + let tryGetSymbolUse (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (longIdent: Ident list) = + longIdent + |> List.tryLast + |> Option.bind (fun ident -> + let line = sourceText.Lines[Line.toZ ident.idRange.EndLine].ToString() + + checkResults.GetSymbolUseAtLocation(ident.idRange.EndLine, ident.idRange.EndColumn, line, longIdent |> List.map _.idText)) + + let private isInFSharpCore (entityFullName: string) (symbol: FSharpSymbol) = + let declaringEntityName = + match symbol with + | :? FSharpUnionCase as case -> case.DeclaringEntity.TryFullName + | :? FSharpMemberOrFunctionOrValue as value -> value.DeclaringEntity |> Option.bind _.TryFullName + | _ -> None + + match declaringEntityName with + | Some name -> + String.Equals(name, entityFullName, StringComparison.Ordinal) + && String.Equals(symbol.Assembly.SimpleName, "FSharp.Core", StringComparison.Ordinal) + | None -> false + + let private isFSharpCoreSymbol (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (entityFullName: string) (longIdent: Ident list) = - match List.tryLast longIdent with - | None -> false - | Some ident -> - let line = sourceText.Lines[Line.toZ ident.idRange.EndLine].ToString() - let names = longIdent |> List.map _.idText - - match checkResults.GetSymbolUseAtLocation(ident.idRange.EndLine, ident.idRange.EndColumn, line, names) with - | None -> false - | Some symbolUse -> - let declaringEntityName = - match symbolUse.Symbol with - | :? FSharpUnionCase as case -> case.DeclaringEntity.TryFullName - | :? FSharpMemberOrFunctionOrValue as value -> value.DeclaringEntity |> Option.bind _.TryFullName - | _ -> None - - match declaringEntityName with - | Some name -> - String.Equals(name, entityFullName, StringComparison.Ordinal) - && String.Equals(symbolUse.Symbol.Assembly.SimpleName, "FSharp.Core", StringComparison.Ordinal) - | None -> false + tryGetSymbolUse checkResults sourceText longIdent + |> Option.exists (fun symbolUse -> isInFSharpCore entityFullName symbolUse.Symbol) let private operators = "Microsoft.FSharp.Core.Operators" - /// How to write a value or union case of FSharp.Core at the position: by its name where that resolves to it, - /// and with its namespace and its module or type where the name means something else. - let private coreName + let private fsharpCoreNameAt (checkResults: FSharpCheckFileResults) (compiledContainer: string) (container: string) @@ -129,9 +125,8 @@ module private OptionalParameterDefaultValueConversion = | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ ident ])) -> ValueSome ident | _ -> ValueNone - /// The identifiers of a name, qualified or not, and the last of them. [] - let private (|Name|_|) (expr: SynExpr) = + let private (|IdentsAndLast|_|) (expr: SynExpr) = match expr with | SynExpr.Ident ident -> ValueSome(struct ([ ident ], ident)) | SynExpr.LongIdent(longDotId = SynLongIdent(id = ids)) -> @@ -153,72 +148,74 @@ module private OptionalParameterDefaultValueConversion = let private isStruct = isAttribute [ "Struct"; "StructAttribute" ] - /// The annotated parameter the pattern declares, with the attributes around it and the range of `?x`, if any. - let rec private tryShape (attributes: SynAttributes) (pat: SynPat) = + let rec private tryAnnotatedParameter (attributes: SynAttributes) (pat: SynPat) = match pat with - | SynPat.Paren(pat = inner) -> tryShape attributes inner - | SynPat.Attrib(pat = inner; attributes = more) -> tryShape [ yield! attributes; yield! more ] inner + | SynPat.Paren(pat = inner) -> tryAnnotatedParameter attributes inner + | SynPat.Attrib(pat = inner; attributes = more) -> tryAnnotatedParameter [ yield! attributes; yield! more ] inner | SynPat.Typed(pat = SynPat.OptionalVal(ident, m); targetType = typeName) -> ValueSome(struct (ident, typeName, attributes, ValueSome m)) | SynPat.Typed(pat = SynPat.Named(ident = SynIdent(ident, _); isThisVal = false); targetType = typeName) -> ValueSome(struct (ident, typeName, attributes, ValueNone)) | _ -> ValueNone + let private tryDotNetOptional (attributes: SynAttributes) = + let listed = attributes |> List.collect _.Attributes + + let hasOnlyOptionalAttributes = + List.exists isOptional listed + && listed + |> List.forall (fun attribute -> isOptional attribute || isDefaultParameterValue attribute) + + match List.tryFind isDefaultParameterValue listed with + | _ when not hasOnlyOptionalAttributes -> ValueNone + | None -> ValueSome(Form.DotNetOptional(attributes, ValueNone)) + | Some attribute -> + match attribute.ArgExpr with + | SynExpr.Paren(expr = SynExpr.Const _ as constant) -> ValueSome(Form.DotNetOptional(attributes, ValueSome constant)) + | _ -> ValueNone + let private tryForm (attributes: SynAttributes) (optionalValRange: range voption) = match optionalValRange with | ValueSome _ when attributes |> List.exists (fun list -> List.exists isStruct list.Attributes) -> ValueNone - | ValueSome m -> ValueSome(Form.FSharp m) - | ValueNone -> - let isOptionalAttribute attribute = - isOptional attribute || isDefaultParameterValue attribute - - let listed = attributes |> List.collect _.Attributes + | ValueSome m -> ValueSome(Form.FSharpOptional m) + | ValueNone -> tryDotNetOptional attributes - if not (List.exists isOptional listed && List.forall isOptionalAttribute listed) then - ValueNone - else - match List.tryFind isDefaultParameterValue listed with - | None -> ValueSome(Form.DotNet(attributes, ValueNone)) - | Some attribute -> - match attribute.ArgExpr with - | SynExpr.Paren(expr = SynExpr.Const _ as constant) -> ValueSome(Form.DotNet(attributes, ValueSome constant)) - | _ -> ValueNone - - let rec private tryMember (path: SyntaxVisitorPath) = + let rec private tryEnclosingMember (path: SyntaxVisitorPath) = match path with | SyntaxNode.SynBinding(SynBinding(headPat = SynPat.LongIdent(longDotId = SynLongIdent(id = ids))) as binding) :: SyntaxNode.SynMemberDefn(SynMemberDefn.Member _) :: _ -> match List.tryLast ids with | Some name -> ValueSome(struct (name, binding)) | None -> ValueNone - | SyntaxNode.SynPat _ :: rest -> tryMember rest + | SyntaxNode.SynPat _ :: rest -> tryEnclosingMember rest | _ -> ValueNone + let private searchResultAt (path: SyntaxVisitorPath) (pat: SynPat) = + match tryAnnotatedParameter [] pat, tryEnclosingMember path with + | ValueSome(struct (ident, typeName, attributes, optionalValRange)), ValueSome(struct (memberName, binding)) -> + tryForm attributes optionalValRange + |> ValueOption.map (fun form -> + { + Ident = ident + TypeName = typeName + Form = form + MemberName = memberName + MemberBinding = binding + }) + |> Search.Found + | _ -> Search.Searching + let tryParameter (caret: pos) (parseTree: ParsedInput) = let search = (Search.Searching, parseTree) ||> ParsedInput.fold (fun search path node -> match search, node with - | Search.Searching, SyntaxNode.SynPat pat when Position.posGeq caret pat.Range.Start && Position.posGeq pat.Range.End caret -> - match tryShape [] pat, tryMember path with - | ValueSome(struct (ident, typeName, attributes, optionalValRange)), ValueSome(struct (memberName, binding)) -> - tryForm attributes optionalValRange - |> ValueOption.map (fun form -> - { - Ident = ident - TypeName = typeName - Form = form - MemberName = memberName - MemberBinding = binding - }) - |> Search.Found - | _ -> Search.Searching + | Search.Searching, SyntaxNode.SynPat pat when Range.rangeContainsPos pat.Range caret -> searchResultAt path pat | _ -> search) match search with | Search.Found parameter -> parameter | Search.Searching -> ValueNone - /// Whether the constant can be the `DefaultParameterValue` of a parameter of the type with the full name. let isConstantOfType (typeFullName: string option) (constant: SynConst) = match typeFullName, constant with | Some "System.Int32", SynConst.Int32 _ @@ -248,22 +245,25 @@ module private OptionalParameterDefaultValueConversion = Some(expr, path) | _ -> None) - /// `defaultArg x c` around a use of the parameter, with its constant and what contains it. - let private tryDefaultArgUse (isFSharpCore: string -> Ident list -> bool) (parseTree: ParsedInput) (useRange: range) = + let private tryDefaultArgUse + (checkResults: FSharpCheckFileResults) + (sourceText: SourceText) + (parseTree: ParsedInput) + (useRange: range) + = match tryUseNode parseTree useRange with | Some(node, - SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = Name(func, name); argExpr = arg) as inner) :: SyntaxNode.SynExpr(SynExpr.App( + SyntaxNode.SynExpr(SynExpr.App(isInfix = false; funcExpr = IdentsAndLast(func, name); argExpr = arg) as inner) :: SyntaxNode.SynExpr(SynExpr.App( isInfix = false; funcExpr = funcExpr; argExpr = (SynExpr.Const _ as defaultValue)) as application) :: rest) when isSame arg node && hasText "defaultArg" name && isSame funcExpr inner - && isFSharpCore operators func + && isFSharpCoreSymbol checkResults sourceText operators func -> ValueSome(struct (application, defaultValue, rest)) | _ -> ValueNone - /// The whole line of `let x = defaultArg x c` when that line does nothing but rebind the parameter immutably. - let private tryShadowingLine (sourceText: SourceText) (name: string) (application: SynExpr) (path: SyntaxVisitorPath) = + let private tryShadowingLineSpan (sourceText: SourceText) (name: string) (application: SynExpr) (path: SyntaxVisitorPath) = match path with | SyntaxNode.SynBinding(SynBinding( isMutable = false; headPat = SynPat.Named(ident = SynIdent(ident, _)); expr = rhs; trivia = trivia)) :: SyntaxNode.SynExpr(SynExpr.LetOrUse letOrUse) :: _ when @@ -306,7 +306,6 @@ module private OptionalParameterDefaultValueConversion = | _ :: rest -> tryContainerRange rest | [] -> ValueNone - /// Whether an `open System.Runtime.InteropServices` in a module or namespace enclosing the position precedes it. let private hasInteropServicesOpenAt (position: pos) (parseTree: ParsedInput) = (false, parseTree) ||> ParsedInput.fold (fun found path node -> @@ -319,17 +318,12 @@ module private OptionalParameterDefaultValueConversion = && List.forall2 hasText interopServices ids -> match tryContainerRange path with - | ValueSome container -> - Position.posGeq position container.Start - && Position.posGeq container.End position + | ValueSome container -> Range.rangeContainsPos container position | ValueNone -> false | _ -> false) - /// `?x: T` with every use being `defaultArg x c` becomes `[] x: T`, - /// unless a `?` parameter before it would then precede a non-optional one (FS1212). - /// The attributes carry their namespace where no `open` of it is in scope. let tryToDotNetChanges - (isFSharpCore: string -> Ident list -> bool) + (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (parseTree: ParsedInput) (parameter: Parameter) @@ -342,7 +336,7 @@ module private OptionalParameterDefaultValueConversion = let found = (ValueSome [], uses) ||> List.fold (fun found useRange -> - match found, tryDefaultArgUse isFSharpCore parseTree useRange with + match found, tryDefaultArgUse checkResults sourceText parseTree useRange with | ValueSome found, ValueSome defaultArgUse -> ValueSome(defaultArgUse :: found) | _ -> ValueNone) @@ -379,7 +373,7 @@ module private OptionalParameterDefaultValueConversion = TextChange(TextSpan((spanOf sourceText optionalValRange).Start, 1), $"[<{attributeText}>] ") for struct (application, _, path) in found do - match tryShadowingLine sourceText parameter.Ident.idText application path with + match tryShadowingLineSpan sourceText parameter.Ident.idText application path with | ValueSome line -> TextChange(line, "") | ValueNone -> TextChange(spanOf sourceText application.Range, name) ] @@ -425,8 +419,6 @@ module private OptionalParameterDefaultValueConversion = | SynPat.LongIdent(argPats = SynArgPats.Pats pats) -> pats |> List.exists (hasRequiredAfter position) | _ -> false - /// `[] x: T` becomes `?x: T` with `let x = defaultArg x c` starting the body, - /// or `[] ?x: T` with `let x = defaultValueArg x c`, unless a non-optional parameter follows it (FS1212). let tryToFSharpChanges (checkResults: FSharpCheckFileResults) (sourceText: SourceText) @@ -458,7 +450,7 @@ module private OptionalParameterDefaultValueConversion = | _ -> body let defaultFunction = - coreName checkResults "Operators" "Operators" defaultFunction body.Range.Start + fsharpCoreNameAt checkResults "Operators" "Operators" defaultFunction body.Range.Start let declaration = $"let {name} = {defaultFunction} {name} {defaultText}" let memberLine = sourceText.Lines[Line.toZ trivia.LeadingKeyword.Range.StartLine] @@ -473,13 +465,10 @@ module private OptionalParameterDefaultValueConversion = let indent = String(' ', leadingSpaces memberLine + indentSize) let bodyStart = (spanOf sourceText body.Range).Start - // A comment between `=` and the body stays on the member line. - ValueSome( - TextChange( - TextSpan.FromBounds(startOfWhiteSpaceBefore sourceText bodyStart, bodyStart), - $"{lineBreak}{indent}{declaration}{lineBreak}{indent}" - ) - ) + let whiteSpaceBeforeBody = + TextSpan.FromBounds(startOfWhiteSpaceBefore sourceText bodyStart, bodyStart) + + ValueSome(TextChange(whiteSpaceBeforeBody, $"{lineBreak}{indent}{declaration}{lineBreak}{indent}")) | _ -> ValueNone match bodyChange with @@ -495,11 +484,6 @@ module private OptionalParameterDefaultValueConversion = |> ValueSome | _ -> ValueNone - let private isOperator (name: string) (expr: SynExpr) = - match expr with - | SynExpr.LongIdent(longDotId = SynLongIdent(id = [ operator ])) -> hasText name operator - | _ -> false - let rec private functionOf (expr: SynExpr) = match expr with | SynExpr.App(isInfix = false; funcExpr = funcExpr) -> functionOf funcExpr @@ -513,7 +497,6 @@ module private OptionalParameterDefaultValueConversion = | SynExpr.Const _ -> true | _ -> false - /// The values passed as `?name = value` in the arguments of a call. let private optionalArgumentValues (name: string) (arguments: SynExpr) = let arguments = match arguments with @@ -527,12 +510,11 @@ module private OptionalParameterDefaultValueConversion = isInfix = false funcExpr = SynExpr.App( isInfix = true - funcExpr = equals + funcExpr = SingleIdent equals argExpr = SynExpr.LongIdent(isOptional = true; longDotId = SynLongIdent(id = [ argumentName ]))) - argExpr = value) when isOperator "op_Equality" equals && hasText name argumentName -> Some value + argExpr = value) when hasText "op_Equality" equals && hasText name argumentName -> Some value | _ -> None) - /// `Some v` becomes `ValueSome v`, `None` becomes `ValueNone`, and any other option goes through `ValueOption.ofOption`. let private valueOptionChanges (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (value: SynExpr) = let valueSpan = spanOf sourceText value.Range @@ -546,7 +528,7 @@ module private OptionalParameterDefaultValueConversion = [ TextChange( spanOf sourceText head.idRange, - coreName checkResults "FSharpValueOption`1" "ValueOption" case head.idRange.Start + fsharpCoreNameAt checkResults "FSharpValueOption`1" "ValueOption" case head.idRange.Start ) ] | _ when isAtomic value -> [ TextChange(TextSpan(valueSpan.Start, 0), "ValueOption.ofOption ") ] @@ -556,8 +538,6 @@ module private OptionalParameterDefaultValueConversion = TextChange(TextSpan(valueSpan.End, 0), ")") ] - /// Changes to the `?name = value` arguments of the call whose function ends at the use of the member, - /// for the parameter becoming a `[] ?name`. let callSiteChanges (checkResults: FSharpCheckFileResults) (sourceText: SourceText) @@ -591,10 +571,6 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ Seq.exists (fun d -> String.Equals(d.FilePath, signaturePath, StringComparison.OrdinalIgnoreCase)) - static let tryGetSymbolUse (checkResults: FSharpCheckFileResults) (sourceText: SourceText) (ident: Ident) = - let line = sourceText.Lines[Line.toZ ident.idRange.EndLine].ToString() - checkResults.GetSymbolUseAtLocation(ident.idRange.EndLine, ident.idRange.EndColumn, line, [ ident.idText ]) - static let isConvertibleMember (symbol: FSharpSymbol) = match symbol with | :? FSharpMemberOrFunctionOrValue as mfv -> @@ -633,7 +609,6 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ @@ -655,27 +630,31 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ false) | _ -> ValueNone - /// Whether `?x` can replace the attributes of the parameter: they are the `OptionalAttribute` and at most the - /// `DefaultParameterValueAttribute` of `System.Runtime.InteropServices`, the default value is one the compiler - /// passes for a parameter of this type, and an option of the type can exist. - static let isDotNetOptional (parameter: FSharpParameter) (defaultValue: SynExpr voption) = + static let canBeFSharpOptional (defaultValue: SynExpr voption) (parameter: FSharpParameter) = let attributes = parameter.Attributes + let passesMissingValueWhenOmitted (definition: FSharpEntity) = hasFullName "System.Object" definition + attributes |> Seq.exists isInteropOptional && attributes |> Seq.forall (fun attribute -> isInteropOptional attribute || isInteropDefaultParameterValue attribute) && match tryTypeDefinition parameter.Type, defaultValue with | ValueSome definition, _ when isByRefLike definition -> false - // `[]` alone passes `Missing.Value` for an omitted `obj`, not the `Unchecked.defaultof` that `?` would give. - | ValueSome definition, ValueNone -> not (hasFullName "System.Object" definition) + | ValueSome definition, ValueNone -> not (passesMissingValueWhenOmitted definition) | ValueSome definition, ValueSome(SynExpr.Const(constant, _)) -> OptionalParameterDefaultValueConversion.isConstantOfType definition.TryFullName constant | ValueNone, ValueNone -> true | _ -> false - /// Whether the project and the F# projects that depend on it take a value option for a `?` parameter, - /// which a caller compiled for an earlier language version cannot pass. - static let supportsStructOptionalParameters (project: Project) = + static let isConvertibleDotNetOptional + (memberSymbol: FSharpSymbol) + (parameter: OptionalParameterDefaultValueConversion.Parameter) + (defaultValue: SynExpr voption) + = + tryParameterSymbol memberSymbol parameter.Ident.idText + |> ValueOption.exists (canBeFSharpOptional defaultValue) + + static let projectAndDependentsSupportStructOptionals (project: Project) = cancellableTask { let solution = project.Solution @@ -703,6 +682,140 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ Seq.groupBy (fun (useDocument: Document, _) -> useDocument.Id) + |> Seq.toArray + + let mutable solution = document.Project.Solution + + for documentId, documentUses in usesByDocument do + let useDocument = solution.GetDocument documentId + let! text = useDocument.GetTextAsync cancellationToken + + let! useParseResults, useCheckResults = + useDocument.GetFSharpParseAndCheckResultsAsync(nameof FSharpConvertOptionalParameterDefaultValueRefactoring) + + let changes = + [ + if documentId = document.Id then + yield! definitionChanges + + for _, useRange in documentUses do + yield! + OptionalParameterDefaultValueConversion.callSiteChanges + useCheckResults + text + useParseResults.ParseTree + parameter.Ident.idText + useRange + ] + |> List.distinctBy _.Span + |> List.sortBy _.Span.Start + + solution <- solution.WithDocumentText(documentId, text.WithChanges changes) + + if not (usesByDocument |> Array.exists (fun (documentId, _) -> documentId = document.Id)) then + solution <- solution.WithDocumentText(document.Id, sourceText.WithChanges definitionChanges) + + return solution + } + + static let actionsFor + (document: Document) + (sourceText: SourceText) + (parseTree: ParsedInput) + (checkResults: FSharpCheckFileResults) + (memberUse: FSharpSymbolUse) + (parameter: OptionalParameterDefaultValueConversion.Parameter) + = + cancellableTask { + let! cancellationToken = CancellableTask.getCancellationToken () + + match parameter.Form with + | OptionalParameterDefaultValueConversion.Form.FSharpOptional optionalValRange -> + match OptionalParameterDefaultValueConversion.tryGetSymbolUse checkResults sourceText [ parameter.Ident ] with + | Some parameterUse -> + let memberRange = parameter.MemberBinding.RangeOfBindingWithRhs + + let isUseInMemberBody (symbolUse: FSharpSymbolUse) = + not symbolUse.IsFromDefinition + && Position.posGeq symbolUse.Range.Start memberRange.Start + && Position.posGeq memberRange.End symbolUse.Range.End + + let usesInBody = + checkResults.GetUsesOfSymbolInFile(parameterUse.Symbol, cancellationToken = cancellationToken) + |> Seq.filter isUseInMemberBody + |> Seq.map _.Range + |> List.ofSeq + + return + OptionalParameterDefaultValueConversion.tryToDotNetChanges + checkResults + sourceText + parseTree + parameter + (tryOptionalValueTypeName parameterUse.Symbol) + optionalValRange + usesInBody + |> ValueOption.map (documentAction document sourceText (SR.UseDotNetOptionalParameter())) + |> ValueOption.toList + | None -> return [] + | OptionalParameterDefaultValueConversion.Form.DotNetOptional(lists, defaultValue) when + isConvertibleDotNetOptional memberUse.Symbol parameter defaultValue + -> + let! options = document.GetOptionsAsync cancellationToken + let! isStructSupported = projectAndDependentsSupportStructOptionals document.Project + + let indentSize = + options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) + + let tryChanges (asStruct: bool) = + OptionalParameterDefaultValueConversion.tryToFSharpChanges + checkResults + sourceText + indentSize + parameter + lists + defaultValue + asStruct + + return + [ + match tryChanges false with + | ValueSome changes -> yield documentAction document sourceText (SR.UseFSharpOptionalParameter()) changes + | ValueNone -> () + + match tryChanges true with + | ValueSome definitionChanges when isStructSupported -> + let title = SR.UseFSharpStructOptionalParameter() + + yield + CodeAction.Create( + title, + changedSolution document sourceText checkResults memberUse parameter definitionChanges, + title + ) + | _ -> () + ] + | OptionalParameterDefaultValueConversion.Form.DotNetOptional _ -> return [] + } + override _.ComputeRefactoringsAsync context = cancellableTask { let document = context.Document @@ -722,137 +835,10 @@ type internal FSharpConvertOptionalParameterDefaultValueRefactoring [ - match parameter.Form with - | OptionalParameterDefaultValueConversion.Form.FSharp optionalValRange -> - match tryGetSymbolUse checkResults sourceText parameter.Ident with - | Some parameterUse -> - let memberRange = parameter.MemberBinding.RangeOfBindingWithRhs - - let uses = - checkResults.GetUsesOfSymbolInFile(parameterUse.Symbol, cancellationToken = cancellationToken) - // Named arguments at call sites are uses of the parameter too; only the body matters here. - |> Seq.filter (fun symbolUse -> - not symbolUse.IsFromDefinition - && Position.posGeq symbolUse.Range.Start memberRange.Start - && Position.posGeq memberRange.End symbolUse.Range.End) - |> Seq.map _.Range - |> List.ofSeq - - match - OptionalParameterDefaultValueConversion.tryToDotNetChanges - (OptionalParameterDefaultValueConversion.isFSharpCoreSymbol checkResults sourceText) - sourceText - parseResults.ParseTree - parameter - (tryOptionalValueTypeName parameterUse.Symbol) - optionalValRange - uses - with - | ValueSome changes -> - let title = SR.UseDotNetOptionalParameter() - - let changedDocument = - cancellableTask { return document.WithText(sourceText.WithChanges changes) } - - context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) - | ValueNone -> () - | None -> () - | OptionalParameterDefaultValueConversion.Form.DotNet(lists, defaultValue) -> - let isConvertible = - match tryParameterSymbol memberUse.Symbol parameter.Ident.idText with - | ValueSome parameterSymbol -> isDotNetOptional parameterSymbol defaultValue - | ValueNone -> false - - if isConvertible then - let! options = document.GetOptionsAsync cancellationToken - let! isStructSupported = supportsStructOptionalParameters document.Project - - let indentSize = - options.GetOption(FormattingOptions.IndentationSize, FSharpConstants.FSharpLanguageName) - - let tryChanges (asStruct: bool) = - OptionalParameterDefaultValueConversion.tryToFSharpChanges - checkResults - sourceText - indentSize - parameter - lists - defaultValue - asStruct - - match tryChanges false with - | ValueSome changes -> - let title = SR.UseFSharpOptionalParameter() - - let changedDocument = - cancellableTask { return document.WithText(sourceText.WithChanges changes) } - - context.RegisterRefactoring(CodeAction.Create(title, changedDocument, title)) - | ValueNone -> () - - match tryChanges true with - | ValueSome definitionChanges when isStructSupported -> - let title = SR.UseFSharpStructOptionalParameter() - - // `?x = Some 1` passes an option to `[] x`; a struct `?x` takes a value option instead. - let changedSolution = - cancellableTask { - let! cancellationToken = CancellableTask.getCancellationToken () - let! memberUses = SymbolHelpers.getSymbolUses memberUse document checkResults - - let usesByDocument = - memberUses - |> Seq.groupBy (fun (useDocument: Document, _) -> useDocument.Id) - |> Seq.toArray - - let mutable solution = document.Project.Solution - - for documentId, documentUses in usesByDocument do - let useDocument = solution.GetDocument documentId - let! text = useDocument.GetTextAsync cancellationToken - - let! useParseResults, useCheckResults = - useDocument.GetFSharpParseAndCheckResultsAsync( - nameof FSharpConvertOptionalParameterDefaultValueRefactoring - ) - - let changes = - [ - if documentId = document.Id then - yield! definitionChanges - - for _, useRange in documentUses do - yield! - OptionalParameterDefaultValueConversion.callSiteChanges - useCheckResults - text - useParseResults.ParseTree - parameter.Ident.idText - useRange - ] - |> List.distinctBy _.Span - |> List.sortBy _.Span.Start - - solution <- solution.WithDocumentText(documentId, text.WithChanges changes) - - if not (usesByDocument |> Array.exists (fun (documentId, _) -> documentId = document.Id)) then - solution <- solution.WithDocumentText(document.Id, sourceText.WithChanges definitionChanges) - - return solution - } - - let action = - CodeAction.Create( - title, - Func>(fun cancellationToken -> - CancellableTask.start cancellationToken changedSolution), - title - ) - - context.RegisterRefactoring action - | _ -> () + let! actions = actionsFor document sourceText parseResults.ParseTree checkResults memberUse parameter + List.iter context.RegisterRefactoring actions | _ -> () } |> CancellableTask.startAsTask context.CancellationToken From c684dd3640ec877e0b9f221e155b2eb971abba88 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Thu, 15 Oct 2026 03:38:13 +0100 Subject: [PATCH 8/9] Parameterize the optional parameter default value conversion tests The module header and the System.Runtime.InteropServices open come from two helpers instead of being written into every test string. The one-off conversion facts and the two struct conversion theories become rows of three named theories, every row of the theories gets a name so a failure says which scenario it was, and the comment about re-checking a synthetic project is the name of a helper, assertCompilesAsProject. Every input and every expected result is kept: 47 cases before and after. Co-Authored-By: Claude Sonnet 5.5 --- ...nvertOptionalParameterDefaultValueTests.fs | 906 ++++++------------ 1 file changed, 294 insertions(+), 612 deletions(-) diff --git a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs index 332ebdb19db..6866844d3c8 100644 --- a/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs +++ b/vsintegration/tests/FSharp.Editor.Tests/Refactors/ConvertOptionalParameterDefaultValueTests.fs @@ -19,6 +19,17 @@ open FSharp.Editor.Tests.Helpers open FSharp.Editor.Tests.Refactors.RefactorTestFramework open FSharp.Test.ProjectGeneration +let private newLineOf (code: string) = + if code.IndexOf('\r') >= 0 then "\r\n" else "\n" + +let private moduleM (code: string) = + let newLine = newLineOf code + $"{newLine}module M{newLine}{newLine}{code}{newLine}" + +let private moduleMWithInterop (code: string) = + let newLine = newLineOf code + moduleM $"open System.Runtime.InteropServices{newLine}{newLine}{code}" + let private caretAt (code: string) (marker: string) = code.IndexOf(marker, StringComparison.Ordinal) @@ -62,6 +73,12 @@ let private refactored (code: string) (marker: string) = refactoredBy Seq.head c let private refactoredWith (title: string) (code: string) (marker: string) = refactoredBy (Seq.find (hasTitle title)) code marker +let private assertRefactored (wrap: string -> string) (marker: string) (before: string) (after: string) = + Assert.Equal(wrap after, refactored (wrap before) marker) + +let private assertRefactoredWith (title: string) (wrap: string -> string) (marker: string) (before: string) (after: string) = + Assert.Equal(wrap after, refactoredWith title (wrap before) marker) + let private actionsAt (code: string) (marker: string) = use context = TestContext.CreateWithCode code tryGetRefactoringActions code (caretAt code marker) context (new FSharpConvertOptionalParameterDefaultValueRefactoring()) @@ -81,6 +98,9 @@ let private actionsIn (document: Document) (marker: string) = actions +let private titlesOf (actions: CodeAction seq) = + actions |> Seq.map _.Title |> List.ofSeq + let private documentOf (solution: Solution) (project: SyntheticProject) (fileId: string) = solution.GetDocumentIdsWithFilePath(project.GetFilePath fileId) |> Seq.exactlyOne @@ -91,276 +111,182 @@ let private fileOf (fileId: string) (source: string) = Source = source } -[] -[] x: int) = x""" + +[] +[] greeting: string) = $"{greeting}, {name}" let a = Greeter().Greet("Ada") -let b = Greeter().Greet("Ada", greeting = "Hi") -""")>] -[] +[] greeting: string) = greeting -let a = Greeter().Greet() -""")>] -let ``Attributes are fully qualified where System.Runtime.InteropServices is not open`` (before: string, after: string) = - Assert.Equal(after, refactored before "?greeting") +let a = Greeter().Greet()""")>] +let ``Attributes are fully qualified where System.Runtime.InteropServices is not open`` (_scenario: string, before: string, after: string) = + assertRefactored moduleM "?greeting" before after [] -[] step: int) = step + 1 -let n = Counter.Next() -""")>] -[] +[] step: float) = - float value + step -""")>] -[] +[] step: Step) = - step + 1 -""")>] -[] +[] step: int) = - step + 1 -""")>] -let ``Shadowing default converts both ways`` (fsharpForm: string, dotNetForm: string) = - Assert.Equal(dotNetForm, refactored fsharpForm "?step") - Assert.Equal(fsharpForm, refactored dotNetForm "step:") + step + 1""")>] +let ``Shadowing default converts both ways`` (_scenario: string, fsharpForm: string, dotNetForm: string) = + assertRefactored moduleMWithInterop "?step" fsharpForm dotNetForm + assertRefactored moduleMWithInterop "step:" dotNetForm fsharpForm -[] -let ``Inline defaults are replaced by the parameter`` () = - let before = - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next(value: int, ?step: int) = value + defaultArg step 1 -""" - - let after = - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next(value: int, [] step: int) = value + step -""" - - Assert.Equal(after, refactored before "?step") - -[] -let ``Body on the member line moves below it when converting back`` () = - let before = - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next(value: int, [] step: int) = value + step -""" - - let after = - """ -module M - -open System.Runtime.InteropServices - -type Counter() = +[] +[] step: int) = value + step""", + "?step")>] +[] step: int) = value + step""", + """type Counter() = static member Next(value: int, ?step: int) = let step = defaultArg step 1 - value + step -""" - - Assert.Equal(after, refactored before "step:") - -[] -[] flag: bool) = 1 -""")>] -[] +[] flag: bool) = 1""", + "?flag")>] +[] step: int) = - step + 1 -""", - "step:", - """ -module M - -open System.Runtime.InteropServices - -type C() = + step + 1""", + """type C() = static member M(?step: int) = let step = defaultArg step Unchecked.defaultof<_> - step + 1 -""")>] -[] step: int) = (* keep this *) step + 1 -""", - "step:", - """ -module M - -open System.Runtime.InteropServices - -type C() = + step + 1""", + "step:")>] +[] step: int) = (* keep this *) step + 1""", + """type C() = static member M(?step: int) = (* keep this *) let step = defaultArg step Unchecked.defaultof<_> - step + 1 -""")>] -let ``Optional without a default value`` (before: string, marker: string, after: string) = - Assert.Equal(after, refactored before marker) + step + 1""", + "step:")>] +[] x: int) = + let mutable x = x + x <- x + 1 + x""", + "?x")>] +let ``Optional parameter converts between the F# and the .NET form`` (_scenario: string, before: string, after: string, marker: string) = + assertRefactored moduleMWithInterop marker before after -let private fsharpForm = - """ -module M +[] +[] x: int) = x -""" + static member M([] x: int) = x""")>] +[ Seq.map _.Title |> List.ofSeq + type C() = + static member M(?x: int) = defaultArg x 0""", + """module Inner = + open System.Runtime.InteropServices + + type C() = + static member M([] x: int) = x""")>] +let ``Attributes are qualified unless an enclosing module opens System.Runtime.InteropServices`` + (_scenario: string, before: string, after: string) + = + assertRefactored moduleM "?x" before after [] let ``Title names the target form`` () = @@ -368,69 +294,104 @@ let ``Title names the target form`` () = Assert.Equal([ fsharpTitle; structTitle ], titlesOf (actionsAt dotNetForm "x:")) [] -[] step: int) = - value + step -""", - """ -module M - -open System.Runtime.InteropServices - -type Counter() = + value + step""", + """type Counter() = static member Next(value: int, [] ?step: int) = let step = defaultValueArg step 1 - value + step -""")>] -[] +[] step: int) = - step + 1 -""", - """ -module M + step + 1""", + """type C() = + static member M([] ?step: int) = + let step = defaultValueArg step Unchecked.defaultof<_> + step + 1""")>] +[] step: int) = + step + 1""", + """let defaultValueArg (_: int voption) (fallback: int) = fallback + 1 type C() = static member M([] ?step: int) = - let step = defaultValueArg step Unchecked.defaultof<_> + let step = Microsoft.FSharp.Core.Operators.defaultValueArg step 1 + step + 1""")>] +[] step: int) = step + 1 -""")>] -[] ?step: int) = + let step = defaultValueArg step 1 + step + 1 + +let a = Counter.Next() +let b = Counter.Next(?step = ValueSome 2) +let c = Counter.Next(?step = ValueNone) +let d (s: int option) = Counter.Next(?step = ValueOption.ofOption s) +let e (s: int list) = Counter.Next(?step = ValueOption.ofOption (List.tryHead s))""")>] +[([] step: int) = + step + sizeof<'T> -let defaultValueArg (_: int voption) (fallback: int) = fallback + 1 +let a = Counter.Next(?step = Some 2)""", + """type Counter() = + static member Next<'T>([] ?step: int) = + let step = defaultValueArg step 1 + step + sizeof<'T> -type C() = - static member M([] step: int) = +let a = Counter.Next(?step = ValueSome 2)""")>] +[] step: int) = step + 1 -""", - """ -module M -open System.Runtime.InteropServices +let a = Counter.Next(?step = ``Some`` 2) +let b = Counter.Next(?step = ``None``)""", + """type Counter() = + static member Next([] ?step: int) = + let step = defaultValueArg step 1 + step + 1 + +let a = Counter.Next(?step = ValueSome 2) +let b = Counter.Next(?step = ValueNone)""")>] +[] step: int) = + step + 1 -let defaultValueArg (_: int voption) (fallback: int) = fallback + 1 +type Shadow = + | ValueSome of int + | ValueNone -type C() = - static member M([] ?step: int) = - let step = Microsoft.FSharp.Core.Operators.defaultValueArg step 1 +let a = Counter.Next(?step = Some 2) +let b = Counter.Next(?step = None)""", + """type Counter() = + static member Next([] ?step: int) = + let step = defaultValueArg step 1 step + 1 -""")>] -let ``Converting back to a struct optional parameter uses defaultValueArg`` (dotNetForm: string, structForm: string) = - Assert.Equal(structForm, refactoredWith structTitle dotNetForm "step:") + +type Shadow = + | ValueSome of int + | ValueNone + +let a = Counter.Next(?step = Microsoft.FSharp.Core.ValueOption.ValueSome 2) +let b = Counter.Next(?step = Microsoft.FSharp.Core.ValueOption.ValueNone)""")>] +let ``Converting to a struct optional parameter`` (_scenario: string, before: string, after: string) = + assertRefactoredWith structTitle moduleMWithInterop "step:" before after [] let ``Struct optional parameter is not offered before F# 10`` () = @@ -443,150 +404,97 @@ let ``Struct optional parameter is not offered before F# 10`` () = Assert.Equal([ fsharpTitle ], titlesOf actions) [] -[] -[] -[] -[] -[] -[] ?x: int) = defaultValueArg x 0 -""", +[] ?x: int) = defaultValueArg x 0""", "?x")>] -[] x: int) = x -""", + static member M([] x: int) = x""", "x:")>] -[ int - default _.M(?x: int) = defaultArg x 0 -""", + default _.M(?x: int) = defaultArg x 0""", "?x: int)")>] -[] -[] -[] x: obj) = x -""", + static member M([] x: obj) = x""", "x:")>] -[] -[] x: int) = x -""", + static member M([] x: int) = x""", "x:")>] -[] x: int) = x -""", + static member M([] x: int) = x""", "x:")>] -[] x: string) = x -""", + static member M([] x: string) = x""", "x:")>] -[] x: int, y: int) = x + y -""", + static member M([] x: int, y: int) = x + y""", "x:")>] -[] x: outref) = x <- true -""", + static member M([] x: outref) = x <- true""", "x:")>] -[] @@ -594,270 +502,44 @@ type S = val Value: int type C() = - static member M([] x: S) = x.Value -""", + static member M([] x: S) = x.Value""", "x:")>] -[] [] x: int[]) = x.Length -""", + static member M([] [] x: int[]) = x.Length""", "x:")>] -let ``No action`` (code: string, marker: string) = Assert.Empty(actionsAt code marker) - -[] -let ``Mutable shadowing keeps its binding`` () = - let before = - """ -module M - -open System.Runtime.InteropServices - -type C() = - static member M(?x: int) = - let mutable x = defaultArg x 0 - x <- x + 1 - x -""" - - let after = - """ -module M - -open System.Runtime.InteropServices - -type C() = - static member M([] x: int) = - let mutable x = x - x <- x + 1 - x -""" - - Assert.Equal(after, refactored before "?x") - -[] -let ``Open in another module does not count as in scope`` () = - let before = - """ -module M - -module Other = - open System.Runtime.InteropServices - - let o = 1 - -type C() = - static member M(?x: int) = defaultArg x 0 -""" - - let after = - """ -module M - -module Other = - open System.Runtime.InteropServices - - let o = 1 - -type C() = - static member M([] x: int) = x -""" - - Assert.Equal(after, refactored before "?x") - -[] -let ``Open in an enclosing module counts as in scope`` () = - let before = - """ -module M - -module Inner = - open System.Runtime.InteropServices - - type C() = - static member M(?x: int) = defaultArg x 0 -""" - - let after = - """ -module M - -module Inner = - open System.Runtime.InteropServices - - type C() = - static member M([] x: int) = x -""" - - Assert.Equal(after, refactored before "?x") - -[] -[] step: int) = - step + 1 - -let a = Counter.Next() -let b = Counter.Next(?step = Some 2) -let c = Counter.Next(?step = None) -let d (s: int option) = Counter.Next(?step = s) -let e (s: int list) = Counter.Next(?step = List.tryHead s) -""", - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next([] ?step: int) = - let step = defaultValueArg step 1 - step + 1 - -let a = Counter.Next() -let b = Counter.Next(?step = ValueSome 2) -let c = Counter.Next(?step = ValueNone) -let d (s: int option) = Counter.Next(?step = ValueOption.ofOption s) -let e (s: int list) = Counter.Next(?step = ValueOption.ofOption (List.tryHead s)) -""")>] -[([] step: int) = - step + sizeof<'T> - -let a = Counter.Next(?step = Some 2) -""", - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next<'T>([] ?step: int) = - let step = defaultValueArg step 1 - step + sizeof<'T> - -let a = Counter.Next(?step = ValueSome 2) -""")>] -[] step: int) = - step + 1 - -let a = Counter.Next(?step = ``Some`` 2) -let b = Counter.Next(?step = ``None``) -""", - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next([] ?step: int) = - let step = defaultValueArg step 1 - step + 1 - -let a = Counter.Next(?step = ValueSome 2) -let b = Counter.Next(?step = ValueNone) -""")>] -[] step: int) = - step + 1 - -type Shadow = - | ValueSome of int - | ValueNone - -let a = Counter.Next(?step = Some 2) -let b = Counter.Next(?step = None) -""", - """ -module M - -open System.Runtime.InteropServices - -type Counter() = - static member Next([] ?step: int) = - let step = defaultValueArg step 1 - step + 1 - -type Shadow = - | ValueSome of int - | ValueNone - -let a = Counter.Next(?step = Microsoft.FSharp.Core.ValueOption.ValueSome 2) -let b = Counter.Next(?step = Microsoft.FSharp.Core.ValueOption.ValueNone) -""")>] -let ``Converting to a struct optional parameter converts optional arguments to value options`` (before: string, after: string) = - Assert.Equal(after, refactoredWith structTitle before "step:") +let ``No action`` (_scenario: string, code: string, marker: string) = + Assert.Empty(actionsAt (moduleM code) marker) [] let ``Escaped parameter name keeps its backticks`` () = let fsharpForm = - """ -module M - -open System.Runtime.InteropServices - -type C() = - static member M(?``type``: int) = defaultArg ``type`` 7 -""" + """type C() = + static member M(?``type``: int) = defaultArg ``type`` 7""" let dotNetForm = - """ -module M - -open System.Runtime.InteropServices - -type C() = - static member M([] ``type``: int) = ``type`` -""" + """type C() = + static member M([] ``type``: int) = ``type``""" let shadowingForm = - """ -module M - -open System.Runtime.InteropServices - -type C() = + """type C() = static member M(?``type``: int) = let ``type`` = defaultArg ``type`` 7 - ``type`` -""" + ``type``""" - Assert.Equal(dotNetForm, refactored fsharpForm "?``type``") - Assert.Equal(shadowingForm, refactored dotNetForm "``type``:") + assertRefactored moduleMWithInterop "?``type``" fsharpForm dotNetForm + assertRefactored moduleMWithInterop "``type``:" dotNetForm shadowingForm [] let ``No action when the file has a signature`` () = let signature = - """ -module M - -type C = + moduleM + """type C = new: unit -> C - static member M: ?x: int -> int -""" + static member M: ?x: int -> int""" let document = RoslynTestHelpers.GetFsiAndFsDocuments signature fsharpForm |> Seq.last @@ -889,6 +571,13 @@ let private inOneProject (definition: string) (callers: string) = AutoAddModules = false } +let private assertCompilesAsProject (definition: string) (callers: string) = + let solution, _ = RoslynTestHelpers.CreateSolution(inOneProject definition callers) + use _ = new TestContext(solution) + + for document in (Seq.exactlyOne solution.Projects).Documents do + assertNoErrors document + [] let ``Converting to a struct optional parameter converts optional arguments in another file`` () = let project = inOneProject counterDefinition counterCallers @@ -933,14 +622,7 @@ let b = Definition.Counter.Next(?step = ValueSome 2) callers ) - // The checker reads the other file of a synthetic project from disk, so the result is checked as a project of its own. - let checkedSolution, _ = - RoslynTestHelpers.CreateSolution(inOneProject definition callers) - - use _ = new TestContext(checkedSolution) - - for document in (Seq.exactlyOne checkedSolution.Projects).Documents do - assertNoErrors document + assertCompilesAsProject definition callers [] [] From eedf26052bf586ff709ab3f6fef53109b1d03399 Mon Sep 17 00:00:00 2001 From: XperiAndri Date: Fri, 30 Oct 2026 00:54:13 +0100 Subject: [PATCH 9/9] Bring the rebased branch to what it had before the rebase Replaying the branch onto main left some files short of the branch's own content; this restores it. Co-Authored-By: Claude Sonnet 5.5 --- docs/release-notes/.VisualStudio/18.vNext.md | 2 -- vsintegration/src/FSharp.Editor/FSharp.Editor.resx | 5 +++++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/release-notes/.VisualStudio/18.vNext.md b/docs/release-notes/.VisualStudio/18.vNext.md index 44946fbf285..aab3bf38155 100644 --- a/docs/release-notes/.VisualStudio/18.vNext.md +++ b/docs/release-notes/.VisualStudio/18.vNext.md @@ -7,8 +7,6 @@ * 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)) * Added a **Tools > Options > F# Tools > Compiler** option, **"Use the .NET SDK F# compiler for builds"**, on by default. Visual Studio uses the .NET SDK F# compiler when the project supplies SDK paths, matching `dotnet build`. Otherwise, it uses the bundled .NET Framework compiler. Turn the option off to use the bundled compiler for all projects. Changes are saved on Apply or OK; Cancel discards unsaved changes. Command-line builds are unaffected. ([Issue #20484](https://github.com/dotnet/fsharp/issues/20484), [PR #20485](https://github.com/dotnet/fsharp/pull/20485)) * F# types, modules, members and values now appear in the GitHub Copilot Chat `#` mention picker, and attach their declaration source as context. ([PR #20409](https://github.com/dotnet/fsharp/pull/20409)) -* Refactoring to switch a member's optional parameter between F# `?x: T` and the .NET-compatible `[] x: T`, moving the default between `defaultArg x c` in the body and the attribute; F# call sites keep working unchanged. -* Refactoring to switch a member's optional parameter between F# `?x: T` and the .NET-compatible `[] x: T`, moving the default between `defaultArg x c` in the body and the attribute; F# call sites keep working unchanged. From F# 10 the .NET form also converts back to `[] ?x: T` with `defaultValueArg x c`. ### Fixed diff --git a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx index 26722fc9e22..37a92777b5c 100644 --- a/vsintegration/src/FSharp.Editor/FSharp.Editor.resx +++ b/vsintegration/src/FSharp.Editor/FSharp.Editor.resx @@ -368,6 +368,11 @@ Use live (unsaved) buffers for analysis Returns: + + {0} in {1} +{2} + Tooltip of an F# declaration in the Copilot Chat "#" picker, as Copilot shows a C# one. {0} is the kind of declaration (Class, Method, Field...), {1} the file name, {2} the declaration's name. + Use [<Optional; DefaultParameterValue>] for optional parameter