f1 key on type does not query reference documentation.

[cartermp]

Assuming the following code:

let combine strs = System.String.Concat(Array.ofList(strs) : string[])

Hit the f1 key on Concat.

Expected: Browser tab (en-us) open with this link: https://msdn.microsoft.com/en-us/library/0wkb0y3w(v=vs.110).aspx
Actual: This link is opened instead: https://docs.microsoft.com/en-us/visualstudio/ide/creating-solutions-and-projects

[cartermp]

This is the interface which should be implemented: https://github.com/dotnet/roslyn/blob/master/src/VisualStudio/Core/Def/Implementation/F1Help/IHelpContextService.cs

Spoke with @CyrusNajmabadi and he said that string FormatSymbol(ISymbol symbol); can just return null, as it's not important for us.

[dsyme]

@CyrusNajmabadi @cartermp And I presume GetHelpTerm is just the thing returned by this code below?

    /// <summary>Compute the Visual Studio F1-help key identifier for the given location, based on name resolution results</summary>
    ///
    /// <param name="line">The line number where the information is being requested.</param>
    /// <param name="colAtEndOfNames">The column number at the end of the identifiers where the information is being requested.</param>
    /// <param name="lineText">The text of the line where the information is being requested.</param>
    /// <param name="names">The identifiers at the location where the information is being requested.</param>
    member GetF1KeywordAlternate                   : line:int * colAtEndOfNames:int * lineText:string * names:string list -> Async<string option>

Which was previously called in this code:

        override this.GetF1KeywordString(span : TextSpan, context : IVsUserContext) : unit =
            let shouldTryToFindIdentToTheLeft (token : FSharpTokenInfo) =
                match token.CharClass with
                | FSharpTokenCharKind.WhiteSpace -> true
                | FSharpTokenCharKind.Delimiter -> true
                | _ -> false

            let keyword =
                let line = span.iStartLine
                let lineText = VsTextLines.LineText (VsTextView.Buffer view) line                       
                let tokenInformation, col =
                    let col = 
                        if span.iStartIndex = lineText.Length && span.iStartIndex > 0 then
                            // if we are at the end of the line, we always step back one character
                            span.iStartIndex - 1
                        else
                            span.iStartIndex
                    let textColorState = VsTextLines.TextColorState (VsTextView.Buffer view)
                    match colorizer.Value.GetTokenInformationAt(textColorState,line,col) with
                    | Some token as original when col > 0 && shouldTryToFindIdentToTheLeft token ->
                        // try to step back one char
                        match colorizer.Value.GetTokenInformationAt(textColorState,line,col-1) with
                        | Some token as newInfo when token.CharClass <> FSharpTokenCharKind.WhiteSpace -> newInfo, col - 1
                        |   _ -> original, col
                    | otherwise -> otherwise, col

                match tokenInformation with
                |   None -> None
                |   Some token ->
                        match token.CharClass, token.ColorClass with
                        |   FSharpTokenCharKind.Keyword, _
                        |   FSharpTokenCharKind.Operator, _ 
                        |   _, FSharpTokenColorKind.PreprocessorKeyword ->
                                lineText.Substring(token.LeftColumn, token.RightColumn - token.LeftColumn + 1) + "_FS" |> Some
                                
                        |   (FSharpTokenCharKind.Comment|FSharpTokenCharKind.LineComment), _ -> Some "comment_FS"
                                
                        |   FSharpTokenCharKind.Identifier, _ ->            
                                try
                                    let lineText = VsTextLines.LineText (VsTextView.Buffer view) line
                                    let possibleIdentifier = QuickParse.GetCompleteIdentifierIsland false lineText col
                                    match possibleIdentifier with
                                    |   None -> None // no help keyword
                                    |   Some(s,colAtEndOfNames, _) ->
                                            if typedResults.HasFullTypeCheckInfo then 
                                                let qualId = PrettyNaming.GetLongNameFromString s
                                                match typedResults.GetF1KeywordAlternate(Range.Line.fromZ line,colAtEndOfNames, lineText, qualId) |> Async.RunSynchronously with
                                                | Some s -> Some s
                                                | None -> None 
                                            else None                           
                                with e ->
                                    Assert.Exception (e)
                                    reraise()
                        | _ -> None
            match keyword with
            |   Some f1Keyword ->
                    context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_Filter, "devlang", "fsharp") |> ignore
                    // TargetFrameworkMoniker is not set for files that are not part of project (scripts and orphan fs files)
                    if not (String.IsNullOrEmpty projectSite.TargetFrameworkMoniker) then
                        context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_Filter, "TargetFrameworkMoniker", projectSite.TargetFrameworkMoniker) |> ignore
                    context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_LookupF1_CaseSensitive, "keyword", f1Keyword) |> ignore
                    ()
            |   None -> ()

