improve compiler error message for failed overload resolution (#6596)

* migrate (as a separate copy) fsharpqa tests that are going to be impacted by #6596

the baseline files are empty on purpose, expecting CI failure reporting those tests, I intend to update the baseline and clean up the comments / xml tags from fsharpqa syntax in later commit, and then remove those specific tests altogether from fsharpqa if this is OK with the maintainers.

* update the .bsl files

* remove comments that are handled by baseline files, update baseline files

* remove the migrated tests from fsharpqa tests

* need to be more careful when migrating those

* testing if running the test with .fs instead of .fsx makes them work on .netcore

* exclude migrated fsharpqa from dotnet core run

* sample test in fsharpqa (can't run locally for now)

* trying to make it green now.

* checking if this path is covered by a test, trying to identify how to trigger this other overload related error message

* * [MethodCalls.fs] Defining CallerArgs<'T> in, this replaces passing of callerArgsCount, uncurriedCallerArgs and other variants in the overload resolution logic happening in ConstraintSolver & TypeChecker
* [TypeChecker.fs] pass CallerArgs instace at call sites for overload resolution + some commented code as we'll be building a list of given argument types (probably moving as a CallerArgs method or property)
* [ConstraintSolver.fs/fsi] pipe the overload resolution traced callback to `trace.CollectThenUndoOrCommit` as that expression is long and more important in that context

* bit of refactoring of error message building logic during failed overload resolution

[ConstraintSolver.fsi]

* define OverloadInformation and OverloadResolutionFailure, those replace workflow where `string * ((CalledMeth<_> * exn) list)` values were created at call site mixed with message building and matched later in non obvious fashion in `failOverloading` closure
* adjust UnresolvedOverloading and PossibleOverload exceptions

[ConstraintSolver.fs]

* get rid of `GetPossibleOverloads`, this was used only in `failOverloading` closure, and just being passed the same parameters at call site
* give `failOverloading` a more meaningful signature and consolidate the prefix message building logic there
* fix `failOverloading` call sites

[CompilerOps.fs] adjust pattern matching

Expecting behaviour of this code to be identical as before PR

`

* (buildfix) harmonizing .fsi/.fs right before commit doesn't always work with simple copy paste.

* trying to check what kind of things break loose when I change this

I'm trying to identify why we get `(CalledMeth<Expr> * exn list)`, I'm making a cartesian product through getMethodSlotsAndErrors for now, trying to figure out if we can get other than 0 or 1 exception in the list for a given method slot.

I may revert that one if it doesn't make sense make from a reviewer standpoint and/or breaks fsharpqa tests surounding overload resolution error messages.

* (minor) [ConstraintSolver.fs] revert space mishapps to reduce diff, put back comments where they were

* toward displaying the argument names properly (may fail some fsharpqa tests for now)

* missing Resharper's Ctrl+Alt+V "introduce variable" refactoring @auduchinok :)

* pretty print unresolved overloads without all the type submsumption per overload verbose messages

* Overload resolution error messages: things display like I want in fsi, almost like I want in VS2019, this is for the simple non generic method overload case.

I want to check if user experience changes wrt dotnet/fsharp#2503 and put some time to add tests.

* adjust message for candidates overload

* Hijack the split phase for UnresolvedOverloading, remove the match on PossibleOverload.

It consolidates some more of the string building logic, and it now shows as a single compact and exhaustive error message in VS

Thinking I'll change PossibleOverload to not be an exception if going this way is OK

* updating existing failing baseline files that looks correct to me

* quickfix for update.base.line.with.actuals.fsx so that it skips missing base line files in the loop

* (minor) minimize diff, typos, comments

* fix vsintegration tests affected by overload error message changes

* merge issue unused variable warning

* update the 12 fsharpqa migrated baseline with new error messages

* (minor) baseline update script

[update.base.line.with.actuals.fsx]
* add few directories
* error handling (when tests are running, the .err files are locked

* move System.Convert.ToString and System.Threading.Tasks.Task.Run tests from QA

* * moving 3 fsharpqa tests to new test suite
* bring back neg_System.Convert.ToString.OverloadList.fsx which I mistakenly removed during merge

* consolidate all string building logic in CompileOps.fs, fix remaining cases with missing \n (the end result code is less whack-a-mole-ish)

* update base lines

* remove the migrated tests from fsharpqa

* fix vstest error message

* fix env.lst, removed wrong one...

* update baselines of remaining tests

* adding one simple test with many overloads

* appropriate /// comments on `CalledMeth` constructor arguments

* minimize diff / formatting

* trim unused code

* add simple test, one message is interesting, mentioning parameter count for possible overloads

* comment flaky test for now

* code review on the `coerceExpr` function from @TIHan, we can discard first `isOutArg` argument as the only hardcoded usage was guarded by `error` when the value is true, and passing an explicit false which is now guaranteed equivalent to `callerArg.IsOptional`

* code formatting remarks on ConstraintSolver.fs/fsi

* (minor) formatting

* format known argument type / updating .bsl

* update missing baseline

* update missing baselines

* update missing baseline

* minor: make TTrait better in debugger display

* [wip] pass TraitConstraintInfo around failed overload resolution error, and try to display some of it's info

* minimize diff

* signature file mismatch

* removing duplicate in fscomp.txt

* surfacing initial bits of TraitConstraintInfo, roughly for now

* formatting types of known argument in one go, the formatting is still wrong:

* unamed type arguments aren't relabelled (still show their numerical ids)
* SRTP constraints are dismissed, not sure how they should be lined up to pass to SimplifyTypes.CollectInfo

* rework of overload failure message to prettify *ALL* types in a single go, not only argument types, but return types and known generic argument types (this info was never displayed originally but is useful for scenario similar to FSharpPlus implementation)

added `PrettifyDiscriminantAndTypePairs` in TastOps, this allow to weave extra information with the complete list of types, to help reconstituting the several types from their origin (in the usage spot: argument types, return type, generic parameter types)

* fixup the tests and add two tests

* one checking on the new information of known return type and generic parameter types
* one checking we don't leak internal unammed Typar and that they don't all get named 'a despite being different

* updating baselines that got tighter.

* simplify handling of TraitConstraintInfo

* naming couple of fields and turning a property to a methods as it triggers an assert in debugger when inspecting variables

* comments in the assembling of overload resolution error message

* Add information about which argument doesn't match
Add couple of tests
Updating bunch of baselines

* minor updates to testguide and devguide

* fix PrimitiveConstraints.``Invalid object constructor`` test

* fix(?) salsa tests

* minimize diff

* put back tests under !FSHARP_SUITE_DRIVES_CORECLR_TESTS

* missing updated message

* minor adjustments to TESTGUIDE.md

* return type was missing prettifying in prettyLayoutsOfUnresolvedOverloading

* address code review nits / minimize diff / add comment on PrettifyDiscriminantAndTypePairs

* minimize diff

* proposed work around the flaky error message until dotnet/fsharp#6725 has a fix

we keep the fsharpqa test around (but removing the overload error messages from what is asserted out of it) in the meantime

* fixing baselines of new tests from master

* sisyphus round of baseline update

* removing type which isn't in use and popped back up after rebase

* minimize diff

* tidy inconsistent tuple literal

* * removing TTrait properties that end up not being used
* renaming tys and returnTy fields to better match convention (`tys` is used, so no underscore prefix)
* minimizing diff

* minimize diff

* minimize diff

* minimize diff

* link to usage example in same file

* revert converting CallerArg single cased DU into Record

* minimize diff

* minimize diff

* minimize diff

* fix rebase glitches

* fix rebase glitch

* update baseline

* fix base lines / new tests base lines

* edge case: needs a new line after "A unique overload for method '%s' could not be determined based on type information prior to this program point. A type annotation may be needed." if there are no optional parts.

* updating base line for edge case of missing new line

* missing baseline

* removing comment

Author: smoothdeveloper

src/fsharp/CompileOps.fs

@@ -188,7 +188,6 @@ let GetRangeOfDiagnostic(err: PhasedDiagnostic) =
       | NameClash(_, _, _, m, _, _, _) 
       | UnresolvedOverloading(_, _, _, m) 
       | UnresolvedConversionOperator (_, _, _, m)
-      | PossibleOverload(_, _, _, m) 
       | VirtualAugmentationOnNullValuedType m
       | NonVirtualAugmentationOnNullValuedType m
       | NonRigidTypar(_, _, _, _, _, m)
@@ -403,12 +402,9 @@ let warningOn err level specificWarnOn =
     | 3180 -> false // abImplicitHeapAllocation - off by default
     | _ -> level >= GetWarningLevel err 
 
-let SplitRelatedDiagnostics(err: PhasedDiagnostic) = 
+let SplitRelatedDiagnostics(err: PhasedDiagnostic) : PhasedDiagnostic * PhasedDiagnostic list = 
     let ToPhased e = {Exception=e; Phase = err.Phase}
     let rec SplitRelatedException = function
-      | UnresolvedOverloading(a, overloads, b, c) -> 
-           let related = overloads |> List.map ToPhased
-           UnresolvedOverloading(a, [], b, c)|>ToPhased, related
       | ConstraintSolverRelatedInformation(fopt, m2, e) -> 
           let e, related = SplitRelatedException e
           ConstraintSolverRelatedInformation(fopt, m2, e.Exception)|>ToPhased, related
@@ -452,7 +448,6 @@ let ErrorFromApplyingDefault2E() = DeclareResourceString("ErrorFromApplyingDefau
 let ErrorsFromAddingSubsumptionConstraintE() = DeclareResourceString("ErrorsFromAddingSubsumptionConstraint", "%s%s%s")
 let UpperCaseIdentifierInPatternE() = DeclareResourceString("UpperCaseIdentifierInPattern", "")
 let NotUpperCaseConstructorE() = DeclareResourceString("NotUpperCaseConstructor", "")
-let PossibleOverloadE() = DeclareResourceString("PossibleOverload", "%s%s")
 let FunctionExpectedE() = DeclareResourceString("FunctionExpected", "")
 let BakedInMemberConstraintNameE() = DeclareResourceString("BakedInMemberConstraintName", "%s")
 let BadEventTransformationE() = DeclareResourceString("BadEventTransformation", "")
@@ -770,20 +765,100 @@ let OutputPhasedErrorR (os: StringBuilder) (err: PhasedDiagnostic) (canSuggestNa
           os.Append(e.ContextualErrorMessage) |> ignore
 #endif
 
-      | UnresolvedOverloading(_, _, mtext, _) -> 
-          os.Append mtext |> ignore
+      | UnresolvedOverloading(denv, callerArgs, failure, m) ->
+          
+          // extract eventual information (return type and type parameters)
+          // from ConstraintTraitInfo
+          let knownReturnType, genericParameterTypes =
+              match failure with
+              | NoOverloadsFound (cx=Some cx)
+              | PossibleCandidates (cx=Some cx) -> cx.ReturnType, cx.ArgumentTypes
+              | _ -> None, []
+         
+          // prepare message parts (known arguments, known return type, known generic parameters)
+          let argsMessage, returnType, genericParametersMessage =
+              
+              let retTy =
+                  knownReturnType
+                  |> Option.defaultValue (TType.TType_var (Typar.NewUnlinked()))
+              
+              let argRepr = 
+                  callerArgs.ArgumentNamesAndTypes
+                  |> List.map (fun (name,tTy) -> tTy, {ArgReprInfo.Name = name |> Option.map (fun name -> Ident(name, range.Zero)); ArgReprInfo.Attribs = []})
+                  
+              let argsL,retTyL,genParamTysL = NicePrint.prettyLayoutsOfUnresolvedOverloading denv argRepr retTy genericParameterTypes
+              
+              match callerArgs.ArgumentNamesAndTypes with
+              | [] -> None, Layout.showL retTyL, Layout.showL genParamTysL
+              | items ->
+                  let args = Layout.showL argsL  
+                  let prefixMessage =
+                      match items with
+                      | [_] -> FSComp.SR.csNoOverloadsFoundArgumentsPrefixSingular
+                      | _ -> FSComp.SR.csNoOverloadsFoundArgumentsPrefixPlural
+                  Some (prefixMessage args)
+                  , Layout.showL retTyL
+                  , Layout.showL genParamTysL
+
+          let knownReturnType =
+              match knownReturnType with
+              | None -> None
+              | Some _ -> Some (FSComp.SR.csNoOverloadsFoundReturnType returnType)
+
+          let genericParametersMessage =
+              match genericParameterTypes with
+              | [] -> None
+              | [_] -> Some (FSComp.SR.csNoOverloadsFoundTypeParametersPrefixSingular genericParametersMessage)
+              | _ -> Some (FSComp.SR.csNoOverloadsFoundTypeParametersPrefixPlural genericParametersMessage)
+
+          let overloadMethodInfo displayEnv m (x: OverloadInformation) =
+              let paramInfo =
+                  match x.error with
+                  | :? ArgDoesNotMatchError as x ->
+                      let nameOrOneBasedIndexMessage =
+                          x.calledArg.NameOpt
+                          |> Option.map (fun n -> FSComp.SR.csOverloadCandidateNamedArgumentTypeMismatch n.idText)
+                          |> Option.defaultValue (FSComp.SR.csOverloadCandidateIndexedArgumentTypeMismatch ((snd x.calledArg.Position) + 1))
+                      sprintf " // %s" nameOrOneBasedIndexMessage
+                  | _ -> ""
+                  
+              (NicePrint.stringOfMethInfo x.amap m displayEnv x.methodSlot.Method) + paramInfo
+              
+          let nl = System.Environment.NewLine
+          let formatOverloads (overloads: OverloadInformation list) =
+              overloads
+              |> List.map (overloadMethodInfo denv m)
+              |> List.sort
+              |> List.map FSComp.SR.formatDashItem
+              |> String.concat nl
+         
+          // assemble final message composing the parts
+          let msg =
+              let optionalParts =
+                [knownReturnType; genericParametersMessage; argsMessage]
+                |> List.choose id
+                |> String.concat (nl + nl)
+                |> function | "" -> nl
+                            | result -> nl + nl + result + nl + nl
+              
+              match failure with
+              | NoOverloadsFound (methodName, overloads, _) ->
+                  FSComp.SR.csNoOverloadsFound methodName
+                      + optionalParts                      
+                      + (FSComp.SR.csAvailableOverloads (formatOverloads overloads))
+              | PossibleCandidates (methodName, [], _) ->
+                  FSComp.SR.csMethodIsOverloaded methodName
+              | PossibleCandidates (methodName, overloads, _) ->
+                  FSComp.SR.csMethodIsOverloaded methodName
+                      + optionalParts
+                      + FSComp.SR.csCandidates (formatOverloads overloads)
+          
+          os.Append msg |> ignore
 
       | UnresolvedConversionOperator(denv, fromTy, toTy, _) -> 
           let t1, t2, _tpcs = NicePrint.minimalStringsOfTwoTypes denv fromTy toTy
           os.Append(FSComp.SR.csTypeDoesNotSupportConversion(t1, t2)) |> ignore
 
-      | PossibleOverload(_, minfo, originalError, _) -> 
-          // print original error that describes reason why this overload was rejected
-          let buf = new StringBuilder()
-          OutputExceptionR buf originalError
-
-          os.Append(PossibleOverloadE().Format minfo (buf.ToString())) |> ignore
-
       | FunctionExpected _ ->
           os.Append(FunctionExpectedE().Format) |> ignore