fsharp / fsharp-compiler-docs

Doc build for FSharp.Compiler.Service
https://fsharp.github.io/fsharp-compiler-docs
MIT License
279 stars 123 forks source link

GetToolTipText gets the same XML signature for several member overloads #800

Closed nightroman closed 5 years ago

nightroman commented 7 years ago

Repro steps

UPDATE: see the adjusted to the newer API in FSCS (still with the bug) is here: https://github.com/fsharp/FSharp.Compiler.Service/issues/800#issuecomment-334652039

#r @"..\packages\FSharp.Compiler.Service\lib\net45\FSharp.Compiler.Service.dll"
open System
open Microsoft.FSharp.Compiler.SourceCodeServices

let checker = FSharpChecker.Create()

let input = """
open System.IO
File.WriteAllText ("", "")
"""
let inputLines = input.Split('\n')
let file = "/Test.fsx"

let projOptions, errors = checker.GetProjectOptionsFromScript(file, input) |> Async.RunSynchronously
let parseFileResults = checker.ParseFileInProject(file, input, projOptions) |> Async.RunSynchronously
let checkFileAnswer = checker.CheckFileInProject(parseFileResults, file, 0, input, projOptions) |> Async.RunSynchronously
let checkFileResults =
    match checkFileAnswer with
    | FSharpCheckFileAnswer.Succeeded(res) -> res
    | res -> failwithf "Parsing did not finish... (%A)" res

let (FSharpToolTipText tips) =
    checkFileResults.GetToolTipText(3, 18, inputLines.[2], ["File"; "WriteAllText"], FSharpTokenTag.Identifier) |> Async.RunSynchronously

tips
|> List.iter (function
    | FSharpToolTipElement.Group items ->
        for item in items do
            printfn "MainDescription: %s" item.MainDescription
            printfn "XmlDoc: %A" item.XmlDoc
    | _ -> ()
)

Expected behavior

For two overloads of WriteAllText, GetToolTipText gets two items with different MainDescription and different XML doc signatures.

Actual behavior

GetToolTipText gets two items with different MainDescription (expected) and the same XML doc signatures (unexpected). See the output below, the second XML doc signature is expected to be different, with System.Text.Encoding.

Output:

    MainDescription: File.WriteAllText(path: string, contents: string) : unit
    XmlDoc: XmlDocFileSignature
      ("C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.2\mscorlib.dll",
       "M:System.IO.File.WriteAllText(System.String,System.String)")

    MainDescription: File.WriteAllText(path: string, contents: string, encoding: System.Text.Encoding) : unit
    XmlDoc: XmlDocFileSignature
      ("C:\Program Files (x86)\Reference Assemblies\Microsoft\Framework\.NETFramework\v4.6.2\mscorlib.dll",
       "M:System.IO.File.WriteAllText(System.String,System.String)")

MainDescription correctly provides different signatures of two overloads. But XmlDoc.Item2 provides the same XML doc signature for both entries.

Related information

vasily-kirichenko commented 7 years ago

I cannot reproduce it in VFT current master:

image

image

vasily-kirichenko commented 7 years ago

It works in Ionide, too:

image

image

nightroman commented 7 years ago

But are these the same as the case? I am talking about a problem in my app using FCS. It is reduced to the code provided. This code is not working as expected.

vasily-kirichenko commented 7 years ago

Look at VFT or Ionide source code to see if you use the api properly.

vasily-kirichenko commented 7 years ago

For Ionide, it's around here https://github.com/ionide/FsAutoComplete/blob/master/src/FsAutoComplete.Core/Commands.fs#L210 You can build FSAC in debug mode, put it into the Ionide plugin folder, open your scrips in Code, attach to the FSAC Suave process and debug.

/cc @k_cieslak

vasily-kirichenko commented 7 years ago

Ahhhhhhhhhhhhhh. Sorry, I didn't notice that the problem is not in the XML doc comment (which is entirely missing, BTW), but in the 'M:System.IO.File.WriteAllText(System.String,System.String)' part.

vasily-kirichenko commented 7 years ago

However, the "M:..." part is right in VFT:

First overload:

image

Second overload:

image

vasily-kirichenko commented 7 years ago

The above screenshots are taken on this line https://github.com/Microsoft/visualfsharp/blob/master/vsintegration/src/FSharp.Editor/QuickInfo/QuickInfoProvider.fs#L111

nightroman commented 7 years ago

I am on holiday now, will check in a week or two. If my code is wrong, could you suggest what wrong is?

nightroman commented 7 years ago

I am back. Thank you for the links. They are useful, especially the second with a different method GetStructuredToolTipText (that means though that it cannot be used as cannot-repro example). I will investigate the new ways that the links provide.

But I am still interested in the original case working correctly. It gets all overloads in one go, exactly what I need (in my app I am not limited in space for displaying "tooltip", I rather show some mini-help).

If I use the API correctly (I used some testing code from FCS for inspiration) then the result is apparently wrong, XML signature should be different for different overloads.

