Microsoft.DotNet.Interactive.FSharp: Implement F# signature help and update FsAutoComplete

src/Microsoft.DotNet.Interactive.FSharp.Tests/KernelTests.fs

@@ -1,4 +1,4 @@
-// Copyright (c) .NET Foundation and contributors. All rights reserved.
+// Copyright (c) .NET Foundation and contributors. All rights reserved.
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 namespace Microsoft.DotNet.Interactive.FSharp.Tests
@@ -90,8 +90,8 @@ type KernelTests() =
                 "let a = Math.Sin(10.0)"
             ]
 
-        // Math.Sin(a: float) : float
-        texts.Should().ContainAll(@"static member Sin", "a: float", "-> float")
+        // Math.Sin(a : float) : float
+        texts.Should().ContainAll(@"static member Sin", "a : float", "-> float")
 
     [<Fact>]
     member __.``HoverText for Types``() =
@@ -127,8 +127,8 @@ type KernelTests() =
                 "let a = int 20.0"
             ]
 
-        // val inline int : value:'T -> int (requires member op_Explicit)
-        texts.Should().ContainAll("val inline int:", "^T (requires static member op_Explicit )", "-> int")
+        // val inline int : value : 'T -> int (requires member op_Explicit)
+        texts.Should().ContainAll("val inline int :", "^T (requires static member op_Explicit )", "-> int")
 
     [<Fact>]
     member __.``Get completion list for List module then get the 'average' function and verify it has documentation``() =

src/Microsoft.DotNet.Interactive.FSharp/FSharpKernel.fs

@@ -206,15 +206,14 @@ type FSharpKernel () as this =
 
                 let formattedDiagnostics =
                     fsiDiagnostics
-                    |> Array.map (fun d -> d.ToString())
-                    |> Array.map (fun text -> new FormattedValue(PlainTextFormatter.MimeType, text))
+                    |> Array.map (fun d -> new FormattedValue(PlainTextFormatter.MimeType, d.ToString()))
 
                 context.Publish(DiagnosticsProduced(diagnostics, formattedDiagnostics, codeSubmission))
 
             match result with
             | Ok(result) when not isError ->
                 match result with
-                | Some(value) when value.ReflectionType <> typeof<unit> ->
+                | Some(value) when Type.(<>)(value.ReflectionType, typeof<unit>) ->
                     let resultValue = value.ReflectionValue
                     let formattedValues : IReadOnlyList<FormattedValue> = 
                         match resultValue with
@@ -292,8 +291,17 @@ type FSharpKernel () as this =
                 }
 
             match res.TryGetToolTipEnhanced (mkPos line col) lineContent with
-            | Result.Ok (Some (tip, signature, footer, typeDoc)) ->
+            | Result.Ok (Some result) ->
+                let (tip, signature, footer, typeDoc) =
+                    result.ToolTipText,
+                    result.Signature,
+                    result.Footer,
+                    (match result.SymbolInfo with
+                     | TryGetToolTipEnhancedResult.Symbol sxa -> Some sxa.XmlDocSig
+                     | TryGetToolTipEnhancedResult.Keyword kwd -> None)
+
                 let results =
+
                     FsAutoComplete.TipFormatter.formatTipEnhanced
                         tip signature footer typeDoc
                         FsAutoComplete.TipFormatter.FormatCommentStyle.Legacy
@@ -302,7 +310,8 @@ type FSharpKernel () as this =
                         // make footer look like in Ionide
                         let newFooter =
                             footer.Split([|'\n'|], StringSplitOptions.RemoveEmptyEntries)
-                            |> Seq.map (fun line -> line.TrimEnd('\r'))
+                            |> Seq.map (fun line -> line.Trim())  // Trim both ends to handle any whitespace
+                            |> Seq.filter (String.IsNullOrWhiteSpace >> not)  // Remove empty lines
                             |> Seq.filter (fsiAssemblyRx.IsMatch >> not)
                             |> Seq.map (sprintf "*%s*")
                             |> String.concat "\n\n----\n"
@@ -364,6 +373,162 @@ type FSharpKernel () as this =
                 ()
         }
 
