From 2c176f9862d76ada23afa2eaa7f909eda99912d7 Mon Sep 17 00:00:00 2001 From: Eugene Auduchinok Date: Fri, 9 Oct 2026 13:22:56 +0200 Subject: [PATCH] Checker: resolve named arguments of failed and unfinished method calls Record the named arguments and property setters of a method application for the language service even when overload resolution fails, using the overload that assigns the most of them. Treat a bare identifier after named arguments as a named argument still being written. Co-Authored-By: Claude Fable 5.1 --- .../.FSharp.Compiler.Service/11.0.200.md | 1 + .../Checking/Expressions/CheckExpressions.fs | 76 +++++--- src/Compiler/Checking/MethodCalls.fs | 2 +- src/Compiler/Checking/MethodCalls.fsi | 1 - tests/FSharp.Compiler.Service.Tests/Common.fs | 7 + .../TypeChecker/TypeCheckerRecoveryTests.fs | 183 ++++++++++++++++++ 6 files changed, 245 insertions(+), 25 deletions(-) diff --git a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md index 4cd1275d7d6..1b6421abcbc 100644 --- a/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md +++ b/docs/release-notes/.FSharp.Compiler.Service/11.0.200.md @@ -6,6 +6,7 @@ * Capture diagnostics emitted when transparent project checking finishes a project, including FS0247 for namespace-module name collisions. ([Issue #6036](https://github.com/dotnet/fsharp/issues/6036), [PR #20706](https://github.com/dotnet/fsharp/pull/20706)) * Split Linux CoreCLR CI into two parallel test batches to reduce test turnaround time. ([PR #20587](https://github.com/dotnet/fsharp/pull/20587)) +* Checker: resolve named arguments of failed and unfinished method calls ([PR #20749](https://github.com/dotnet/fsharp/pull/20749)) * Fix `NativePtr.stackalloc` used inside the `with` handler, filter or `finally` block of a `try` expression producing an assembly that throws `InvalidProgramException` at method load. Direct or mandatory-inline allocations are now reported at compile time as error FS3924; calls to non-inline allocation helpers, including handler-local helpers, remain legal and retain their method boundary during inlining and let-binding elimination. ([Issue #20295](https://github.com/dotnet/fsharp/issues/20295), [PR #20491](https://github.com/dotnet/fsharp/pull/20491)) * `FsiEvaluationSession.EvalInteraction` and `EvalInteractionNonThrowing` evaluate every `;;`-separated interaction in the text they are given, as their documentation has always said, instead of silently discarding everything after the first. ([PR #20396](https://github.com/dotnet/fsharp/pull/20396)) * Prevent `InvalidProgramException` when runtime async awaits a `Task` upcast to `Task`, including through `ConfigureAwait`. ([Issue #20686](https://github.com/dotnet/fsharp/issues/20686), [PR #20726](https://github.com/dotnet/fsharp/pull/20726)) diff --git a/src/Compiler/Checking/Expressions/CheckExpressions.fs b/src/Compiler/Checking/Expressions/CheckExpressions.fs index 405e0300c85..5d404eb26eb 100644 --- a/src/Compiler/Checking/Expressions/CheckExpressions.fs +++ b/src/Compiler/Checking/Expressions/CheckExpressions.fs @@ -3305,11 +3305,56 @@ let GetMethodArgs arg = // do not abort overload resolution in case if named arguments are mixed with errors match argExpr with | SynExpr.ArbitraryAfterError _ -> None + | SynExpr.Ident id -> + errorR (Error(FSComp.SR.tcNameArgumentsMustAppearLast(), argExpr.Range)) + Some(false, id, SynExpr.ArbitraryAfterError("namedArgWithoutValue", id.idRange.EndRange)) | _ -> error(Error(FSComp.SR.tcNameArgumentsMustAppearLast(), argExpr.Range)) | namedArg -> namedArg) unnamedCallerArgs, namedCallerArgs +let RecordNamedArgsOfMethodApplication (cenv: cenv) (env: TcEnv) (ad: AccessorDomain) (calledMeth: CalledMeth<'T>) = + let container = ArgumentContainer.Method calledMeth.Method + + for argSet in calledMeth.ArgSets do + for assignedArg in argSet.AssignedNamedArgs do + match assignedArg.NamedArgIdOpt with + | None -> () + | Some id -> + let calledArgId = defaultArg assignedArg.CalledArg.NameOpt id + let item = Item.OtherName (Some calledArgId, assignedArg.CalledArg.CalledArgumentType, None, Some container, calledArgId.idRange) + CallNameResolutionSink cenv.tcSink (id.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, ad) + +let RecordFailedMethodApplication (cenv: cenv) (env: TcEnv) (ad: AccessorDomain) afterResolution (calledMethGroup: CalledMeth<'T> list) = + match cenv.tcSink.CurrentSink, calledMethGroup with + | None, _ -> () + | Some _, [] -> + match afterResolution with + | AfterResolution.RecordResolution(_, _, _, onFailure) -> onFailure() + | AfterResolution.DoNothing -> () + | Some _, _ -> + let calledMeth = + calledMethGroup + |> List.maxBy (fun calledMeth -> + calledMeth.TotalNumUnnamedCalledArgs >= calledMeth.TotalNumUnnamedCallerArgs, + calledMeth.TotalNumAssignedNamedArgs + calledMeth.NumAssignedProps) + + match afterResolution with + | AfterResolution.RecordResolution(_, _, callSink, _) -> callSink (calledMeth.Method, calledMeth.AssociatedPropertyInfo, calledMeth.CalledTyparInst) + | AfterResolution.DoNothing -> () + + RecordNamedArgsOfMethodApplication cenv env ad calledMeth + + // TcSetterArgExpr does not run for an unresolved call + for AssignedItemSetter(id, setter, _) in calledMeth.AssignedItemSetters do + let defnItem = + match setter with + | AssignedPropSetter (_, pinfo, _, _) -> Item.Property (pinfo.PropertyName, [pinfo], None) + | AssignedILFieldSetter finfo -> Item.ILField finfo + | AssignedRecdFieldSetter rfinfo -> Item.RecdField rfinfo + + CallNameResolutionSink cenv.tcSink (id.idRange, env.NameEnv, Item.SetterArg (id, defnItem), emptyTyparInst, ItemOccurrence.Use, ad) + let NotNullIfNotNullParamNames g (minfo: MethInfo) = match minfo with | ILMeth(ilMethInfo = ilminfo) when ilminfo.RawMetadata.Return.CustomAttrsStored.HasWellKnownAttribute (g, WellKnownILAttributes.NotNullIfNotNullAttribute) -> @@ -10771,19 +10816,16 @@ and TcMethodApplication let unnamedCurriedCallerArgs, namedCurriedCallerArgs, lambdaVars, returnTy, tpenv = TcMethodApplication_CheckArguments cenv env exprTy curriedCallerArgsOpt candidates preArgumentTypeCheckingCalledMethGroup callerObjArgTys ad mMethExpr mItem tpenv - let preArgumentTypeCheckingCalledMethGroup = - preArgumentTypeCheckingCalledMethGroup |> List.map (fun cmeth -> (cmeth.Method, cmeth.CalledTyArgs, cmeth.AssociatedPropertyInfo, cmeth.UsesParamArrayConversion)) - let uniquelyResolved = match uniquelyResolved with - | ErrorResult _ -> - match afterResolution with - | AfterResolution.DoNothing -> () - | AfterResolution.RecordResolution(_, _, _, onFailure) -> onFailure() + | ErrorResult _ -> RecordFailedMethodApplication cenv env ad afterResolution preArgumentTypeCheckingCalledMethGroup | _ -> () uniquelyResolved |> CommitOperationResult + let preArgumentTypeCheckingCalledMethGroup = + preArgumentTypeCheckingCalledMethGroup |> List.map (fun cmeth -> (cmeth.Method, cmeth.CalledTyArgs, cmeth.AssociatedPropertyInfo, cmeth.UsesParamArrayConversion)) + // STEP 3. Resolve overloading /// Select the called method that's the result of overload resolution let finalCalledMeth = @@ -10870,8 +10912,8 @@ and TcMethodApplication | AfterResolution.RecordResolution(_, _, callSink, _), Some result -> (result.Method, result.AssociatedPropertyInfo, result.CalledTyparInst) |> callSink - | AfterResolution.RecordResolution(_, _, _, onFailure), None -> - onFailure() + | AfterResolution.RecordResolution _, None -> + RecordFailedMethodApplication cenv env ad afterResolution postArgumentTypeCheckingCalledMethGroup // Raise the errors from the constraint solving RaiseOperationResult errors @@ -10910,23 +10952,11 @@ and TcMethodApplication /// STEP 5. Build the argument list. Adjust for optional arguments, byref arguments and coercions. - let objArgPreBinder, objArgs, allArgsPreBinders, allArgs, allArgsCoerced, optArgPreBinder, paramArrayPreBinders, outArgExprs, outArgTmpBinds = + let objArgPreBinder, objArgs, allArgsPreBinders, allArgsCoerced, optArgPreBinder, paramArrayPreBinders, outArgExprs, outArgTmpBinds = let tcVal = LightweightTcValForUsingInBuildMethodCall g env.TraitContext AdjustCallerArgs tcVal TcFieldInit env.eCallerMemberName cenv.infoReader ad finalCalledMeth objArgs lambdaVars mItem mMethExpr - // Record the resolution of the named argument for the Language Service - allArgs |> List.iter (fun assignedArg -> - match assignedArg.NamedArgIdOpt with - | None -> () - | Some id -> - let idOpt = Some (defaultArg assignedArg.CalledArg.NameOpt id) - let m = - match assignedArg.CalledArg.NameOpt with - | Some id -> id.idRange - | None -> id.idRange - let container = ArgumentContainer.Method finalCalledMethInfo - let item = Item.OtherName (idOpt, assignedArg.CalledArg.CalledArgumentType, None, Some container, m) - CallNameResolutionSink cenv.tcSink (id.idRange, env.NameEnv, item, emptyTyparInst, ItemOccurrence.Use, ad)) + RecordNamedArgsOfMethodApplication cenv env ad finalCalledMeth /// STEP 6. Build the call expression, then adjust for byref-returns, out-parameters-as-tuples, post-hoc property assignments, methods-as-first-class-value, diff --git a/src/Compiler/Checking/MethodCalls.fs b/src/Compiler/Checking/MethodCalls.fs index 9377d28aa1f..9ffd105d797 100644 --- a/src/Compiler/Checking/MethodCalls.fs +++ b/src/Compiler/Checking/MethodCalls.fs @@ -1830,7 +1830,7 @@ let AdjustCallerArgs tcVal tcFieldInit eCallerMemberName (infoReader: InfoReader AdjustCallerArgExpr tcVal g amap infoReader ad isOutArg calledArgTy reflArgInfo callerArgTy m e) |> List.unzip - objArgPreBinder, objArgs, allArgsPreBinders, allArgs, allArgsCoerced, optArgPreBinder, paramArrayPreBinders, outArgExprs, outArgTmpBinds + objArgPreBinder, objArgs, allArgsPreBinders, allArgsCoerced, optArgPreBinder, paramArrayPreBinders, outArgExprs, outArgTmpBinds //------------------------------------------------------------------------- diff --git a/src/Compiler/Checking/MethodCalls.fsi b/src/Compiler/Checking/MethodCalls.fsi index 70c4930b7bb..6c4157a143f 100644 --- a/src/Compiler/Checking/MethodCalls.fsi +++ b/src/Compiler/Checking/MethodCalls.fsi @@ -450,7 +450,6 @@ val AdjustCallerArgs: (Expr -> Expr) * Expr list * 'b option list * - AssignedCalledArg list * Expr list * (Expr -> Expr) * 'c option list * diff --git a/tests/FSharp.Compiler.Service.Tests/Common.fs b/tests/FSharp.Compiler.Service.Tests/Common.fs index 04e88750f97..a5b148c4d91 100644 --- a/tests/FSharp.Compiler.Service.Tests/Common.fs +++ b/tests/FSharp.Compiler.Service.Tests/Common.fs @@ -401,6 +401,13 @@ let inline dumpDiagnosticNumbers (results: FSharpCheckFileResults) = |> Array.map (fun e -> e.Range.ToString(), e.ErrorNumber) |> List.ofArray +let dumpSymbolUses line (results: FSharpCheckFileResults) = + results.GetAllUsesOfAllSymbolsInFile() + |> Seq.filter (fun su -> su.Range.StartLine = line && not su.IsFromDefinition) + |> Seq.sortBy _.Range.StartColumn + |> Seq.map (fun su -> su.Range.ToString(), string su.Symbol) + |> List.ofSeq + let getSymbolUses (results: FSharpCheckFileResults) = results.GetAllUsesOfAllSymbolsInFile() diff --git a/tests/FSharp.Compiler.Service.Tests/TypeChecker/TypeCheckerRecoveryTests.fs b/tests/FSharp.Compiler.Service.Tests/TypeChecker/TypeCheckerRecoveryTests.fs index 03658cd74e6..716f5e88c30 100644 --- a/tests/FSharp.Compiler.Service.Tests/TypeChecker/TypeCheckerRecoveryTests.fs +++ b/tests/FSharp.Compiler.Service.Tests/TypeChecker/TypeCheckerRecoveryTests.fs @@ -63,6 +63,189 @@ Math.Max(a,b,) assertHasSymbolUsages ["Max"] checkResults +module NamedArgs = + let private types = """ +[] +module Types + +type T(?a: int, ?b: string) = + member _.M(a: int, b: string) = () + member _.M(a: int, b: string, c: float) = () + member _.N(a: int, b: string) = () + member _.N(a: string, b: int) = () + member _.O(?a: int, ?b: string) = () + member _.R(a: int, b: string) = () + member _.R(x: int, y: int, z: int, b: string, c: float) = () + member val A = 0 with get, set + member val B = "" with get, set + +let x = T() +""" + + let private check source = + let options = createProjectOptionsFromNamedSources [ "Types.fs", types; "Test.fs", source ] [ "--target:exe" ] + let filePath = options.SourceFiles |> Array.find (fun path -> path.EndsWith "Test.fs") + let _, checkResults = parseAndCheckFile filePath source options + checkResults + + let private checkAtCaret markedSource = + let context = Checker.getResolveContext markedSource + let checkResults = check context.SourceContext.Source + checkResults.GetSymbolUsesAtLocation(context.Pos.Line, context.Pos.Column, context.SourceContext.LineText, context.Names) + |> List.exactlyOne + + [] + let ``Named arg 01 - Unfinished`` () = + check "x.M(a = 1, b =)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + "(1,11--1,12)", "parameter b" + ] + + [] + let ``Named arg 02 - Unfinished, EOF`` () = + check "x.M(a = 1, b =" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + "(1,11--1,12)", "parameter b" + ] + + [] + let ``Named arg 03 - Missing required arg`` () = + check "x.M(a =)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + ] + + let symbolUse = checkAtCaret "x.M{caret}(a =)" + let m = symbolUse.Symbol :?> FSharpMemberOrFunctionOrValue + m.CurriedParameterGroups |> Seq.concat |> Seq.map _.DisplayName |> List.ofSeq |> shouldEqual [ "a"; "b" ] + + [] + let ``Named arg 04 - Unknown`` () = + check "x.M(a = 1, z = 2)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + ] + + [] + let ``Named arg 05 - Overload by assigned names`` () = + check "x.M(a = 1, c = 2)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + "(1,11--1,12)", "parameter c" + ] + + let symbolUse = checkAtCaret "x.M{caret}(a = 1, c = 2)" + let m = symbolUse.Symbol :?> FSharpMemberOrFunctionOrValue + m.CurriedParameterGroups |> Seq.concat |> Seq.map _.DisplayName |> List.ofSeq |> shouldEqual [ "a"; "b"; "c" ] + + [] + let ``Named arg 06 - Ident, first arg`` () = + check "x.M(a)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + ] + + [] + let ``Named arg 07 - Ident, first arg, value in scope`` () = + check "let a = 1 in x.O(a)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,13--1,14)", "val x" + "(1,13--1,16)", "member O" + "(1,17--1,18)", "val a" + ] + + [] + let ``Named arg 08 - Ident after named`` () = + let checkResults = check "x.M(a = 1, b)" + + dumpDiagnosticNumbers checkResults |> shouldEqual [ + "(1,11--1,12)", 691 + ] + + dumpSymbolUses 1 checkResults |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + "(1,11--1,12)", "parameter b" + ] + + [] + let ``Named arg 09 - Ident after named, unknown`` () = + let checkResults = check "x.M(a = 1, z)" + + dumpDiagnosticNumbers checkResults |> shouldEqual [ + "(1,11--1,12)", 691 + "(1,0--1,13)", 505 + ] + + dumpSymbolUses 1 checkResults |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member M" + "(1,4--1,5)", "parameter a" + ] + + [] + let ``Named arg 10 - Lambda mismatch`` () = + check "x.O(a = 1, b = fun () -> \"\")" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member O" + "(1,4--1,5)", "parameter a" + "(1,11--1,12)", "parameter b" + ] + + [] + let ``Named arg 11 - No overload`` () = + check "x.N(a = 1.0, b = 1.0)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member N" + "(1,4--1,5)", "parameter a" + "(1,13--1,14)", "parameter b" + ] + + [] + let ``Named arg 12 - Positional fit`` () = + check "x.R(1, 2, 3, b =)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,0--1,1)", "val x" + "(1,0--1,3)", "member R" + "(1,13--1,14)", "parameter b" + ] + + let symbolUse = checkAtCaret "x.R{caret}(1, 2, 3, b =)" + let m = symbolUse.Symbol :?> FSharpMemberOrFunctionOrValue + m.CurriedParameterGroups |> Seq.concat |> Seq.map _.DisplayName |> List.ofSeq |> shouldEqual [ "x"; "y"; "z"; "b"; "c" ] + + [] + let ``Setter 01 - Unfinished`` () = + check "new T(a = 1, B =)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,4--1,5)", "T" + "(1,4--1,5)", "member .ctor" + "(1,6--1,7)", "parameter a" + "(1,13--1,14)", "property B" + ] + + [] + let ``Setter 02 - Ident after named`` () = + check "new T(A = 1, B)" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,4--1,5)", "T" + "(1,4--1,5)", "member .ctor" + "(1,6--1,7)", "property A" + "(1,13--1,14)", "property B" + ] + + [] + let ``Setter 03 - No overload`` () = + check "new T(z = 1, B = \"\")" |> dumpSymbolUses 1 |> shouldEqual [ + "(1,4--1,5)", "T" + "(1,4--1,5)", "member .ctor" + "(1,13--1,14)", "property B" + ] + module Constraints = [] let ``Type 01`` () =