"System.Func overload-is-better": checking if anything breaks, or not (in which case, we'd like a test)

src/fsharp/CompilerDiagnostics.fs

@@ -195,16 +195,17 @@ let GetRangeOfDiagnostic(err: PhasedDiagnostic) =
       | DeprecatedCommandLineOptionNoDescription(_, m)
       | InternalCommandLineOption(_, m)
       | HashIncludeNotAllowedInNonScript m
-      | HashReferenceNotAllowedInNonScript m
-      | HashDirectiveNotAllowedInNonScript m
-      | FileNameNotResolved(_, _, m)
-      | LoadedSourceNotFoundIgnoring(_, m)
-      | MSBuildReferenceResolutionWarning(_, _, m)
-      | MSBuildReferenceResolutionError(_, _, m)
-      | AssemblyNotResolved(_, m)
-      | HashLoadedSourceHasIssues(_, _, m)
-      | HashLoadedScriptConsideredSource m ->
-          Some m
+      | HashReferenceNotAllowedInNonScript m 
+      | HashDirectiveNotAllowedInNonScript m  
+      | FileNameNotResolved(_, _, m) 
+      | LoadedSourceNotFoundIgnoring(_, m) 
+      | MSBuildReferenceResolutionWarning(_, _, m) 
+      | MSBuildReferenceResolutionError(_, _, m) 
+      | AssemblyNotResolved(_, m) 
+      | HashLoadedSourceHasIssues(_, _, m) 
+      | HashLoadedScriptConsideredSource m
+      | FuncIsBetterOverloadResolutionDeprecation(m=m) ->
+        Some m
       // Strip TargetInvocationException wrappers
       | :? System.Reflection.TargetInvocationException as e ->
           RangeFromException e.InnerException
@@ -1378,6 +1379,10 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNa
           let errorText = FunctionValueUnexpectedE().Format (NicePrint.stringOfTy denv ty)
           os.Append errorText |> ignore
 
+      | FuncIsBetterOverloadResolutionDeprecation (method=method) ->
+          let _,errorText = FSComp.SR.funcIsBetterOverloadResolutionDeprecation(method.DisplayName)
+          os.Append errorText |> ignore
+
       | UnitTypeExpected (denv, ty, _) ->
           let ty, _cxs = PrettyTypes.PrettifyType denv.g ty
           let warningText = UnitTypeExpectedE().Format (NicePrint.stringOfTy denv ty)

src/fsharp/ConstraintSolver.fs

@@ -234,6 +234,8 @@ exception UnresolvedOverloading of displayEnv: DisplayEnv * callerArgs: CallerAr
 
 exception UnresolvedConversionOperator of displayEnv: DisplayEnv * TType * TType * range
 
+exception FuncIsBetterOverloadResolutionDeprecation of displayEnv: DisplayEnv * method: MethInfo * m:range
+
 type TcValF = (ValRef -> ValUseFlag -> TType list -> range -> Expr * TType)
 
 type ConstraintSolverState = 
@@ -2643,9 +2645,10 @@ and ResolveOverloading
                 /// Compare types under the feasibly-subsumes ordering
                 let compareTypes ty1 ty2 = 
                     (ty1, ty2) ||> compareCond (fun x1 x2 -> TypeFeasiblySubsumesType ndeep csenv.g csenv.amap m x2 CanCoerce x1) 
-
+
+                let mutable funcIsBetterSpecialCasingArgPosition = ValueNone
                 /// Compare arguments under the feasibly-subsumes ordering and the adhoc Func-is-better-than-other-delegates rule
-                let compareArg (calledArg1: CalledArg) (calledArg2: CalledArg) =
+                let compareArg argIndex (calledArg1: CalledArg) (calledArg2: CalledArg) =
                     let c = compareTypes calledArg1.CalledArgumentType calledArg2.CalledArgumentType
                     if c <> 0 then c else
 
@@ -2658,8 +2661,9 @@ and ResolveOverloading
                                 tcref1.DisplayName = "Func" &&  
                                 (match tcref1.PublicPath with Some p -> p.EnclosingPath = [| "System" |] | _ -> false) && 
                                 isDelegateTy g ty1 &&
-                                isDelegateTy g ty2 -> true
-
+                                isDelegateTy g ty2 ->
+                                  funcIsBetterSpecialCasingArgPosition <- ValueSome (argIndex,calledArg1)
+                                  true
                             // T is always better than inref<T>
                             | _ when isInByrefTy csenv.g ty2 && typeEquiv csenv.g ty1 (destByrefTy csenv.g ty2) -> 
                                 true
@@ -2722,7 +2726,7 @@ and ResolveOverloading
                                        []
                                 else 
                                     []) @