Thus, the issue still looks valid. (Meanwhile, I'll play with the new alternative GetStructuredToolTipText, thank you once again).

nightroman commented 7 years ago

I played with both suggestions.

(1) FSAC - The suggested code calls GetToolTipText in the same way as I do. But this code does not show how the result is processed, so it does not help. In fact, I am not sure that vscode-ionide uses this method. For example if we leave just File.WriteAllText in the sample code then vscode shows

    The member or object constructor 'WriteAllText' does not take 1 argument(s).
    An overload was found taking 2 arguments

though the sample code still gets info for two existing overloads, one with the correct info, another with some incorrect XML signature.

(2) The suggested code uses a different function GetStructuredToolTipText. I tried it. It gets the result as some pretty heavy and not documented data structures presumably designed for other not documented functions. It looks too difficult to go there without documentation or clear samples.

All in all, the only known somehow documented way is here. This is what I use. This gets not quite right result. Hence the issue.

nightroman commented 7 years ago

The original example uses ParseFileInProject which is deprecated in the latest. Here is the updated code without it. The issue is there, too.

#r @"..\packages\FSharp.Compiler.Service\lib\net45\FSharp.Compiler.Service.dll"
open System
open Microsoft.FSharp.Compiler.SourceCodeServices

let checker = FSharpChecker.Create()

let input = """
open System.IO
File.WriteAllText ("", "")
"""
let inputLines = input.Split('\n')
let file = "/Test.fsx"

let projOptions, errors = checker.GetProjectOptionsFromScript(file, input) |> Async.RunSynchronously
let parseResults, checkAnswer = checker.ParseAndCheckFileInProject(file, 0, input, projOptions) |> Async.RunSynchronously
let checkFileResults =
    match checkAnswer with
    | FSharpCheckFileAnswer.Succeeded(res) -> res
    | res -> failwithf "Parsing did not finish... (%A)" res

let (FSharpToolTipText tips) =
    checkFileResults.GetToolTipText(3, 18, inputLines.[2], ["File"; "WriteAllText"], FSharpTokenTag.Identifier) |> Async.RunSynchronously

tips
|> List.iter (function
    | FSharpToolTipElement.Group items ->
        for item in items do
            printfn "MainDescription: %s" item.MainDescription
            printfn "XmlDoc: %A" item.XmlDoc
    | _ -> ()
)
nightroman commented 5 years ago

Is there any chance this issue is addressed in some "reasonable" future? I am starting to think of workarounds but still hope the fix might come.

baronfel commented 5 years ago

Just to verify, but have you tested with the latest release of FSharp.Compiler.Service? Version 26.0.1 was released recently and contains many fixes from upstream, which may have incidentally fixed this issue.

nightroman commented 5 years ago

Yes, I did. I use FSharp.Compiler.Service a lot, stay updated, and check this issue with every update :(

vasily-kirichenko commented 5 years ago

I've just run your script, the output is:

MainDescription: File.WriteAllText(path: string, contents: string) : unit
XmlDoc: XmlDocFileSignature
  ("/Library/Frameworks/Mono.framework/Versions/5.10.1/lib/mono/4.5/mscorlib.dll",
   "M:System.IO.File.WriteAllText(System.String,System.String)")
MainDescription: File.WriteAllText(path: string, contents: string, encoding: System.Text.Encoding) : unit
XmlDoc: XmlDocFileSignature
  ("/Library/Frameworks/Mono.framework/Versions/5.10.1/lib/mono/4.5/mscorlib.dll",
   "M:System.IO.File.WriteAllText(System.String,System.String)")

Is it still wrong?

nightroman commented 5 years ago

Yes, it is wrong. The XML signature "M:System.IO.File.WriteAllText(System.String,System.String)") is the same for different overloads. As a result I cannot query proper info from XML afterwards.

P.S. I would live with this bug "fine". But I am working on FSharpFar and rather worry about its users.

vasily-kirichenko commented 5 years ago

I see.

I think the problem is at https://github.com/Microsoft/visualfsharp/blob/54c22cdea6e069db28d8dd16723e402e364aab44/src/fsharp/symbols/SymbolHelpers.fs#L622 where the first overload is always got (minfo :: _). However, it's not clear how to pick a right minfo there.

nightroman commented 5 years ago

Maybe the problem is upstream because m seems to be correct (we get different MainDescription for different entries, they come from the same XML) and d is not correct (which contains incorrect minfo for m). (This is my naive understanding).

baronfel commented 5 years ago

The issue is definitely upstream, and it's around where @vasily-kirichenko pointed out. The tooltip resolution of a member in a method group(ie a member with overloads) needs to take into account which specific overload (the m parameter) and use that to find the matching MethInfo instead of using the first one, from what I can tell. I'd suggest raising this as an issue in https://github.com/microsoft/visualfsharp/issues and linking to this one, because any fix will go there first.

nightroman commented 5 years ago

I created the issue. Thank you All for your attention and some good progress.

nightroman commented 5 years ago

The bug was fixed in visualfsharp. Looking forward to the fix merged into FCS as well :)

nightroman commented 5 years ago

All works fine in FCS 28.0, closing.