which was in turn used by this code:

        internal virtual int UpdateLanguageContext(LanguageContextHint hint, IVsTextLines buffer, TextSpan[] ptsSelection, IVsUserContext context)
        {
            // From the docs: Any failure code: means the implementer is "passing" on this opportunity to provide context and the text editor will fall back to other mechanisms.
            if (ptsSelection == null || ptsSelection.Length != 1) return NativeMethods.E_FAIL;
            context.RemoveAttribute(null, null);
            TextSpan span = ptsSelection[0];
            IVsTextLines lastActiveBuffer;
            IVsTextView lastAciveView = this.LastActiveTextView;
            if (lastActiveView == null) return NativeMethods.E_FAIL;
            NativeMethods.ThrowOnFailure(lastActiveView.GetBuffer(out lastActiveBuffer));
            if (lastActiveBuffer != buffer) return NativeMethods.E_FAIL;
            ISource source = GetSource(buffer);
            if (source == null) return NativeMethods.E_FAIL;

            var req = source.BeginBackgroundRequest(span.iStartLine, span.iStartIndex, new TokenInfo(), BackgroundRequestReason.FullTypeCheck, lastActiveView, RequireFreshResults.Yes, new BackgroundRequestResultHandler(this.HandleUpdateLanguageContextResponse));

            if (req == null || req.Result == null) return NativeMethods.E_FAIL;

            if ((req.IsSynchronous ||
                    ((req.Result != null) && req.Result.TryWaitForBackgroundRequestCompletion(1000))))
            {
                if (req.IsAborted) return NativeMethods.E_FAIL;
                if (req.ResultIntellisenseInfo != null)
                {
                    req.ResultIntellisenseInfo.GetF1KeywordString(span, context);
                    return NativeMethods.S_OK;
                }
            }
            else // result is asynchronous and have not completed within 1000 ms
            {
                context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_Filter, "devlang", "fsharp");
                context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_LookupF1_CaseSensitive, "keyword", "fsharp.typechecking.incomplete");
                return NativeMethods.S_OK;
            }
            return NativeMethods.E_FAIL;

        }
        internal void HandleUpdateLanguageContextResponse(BackgroundRequest req)
        {
        }

[cartermp]

@dsyme Yep, that's correct.

[rojepp]

That interface is marked internal.

[vasily-kirichenko]

@rojepp that's not a problem, everything there is internal and we use it. I've no idea how it works though.

[rojepp]

@vasily-kirichenko How do you get it to compile? I don't know much about Roslyn I'm afraid.

[vasily-kirichenko]

You cannot compile it? Strange. Make a WIP PR, we'll look at it.

[rojepp]

It seems I can. I was doing something wrong. I'd be happy to do some work on this if no-one else is working on it?

[vasily-kirichenko]

What about me, I'm not going to work on it.

[vasily-kirichenko]

@rojepp pro tip: run build.cmd vs, then install VisualFSharpOpenSource.vsix before using VS 2017, it will make your life a lot easier :)

[rojepp]

To get the latest fixes? Thanks, I'll do that!

[vasily-kirichenko]

Yes. Without it even tooltips don't work.

[dsyme]

@rojepp that's not a problem, everything there is internal and we use it. I've no idea how it works though

FSharp.Editor is listed as InternalsVisibleTo for Roslyn.

[dsyme]

BTW I've pushed a branch "bleeding-edge" which you can use to get the combination of features @vasily-kirichenko has been working on. PRs should of course be based on master.

https://github.com/Microsoft/visualfsharp/tree/bleeding-edge

[vasily-kirichenko]

@dsyme I'm afraid I've stuck with Inline Rename unless somebody from Roslyn/TS team helps.

[dsyme]

@vasily-kirichenko Ah yes :) Bleeding indeed :)

[cartermp]

@rojepp I just have some basic code that implements the interface, but it doesn't seem to be intercepting the f1 key hit. Have you had success getting it to hit a breakpoint? I noticed that the old language service implementation isn't even called anymore, so that isn't overriding this.

namespace Microsoft.VisualStudio.FSharp.Editor