-                               ((candidate.AllUnnamedCalledArgs, other.AllUnnamedCalledArgs) ||> List.map2 compareArg) 
+                               ((candidate.AllUnnamedCalledArgs, other.AllUnnamedCalledArgs) ||> List.mapi2 compareArg) 
                            // "all args are at least as good, and one argument is actually better"
                            if cs |> List.forall (fun x -> x >= 0) && cs |> List.exists (fun x -> x > 0) then 
                                1
@@ -2755,15 +2759,15 @@ and ResolveOverloading
 
                     // F# 5.0 rule - prior to F# 5.0 named arguments (on the caller side) were not being taken 
                     // into account when comparing overloads.  So adding a name to an argument might mean 
-                    // overloads ould no longer be distinguished.  We thus look at *all* arguments (whether
+                    // overloads could no longer be distinguished.  We thus look at *all* arguments (whether
                     // optional or not) as an additional comparison technique.
                     let c = 
                         if g.langVersion.SupportsFeature(Features.LanguageFeature.NullableOptionalInterop) then
                             let cs = 
                                 let args1 = candidate.AllCalledArgs |> List.concat
                                 let args2 = other.AllCalledArgs |> List.concat
                                 if args1.Length = args2.Length then 
-                                    (args1, args2) ||> List.map2 compareArg
+                                    (args1, args2) ||> List.mapi2 compareArg
                                 else
                                     []
                             // "all args are at least as good, and one argument is actually better"
@@ -2794,7 +2798,18 @@ and ResolveOverloading
                         else 
                            None) 
                 match bestMethods with 
-                | [(calledMeth, warns, t)] -> Some calledMeth, OkResult (warns, ()), WithTrace t
+                | [(calledMeth, warns, t)] -> 
+                  let calledMeth = calledMeth
+                  match funcIsBetterSpecialCasingArgPosition with
+                  | ValueNone -> ()
+                  | ValueSome (argIndex, _calledArg) ->
+                    let callerArg = calledMeth.GetCallerArgAt argIndex
+                    match callerArg.Expr with 
+                    | Expr.Lambda _ ->
+                      // https://github.com/dotnet/fsharp/issues/11534
+                      warning (FuncIsBetterOverloadResolutionDeprecation(denv,calledMeth.Method,m))
+                    | _ -> ()
+                  Some calledMeth, OkResult (warns, ()), WithTrace t
                 | bestMethods -> 
                     let methods = 
                         let getMethodSlotsAndErrors methodSlot errors =

src/fsharp/ConstraintSolver.fsi

@@ -121,6 +121,7 @@ exception ErrorsFromAddingSubsumptionConstraint of tcGlobals: TcGlobals * displa
 exception ErrorFromAddingConstraint             of displayEnv: DisplayEnv * exn * range
 exception UnresolvedConversionOperator          of displayEnv: DisplayEnv * TType * TType * range
 exception UnresolvedOverloading                 of displayEnv: DisplayEnv * callerArgs: CallerArgs<Expr> * failure: OverloadResolutionFailure * range
+exception FuncIsBetterOverloadResolutionDeprecation of displayEnv:DisplayEnv * method: MethInfo * m:range
 exception NonRigidTypar                         of displayEnv: DisplayEnv * string option * range * TType * TType * range
 
 exception ArgDoesNotMatchError                  of error: ErrorsFromAddingSubsumptionConstraint * calledMeth: CalledMeth<Expr> * calledArg: CalledArg * callerArg: CallerArg<Expr>