+    let handleRequestSignatureHelp (requestSignatureHelp: RequestSignatureHelp) (context: KernelInvocationContext) =
+        let text = FSharp.Compiler.Text.SourceText.ofString requestSignatureHelp.Code
+
+        // FCS uses 1-based line numbers
+        let line = requestSignatureHelp.LinePosition.Line + 1
+        let col = requestSignatureHelp.LinePosition.Character
+
+        let lineContent = text.GetLineString(line - 1)
+
+        // Find the start of the current token by working backwards from cursor
+        let rec findTokenStart pos =
+            if pos <= 0 || pos > lineContent.Length then 
+                min pos lineContent.Length |> max 0
+            else
+                let c = lineContent.[pos - 1]
+                if Char.IsWhiteSpace(c) || c = '(' then pos
+                else findTokenStart (pos - 1)
+
+        // Skip whitespace and opening parens backwards from a position
+        let rec skipWhitespaceAndParens pos =
+            if pos <= 0 then 0
+            elif Char.IsWhiteSpace(lineContent.[pos - 1]) || lineContent.[pos - 1] = '(' then
+                skipWhitespaceAndParens (pos - 1)
+            else pos
+
+        // Skip whitespace backwards from a position
+        let rec skipWhitespace pos =
+            if pos <= 0 then 0
+            elif Char.IsWhiteSpace(lineContent.[pos - 1]) then skipWhitespace (pos - 1)
+            else pos
+
+        // Find the start position of the function name (first token on the line)
+        let rec findFunctionName currentEnd =
+            if currentEnd <= 0 then 0
+            else
+                let pos = skipWhitespace currentEnd
+                if pos = 0 then 0
+                else
+                    let start = findTokenStart pos
+                    let checkPos = skipWhitespace start
+                    if checkPos > 0 && lineContent.[checkPos - 1] <> '(' then
+                        findFunctionName checkPos
+                    else
+                        start
+
+        // Find the end position of a token starting from a given position
+        let rec findTokenEnd pos =
+            if pos >= lineContent.Length then lineContent.Length
+            elif Char.IsWhiteSpace(lineContent.[pos]) || lineContent.[pos] = '(' then pos
+            else findTokenEnd (pos + 1)
+
+        // Extract function name by finding the first token on the line
+        let tokenEnd = skipWhitespaceAndParens (min col lineContent.Length)
+        let functionStartPos = findFunctionName tokenEnd
+        let functionName =
+            if functionStartPos < lineContent.Length then
+                let endPos = findTokenEnd functionStartPos
+                if endPos > functionStartPos then
+                    lineContent.Substring(functionStartPos, endPos - functionStartPos).Trim()
+                else ""
+            else ""
+
+        if not (String.IsNullOrWhiteSpace functionName) then
+                // Use F# Compiler Services to get signature information
+                // This works for both user-defined functions and BCL types
+                // Add common opens to help FCS resolve BCL types and make it a complete expression
+                let codeWithContext = 
+                    if requestSignatureHelp.Code.Contains "System." then
+                        // Add open statement and make it a complete (but invalid) expression
+                        // by adding a placeholder argument - FCS should still give us signature help
+                        let placeholder = 
+                            if requestSignatureHelp.Code.EndsWith '(' then
+                                "\"\")" // Close with a placeholder string argument
+                            else
+                                ""
+                        sprintf "open System\n%s%s" requestSignatureHelp.Code placeholder
+                    else
+                        requestSignatureHelp.Code
+                let adjustedLine = if requestSignatureHelp.Code.Contains "System." then line + 1 else line
+                let parse, check, _ctx = script.Value.Fsi.ParseAndCheckInteraction codeWithContext
+                let res = FsAutoComplete.ParseAndCheckResults(parse, check, EntityCache())
+
+                match res.TryGetSignatureData (mkPos adjustedLine (functionStartPos + 1)) lineContent with
+                | Ok (returnType, parameterGroups, generics) ->
+                    let functionDisplayName, documentation =
+                        match res.TryGetSymbolUse (mkPos line (functionStartPos + 1)) lineContent with
+                        | Some symbolUse -> 
+                            let displayName = symbolUse.Symbol.DisplayName
+                            let xmlDoc = symbolUse.Symbol.XmlDoc
+                            let doc = FsAutoComplete.TipFormatter.formatDocumentationFromXmlDoc xmlDoc
+                            let docString = 
+                                match doc with
+                                | FsAutoComplete.TipFormatter.TipFormatterResult.Success formatted -> formatted
+                                | _ -> ""
+                            (displayName, docString)
+                        | None -> (functionName, "")
+
+                    let activeParameter = 
+                        let textBeforeCursor = lineContent.Substring(0, min col lineContent.Length)
+                        let openParenIndex = textBeforeCursor.LastIndexOf '('
+                        let closeParenIndex = textBeforeCursor.LastIndexOf ')'
+
+                        if openParenIndex >= 0 && (closeParenIndex < 0 || openParenIndex > closeParenIndex) then
+                            let textInParens = textBeforeCursor.Substring(openParenIndex + 1)
+                            textInParens.Split(',').Length - 1
+                        else
+                            let startPos = min functionStartPos textBeforeCursor.Length
+                            let textAfterFunction = 
+                                if startPos < textBeforeCursor.Length then
+                                    textBeforeCursor.Substring(startPos).Trim()
+                                else
+                                    ""
+                            let tokens = textAfterFunction.Split([|' '; '\t'|], StringSplitOptions.RemoveEmptyEntries)
+                            max 0 (tokens.Length - 1)
+
+                    let ids = parameterGroups |> List.collect id
+                    let totalParams = ids |> List.length
+                    let activeParameter = min activeParameter (totalParams - 1) |> max 0
+
+                    let parameters = 
+                        ids
+                        |> List.map (fun (name, paramType) ->
+                            ParameterInformation(
+                                label = sprintf "%s: %s" name paramType,
+                                documentation = FormattedValue("text/markdown", "")))
+
+                    let paramsFormatted = 
+                        parameterGroups
+                        |> List.map (fun group ->
+                            group
+                            |> List.map (fun (name, paramType) -> sprintf $"{name}: {paramType}")
+                            |> String.concat " * ")
+                        |> String.concat " -> "
+
+                    let genericsFormatted =
+                        if List.isEmpty generics then ""
+                        else sprintf "<%s>" (String.concat ", " generics)
+
+                    let label = 
+                        if String.IsNullOrWhiteSpace paramsFormatted then
+                            sprintf $"{functionDisplayName}{genericsFormatted} : {returnType}"
+                        else
+                            sprintf $"{functionDisplayName}{genericsFormatted} : {paramsFormatted} -> {returnType}"
+
+                    let signature = SignatureInformation(
+                        label = label,
+                        documentation = FormattedValue("text/markdown", documentation),
+                        parameters = parameters)
+
+                    context.Publish(SignatureHelpProduced(requestSignatureHelp, [signature], activeSignatureIndex = 0, activeParameterIndex = activeParameter))
+                    Task.FromResult ()
+                | Error _ ->
+                    Task.FromResult ()
+        else
+            Task.FromResult ()
+
     let handleRequestDiagnostics (requestDiagnostics: RequestDiagnostics) (context: KernelInvocationContext) =
         task {
             let _parseResults, checkFileResults, _checkProjectResults = script.Value.Fsi.ParseAndCheckInteraction(requestDiagnostics.Code)
@@ -451,6 +616,9 @@ type FSharpKernel () as this =
     interface IKernelCommandHandler<RequestHoverText> with
         member this.HandleAsync(command: RequestHoverText, context: KernelInvocationContext) = handleRequestHoverText command context
 
+    interface IKernelCommandHandler<RequestSignatureHelp> with
+        member this.HandleAsync(command: RequestSignatureHelp, context: KernelInvocationContext) = handleRequestSignatureHelp command context
+
     interface IKernelCommandHandler<RequestValueInfos> with
         member this.HandleAsync(command: RequestValueInfos, context: KernelInvocationContext) = handleRequestValueValueInfos command context
 

src/Microsoft.DotNet.Interactive.FSharp/FSharpScriptHelpers.fs

@@ -12,7 +12,7 @@ open FSharp.Compiler.Interactive.Shell
 open FSharp.Compiler.Diagnostics
 open FSharp.Compiler.EditorServices
 
-[<RequireQualifiedAccess>]
+[<RequireQualifiedAccess; Struct>]
 type LangVersion =
     | V47
     | V50