open System
open System.Composition
open System.Collections.Generic
open System.Collections.Immutable
open System.Linq
open System.Threading
open System.Threading.Tasks
open System.Runtime.CompilerServices

open Microsoft.CodeAnalysis
open Microsoft.CodeAnalysis.Classification
open Microsoft.CodeAnalysis.Editor
open Microsoft.CodeAnalysis.Editor.Host
open Microsoft.CodeAnalysis.Navigation
open Microsoft.CodeAnalysis.Editor.Shared.Utilities
open Microsoft.CodeAnalysis.Host.Mef
open Microsoft.CodeAnalysis.Text
open Microsoft.VisualStudio.LanguageServices.Implementation.F1Help

open Microsoft.VisualStudio.FSharp.LanguageService
open Microsoft.VisualStudio.Text
open Microsoft.VisualStudio.Text.Tagging

open Microsoft.FSharp.Compiler.Range
open Microsoft.FSharp.Compiler.SourceCodeServices

[<Shared>]
[<ExportLanguageService(typeof<IHelpContextService>, FSharpCommonConstants.FSharpLanguageName)>]
type internal FSharpF1HelpProvider =

    static member internal GetKeywordAsync(document: Document, textSpan: TextSpan, cancellationToken: CancellationToken) =
        async {
            document |> ignore
            textSpan |> ignore
            cancellationToken |> ignore
            return Some("hello")
        }

    interface IHelpContextService with
        member this.GetHelpTermAsync(document: Document, textSpan: TextSpan, cancellationToken: CancellationToken) = 
            async {
                let! res = FSharpF1HelpProvider.GetKeywordAsync(document, textSpan, cancellationToken)
                match res with
                | Some keyword -> return keyword
                | None -> return String.Empty
            } |> CommonRoslynHelpers.StartAsyncAsTask cancellationToken

        // The current interface has this member which is not applicable to F#.
        // We can just return 'null' here, as it won't affect the behavior for us.
        member this.FormatSymbol(symbol: ISymbol) = symbol |> ignore; null

        // This is what C# does.  See here:
        //
        // https://github.com/dotnet/roslyn/blob/84bc7eea2e05e9d9d7e1b53c2a107c0ac9857a72/src/VisualStudio/CSharp/Impl/LanguageService/CSharpHelpContextService.cs#L23
        member this.Language = "fsharp"
        member this.Product = "fsharp"

[rojepp]

@cartermp I'm having the same issue. My code isn't getting called. I also tried inheriting from AbstractHelpContextService instead of just implementing the interface, like C# and VB does, but it didn't help. Maybe @CyrusNajmabadi could offer some insight? Is there some additional voodoo needed other than

[<Shared>]
[<ExportLanguageService(typeof<IHelpContextService>, FSharpCommonConstants.FSharpLanguageName)>]

?

[vasily-kirichenko]

Check (twice) that Shared attribute is the right one.

[CyrusNajmabadi]

definitely don't derive from the Abtract guys. They're there to share VB/C# logic.

[CyrusNajmabadi]

You'll have to debug through AbstractLanguageService`2.UpdateLanguageContext

You guys do have a derivation of AbstractLanguageService`2 in F# land, right?

[cartermp]

Okay, thanks @CyrusNajmabadi for pointing me in the right direction.

In our subclass of AbstractLanguageService`2 here, we'll need to do something similar to this, which will get the IHelpContextService service, which we implement as above, and then pass the result into the VS API.

[CyrusNajmabadi]

no no... you shouldn't need ot do that. we already do that for you :)

[CyrusNajmabadi]

We need to figure out why it's not working. can you add a BP to that method and see what happens when F1 is hit in an F# scenario?

[rojepp]