src/fsharp/FSComp.txt

@@ -1563,4 +1563,5 @@ forFormatInvalidForInterpolated4,"Interpolated strings used as type IFormattable
 3390,xmlDocMissingParameter,"This XML comment is incomplete: no documentation for parameter '%s'"
 3391,tcLiteralAttributeCannotUseActivePattern,"A [<Literal>] declaration cannot use an active pattern for its identifier"
 3392,containerDeprecated,"The 'AssemblyKeyNameAttribute' has been deprecated. Use 'AssemblyKeyFileAttribute' instead."
-3393,containerSigningUnsupportedOnThisPlatform,"Key container signing is not supported on this platform."
+3393,containerSigningUnsupportedOnThisPlatform,"Key container signing is not supported on this platform."
+3394,funcIsBetterOverloadResolutionDeprecation,"The method '%s' has overload with 'System.Func' delegate in the signature, to fix the warning, you need to specify the type explicitly."

src/fsharp/MethodCalls.fs

@@ -508,6 +508,29 @@ type CalledMeth<'T>
     /// The argument analysis for each set of curried arguments
     member x.ArgSets = argSets
 
+    member x.GetCallerArgAt index =
+      // Arrays are better...
+      let mutable currentIndex = 0
+      let mutable result = ValueNone
+      for argSet in argSets do
+        for arg in argSet.UnnamedCallerArgs do
+          if currentIndex = index then
+            result <- ValueSome arg
+          currentIndex <- currentIndex + 1
+        if ValueOption.isSome result then () else
+        for arg in argSet.AssignedNamedArgs do
+          if currentIndex = index then
+            result <- ValueSome arg.CallerArg
+          currentIndex <- currentIndex + 1
+        if ValueOption.isSome result then () else
+        for arg in argSet.ParamArrayCallerArgs do
+          if currentIndex = index then
+            result <- ValueSome arg
+          currentIndex <- currentIndex + 1
+
+      match result with
+      | ValueNone -> failwithf $"{nameof CalledMeth}.{nameof x.GetCallerArgAt}: wrong index requested %A{argSets}"
+      | ValueSome callerArg -> callerArg
     /// The return type after implicit deference of byref returns is taken into account
     member x.CalledReturnTypeAfterByrefDeref = 
         let retTy = methodRetTy

src/fsharp/MethodCalls.fsi

@@ -213,6 +213,9 @@ type CalledMeth<'T> =
     member amap: ImportMap
     member infoReader: InfoReader
 
+    /// Get the CallerArg at index
+    member GetCallerArgAt: index: int -> CallerArg<'T>
+
 val NamesOfCalledArgs: calledArgs:CalledArg list -> Ident list
 
 type ArgumentAnalysis =

tests/fsharp/tests.fs

@@ -3064,6 +3064,9 @@ module OverloadResolution =
         let [<Test>] ``neg_System.Threading.Tasks.Task.Run.OverloadList``() = singleNegTest (testConfig "typecheck/overloads") "neg_System.Threading.Tasks.Task.Run.OverloadList"
         let [<Test>] ``neg_System.Drawing.Graphics.DrawRectangleOverloadList.fsx``() = singleNegTest (testConfig "typecheck/overloads") "neg_System.Drawing.Graphics.DrawRectangleOverloadList"
 
+    module ``ad hoc code overload error messages or not``=
+        let [<Test>] ``neg_at_somepoint_in_the_future_func-is-better-or-not.fsx`` () = singleNegTest (testConfig "typecheck/overloads") "neg_at_somepoint_in_the_future_func-is-better-or-not"
+
     module ``ad hoc code overload error messages``=
         let [<Test>] ``neg_many_many_overloads`` () = singleNegTest (testConfig "typecheck/overloads") "neg_many_many_overloads"
         let [<Test>] ``neg_interface_generics`` () = singleNegTest (testConfig "typecheck/overloads") "neg_interface_generics"

tests/fsharp/typecheck/overloads/neg_at_somepoint_in_the_future_func-is-better-or-not.bsl

