Add a refactoring between F# and .NET-compatible optional parameters - #20547
xperiandri wants to merge 4 commits into
Conversation
…meters ?x: T is optional for F# callers and gives the body an option; [<Optional; DefaultParameterValue(c)>] 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) <noreply@anthropic.com>
✅ Release notes checked
|
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…form [<Optional; DefaultParameterValue(c)>] x: T now converts back to either ?x: T with let x = defaultArg x c, or, from F# 10, [<Struct>] ?x: T with let x = defaultValueArg x c. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…e of its section Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
🔍 Tooling Safety Check — Affects-Design-Time
|
Covered casesConverts
Not offered
|
T-Gro
left a comment
There was a problem hiding this comment.
🤖 🕵️ AI review — verify independently.
| let defaultText = | ||
| match defaultValue with | ||
| | ValueSome value -> sourceText.ToString(spanOf sourceText value.Range) | ||
| | ValueNone -> "Unchecked.defaultof<_>" |
There was a problem hiding this comment.
🤖 🕵️ [P2] Converting x to F# ?x changes C.M() from true to false — an omitted obj receives null instead of Missing.Value.
open System.Runtime.InteropServices
type C() =
static member M([<Optional>] x: obj) =
obj.ReferenceEquals(x, System.Reflection.Missing.Value)
printfn "%b" (C.M())| | 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 |
There was a problem hiding this comment.
🤖 🕵️ [P2] Converting ?x to .NET optional changes C.M() from 99 to 0 — matching defaultArg by spelling removes a call to a different function.
let defaultArg (_: int option) (_: int) = 99
type C() =
static member M(?x: int) = defaultArg x 0
printfn "%d" (C.M())| match found, attributeText with | ||
| | ValueSome found, ValueSome attributeText -> | ||
| [ | ||
| TextChange(TextSpan((spanOf sourceText optionalValRange).Start, 1), attributeText) |
There was a problem hiding this comment.
🤖 🕵️ [P2] Converting only ?y to .NET optional produces FS1212 — the remaining F# optional ?x precedes the converted parameter.
type C() =
static member M(?x: int, ?y: int) =
defaultArg x 1 + defaultArg y 2|
|
||
| let struct (prefix, defaultFunction) = | ||
| if asStruct then | ||
| struct ("[<Struct>] ?", "defaultValueArg") |
There was a problem hiding this comment.
🤖 🕵️ [P2] Converting x to [<Struct>] ?x leaves a formerly valid caller failing with FS0001 — Some 1 supplies option, but the parameter now requires voption.
open System.Runtime.InteropServices
type C =
static member M([<Optional; DefaultParameterValue(0)>] x: int) = x
let result = C.M(?x = Some 1)| /// 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 |
There was a problem hiding this comment.
🤖 🕵️ [P2] Converting ?x to .NET optional deletes let mutable x, leaving x <- x + 1 failing with FS0027 because the parameter is immutable.
type C() =
static member M(?x: int) =
let mutable x = defaultArg x 0
x <- x + 1
x| found | ||
| || match node with | ||
| | SyntaxNode.SynModule(SynModuleDecl.Open(target = SynOpenDeclTarget.ModuleOrNamespace(longId = SynLongIdent(id = ids)))) -> | ||
| (ids |> List.map _.idText) = interopServices |
There was a problem hiding this comment.
🤖 🕵️ [P2] Converting ?x to .NET optional produces FS0039 for both attributes — an open in another module suppresses the import needed at C.M.
module Other =
open System.Runtime.InteropServices
let x = 1
type C() =
static member M(?x: int) = defaultArg x 0
Offers
Ctrl+.to switch between?x: TwithdefaultArg x cin the body and[<Optional; DefaultParameterValue(c)>] x: T, which C# and VB callers can omit too. Converting back offers?xwithdefaultArgand, from F# 10,[<Struct>] ?xwithdefaultValueArg. F# call sites compile against both forms, so only the member is edited.Not offered unless every use is
defaultArg x cwith one constant of the parameter's type, or for overrides, abstract slots, constructors, or a file with a paired.fsi.🤖 Generated with Claude Code