@CyrusNajmabadi I don't have Roslyn set up for debugging, but I reimplemented UpdateLanguageContext in our FSharpLanguageService and but a BP in there. The call to document.Project.LanguageServices.GetService<IHelpContextService>() returns null.

    interface IVsLanguageContextProvider with
        member this.UpdateLanguageContext(_dwHint, pBuffer, ptsSelection, pUC) = 
            let textViewAdapter = this.Package.ComponentModel.GetService<IVsEditorAdaptersFactoryService>()
            let textBuffer = textViewAdapter.GetDataBuffer(pBuffer)
            let context =  pUC :?> IVsUserContext
            
            match textBuffer = null, context = null with
            | true,_
            | _,true -> VSConstants.E_UNEXPECTED;
            | _ ->

            let snapshot = textBuffer.CurrentSnapshot
            match snapshot.GetOpenDocumentInCurrentContextWithChanges() with
            | null -> VSConstants.E_FAIL
            | document ->
            match document.Project.LanguageServices.GetService<IHelpContextService>() with
            | null -> VSConstants.E_NOTIMPL
            | helpService ->

            let selectionStart = textBuffer.CurrentSnapshot.GetLineFromLineNumber(ptsSelection.[0].iStartLine).Start + ptsSelection.[0].iStartIndex
            let selectionEnd = textBuffer.CurrentSnapshot.GetLineFromLineNumber(ptsSelection.[0].iEndLine).Start + ptsSelection.[0].iEndIndex
            let _span = Microsoft.CodeAnalysis.Text.TextSpan.FromBounds(unbox selectionStart, unbox selectionEnd)

            // VS help is not cancellable.
            let _cancellationToken = System.Threading.CancellationToken.None;
            let helpTerm = "DummyKeyword" // helpService.GetHelpTermAsync(document, span, cancellationToken).WaitAndGetResult(cancellationToken);

            match String.IsNullOrWhiteSpace(helpTerm) with 
            | true -> VSConstants.S_FALSE
            | _  ->
            context.RemoveAttribute("keyword", null) |> ignore
            context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_Filter, "devlang", helpService.Language) |> ignore
            context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_Filter, "product", helpService.Product) |> ignore
            context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_Filter, "product", "VS") |> ignore
            context.AddAttribute(VSUSERCONTEXTATTRIBUTEUSAGE.VSUC_Usage_LookupF1_CaseSensitive, "keyword", helpTerm) |> ignore

            VSConstants.S_OK

[CyrusNajmabadi]

Is the document correct? does it have the right language?

[rojepp]

It's the right document. It also has document.Project.Language = 'F#'.
I am confused whether IHelpContextService.Language and IHelpContextService.Product should be 'F#' or 'fsharp', though none of them seem to make a difference.

[CyrusNajmabadi]

We'll have to debug to figure out why document.Project.LanguageServices.GetService<IHelpContextService>() is returning null. I don't know enough about F# to know what's going on. But it seems like your service isn't getting exported properly.

[CyrusNajmabadi]

I am confused whether IHelpContextService.Language and IHelpContextService.Product should be 'F#' or 'fsharp', though none of them seem to make a difference.

Those shouldn't matter here, since we're not even getting to that point. That's something to do with how the help system works. Honestly, if you weren't setting those values before, they likely don't matter at all now.

[rojepp]

@vasily-kirichenko System.Composition.SharedAttribute.
@CyrusNajmabadi Thanks for clarifying that

[cartermp]

@OmarTawfik Is there anything special we might need to do to export IHelpContextService aside from having it defined in FSharp.Editor and decorated with [<ExportLanguageService(typeof<IHelpContextService>, FSharpCommonConstants.FSharpLanguageName)>]? That's what other services do, but for some reason this one isn't getting picked up.

[OmarTawfik]

@cartermp first thing I noticed is that you should use FSharpCommonConstants.FSharpLanguageName instead of fsharp. Not sure where the this.Language is used in Roslyn.
Next step, if you cannot get it to be picked up, look for GetService<IHelpContextService>() in Roslyn code base. Put a break point there and you'll find one of two things:

1. The service is being MEF imported, but not used for some reason: easy fix, the Roslyn code should point out why it is not picked up.
2. The service is not being MEF imported: bigger MEF problem, and you'd probably need to use MEF-fu there.

[rojepp]

@OmarTawfik I used FSharpCommonConstants.FSharpLanguageName in the attribute. this.Language is used to send the correct language attribute to MSDN. In previous VS, we added these ourselves here: https://github.com/Microsoft/visualfsharp/blob/0988f1fe19b4136ccf3e54d4fb78cefdb36e001f/vsintegration/src/FSharp.LanguageService/Intellisense.fs#L538

[rojepp]

OK, problem solved at my end! I had open statements for System.Composition and System.ComponentModel.Composition. I removed the latter, and Roslyn picked up the new service. I'll continue working on this tomorrow. Thanks for the input!

[cartermp]

@rojepp Awesome! Looking forward to seeing this working.

Interesting that removing that open statement did the trick.

[CyrusNajmabadi]

Great!

[vasily-kirichenko]

This is the exact problem I've met twice. I was sure it's about the wrong Shared attribute.

[cartermp]

Seems like the kind of thing that a compile error or warning should be generated from.

[cartermp]

Fixed by #1966.