@@ -0,0 +1,16 @@
+
+neg_at_somepoint_in_the_future_func-is-better-or-not.fsx(14,1,14,17): typecheck error FS0193: The method 'M' has overload with 'System.Func' delegate in the signature, to fix the warning, you need to specify the type explicitly.
+
+neg_at_somepoint_in_the_future_func-is-better-or-not.fsx(16,1,16,19): typecheck error FS0193: The method 'M2' has overload with 'System.Func' delegate in the signature, to fix the warning, you need to specify the type explicitly.
+
+neg_at_somepoint_in_the_future_func-is-better-or-not.fsx(25,1,25,37): typecheck error FS0041: A unique overload for method 'M3' could not be determined based on type information prior to this program point. A type annotation may be needed.
+
+Known types of arguments: (unit -> unit) * (unit -> unit)
+
+Candidates:
+ - member C.M3 : [<ParamArray>] funcs:Action array -> unit
+ - member C.M3 : [<ParamArray>] funcs:Func<unit,unit> array -> unit
+
+neg_at_somepoint_in_the_future_func-is-better-or-not.fsx(38,5,38,63): typecheck error FS0193: The method 'Min' has overload with 'System.Func' delegate in the signature, to fix the warning, you need to specify the type explicitly.
+
+neg_at_somepoint_in_the_future_func-is-better-or-not.fsx(40,5,40,74): typecheck error FS0193: The method 'Min' has overload with 'System.Func' delegate in the signature, to fix the warning, you need to specify the type explicitly.

tests/fsharp/typecheck/overloads/neg_at_somepoint_in_the_future_func-is-better-or-not.fsx

@@ -0,0 +1,41 @@
+open System
+
+type C() =
+    member _.M(a: Action<int>) = printfn "M(action)"
+    member _.M(f: Func<int, unit>) = printfn "M(func)"
+    member _.M2(f: Func<unit, unit>) = printfn "M2(func())"
+    member _.M2(f: Action) = printfn "M2(action())"
+    member _.M3([<ParamArray>] funcs: Func<unit,unit> array) = printfn "M3(funcs)"
+    member _.M3([<ParamArray>] funcs: Action array) = printfn "M3(actions)"
+let c = C()
+
+// https://github.com/dotnet/fsharp/issues/11534
+// we may want a warning here, or maybe an error proper at some point
+c.M(fun _ -> ())
+
+c.M2(fun () -> ())
+
+c.M3(Action(fun () -> ()), (fun () -> ()))
+c.M3(Action id, id, id, id)
+c.M3(Func<_,_> id, id, id, id)
+c.M3(Func<_,_>(fun () -> ()), (fun () -> ()))
+c.M3((fun () -> ()), Func<_,_>(fun () -> ()))
+
+// this one doesn't resolve, although without the param array, it would
+c.M3((fun () -> ()), (fun () -> ()))
+
+
+open System.Collections.Generic
+open System.Linq
+
+// here we verify different style of calling BCL overloaded methods
+type Query =
+  static member MinBy_ExplicitDelegate<'T, 'Q, 'Key when 'Key: equality and 'Key: comparison> (source: IEnumerable<'T>, valueSelector: 'T -> 'Key) =
+    Enumerable.Min(source, Func<'T, 'Key>(valueSelector))
+  static member MinBy_ExplicitDelegateWithArgName<'T, 'Q, 'Key when 'Key: equality and 'Key: comparison> (source: IEnumerable<'T>, valueSelector: 'T -> 'Key) =
+    Enumerable.Min(source, selector = Func<'T, 'Key>(valueSelector))
+  static member MinBy_Lambda<'T, 'Q, 'Key when 'Key: equality and 'Key: comparison> (source: IEnumerable<'T>, valueSelector: 'T -> 'Key) =
+    Enumerable.Min(source, (fun (t: 'T) -> (valueSelector t)))
+  static member MinBy_LambdaWithArgName<'T, 'Q, 'Key when 'Key: equality and 'Key: comparison> (source: IEnumerable<'T>, valueSelector: 'T -> 'Key) =
+    Enumerable.Min(source, selector = (fun (t: 'T) -> (valueSelector t)))
+