Show keyword descriptions in completion + refactor (dotnet#9164)

* Completion provider refactor

* Actually show keyword descriptions in tooltips for completion

* Get rid of unnecessary property

* Make the keyword completion list static again

* Just do it in the CompletionProvider

Author: cartermp

Completion/CompletionProvider.fs

@@ -10,18 +10,22 @@ open System.Threading.Tasks
 
 open Microsoft.CodeAnalysis
 open Microsoft.CodeAnalysis.Completion
+open Microsoft.CodeAnalysis.Options
 open Microsoft.CodeAnalysis.Text
+open Microsoft.CodeAnalysis.ExternalAccess.FSharp.Completion
+
+open Microsoft.VisualStudio.Shell
 
 open FSharp.Compiler
 open FSharp.Compiler.Range
 open FSharp.Compiler.SourceCodeServices
-open Microsoft.VisualStudio.Text.Adornments
-open Microsoft.VisualStudio.Text.Editor
+
 module Logger = Microsoft.VisualStudio.FSharp.Editor.Logger
 
 type internal FSharpCompletionProvider
     (
         workspace: Workspace,
+        serviceProvider: SVsServiceProvider,
         checkerProvider: FSharpCheckerProvider,
         projectInfoManager: FSharpProjectOptionsManager,
         assemblyContentProvider: AssemblyContentProvider
@@ -37,20 +41,28 @@ type internal FSharpCompletionProvider
     static let [<Literal>] FullNamePropName = "FullName"
     static let [<Literal>] IsExtensionMemberPropName = "IsExtensionMember"
     static let [<Literal>] NamespaceToOpenPropName = "NamespaceToOpen"
-    static let [<Literal>] IsKeywordPropName = "IsKeyword"
     static let [<Literal>] IndexPropName = "Index"
+    static let [<Literal>] KeywordDescription = "KeywordDescription"
 
     static let keywordCompletionItems =
         Keywords.KeywordsWithDescription
         |> List.filter (fun (keyword, _) -> not (PrettyNaming.IsOperatorName keyword))
         |> List.sortBy (fun (keyword, _) -> keyword)
-
+        |> List.mapi (fun n (keyword, description) ->
+            FSharpCommonCompletionItem.Create(
+                displayText = keyword,
+                displayTextSuffix = "",
+                rules = CompletionItemRules.Default,
+                glyph = Nullable Glyph.Keyword,
+                sortText = sprintf "%06d" (1000000 + n))
+                .AddProperty(KeywordDescription, description))
 
     let checker = checkerProvider.Checker
-
+    
     let settings: EditorOptions = workspace.Services.GetService()
 
-    let documentationBuilder = XmlDocumentation.Provider()   
+    let documentationBuilder = XmlDocumentation.CreateDocumentationBuilder(serviceProvider.XMLMemberIndexService)
+        
     static let noCommitOnSpaceRules = 
         // These are important.  They make sure we don't _commit_ autocompletion when people don't expect them to.  Some examples:
         //
@@ -72,16 +84,16 @@ type internal FSharpCompletionProvider
 
     static let mruItems = Dictionary<(* Item.FullName *) string, (* hints *) int>()
 
-    static member ShouldTriggerCompletionAux(sourceText: SourceText, caretPosition: int, trigger: Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionTrigger, getInfo: (unit -> DocumentId * string * string list), intelliSenseOptions: IntelliSenseOptions) =
+    static member ShouldTriggerCompletionAux(sourceText: SourceText, caretPosition: int, trigger: CompletionTriggerKind, getInfo: (unit -> DocumentId * string * string list), intelliSenseOptions: IntelliSenseOptions) =
         if caretPosition = 0 then
             false
         else
             let triggerPosition = caretPosition - 1
-            let triggerChar = trigger.Character
+            let triggerChar = sourceText.[triggerPosition]
 
-            if trigger.Reason = Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionTriggerReason.Deletion && intelliSenseOptions.ShowAfterCharIsDeleted then
-                Char.IsLetterOrDigit(triggerChar) || triggerChar = '.'
-            elif not (trigger.Reason = Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionTriggerReason.Insertion || trigger.Reason = Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionTriggerReason.InvokeAndCommitIfUnique) then
+            if trigger = CompletionTriggerKind.Deletion && intelliSenseOptions.ShowAfterCharIsDeleted then
+                Char.IsLetterOrDigit(sourceText.[triggerPosition]) || triggerChar = '.'
+            elif not (trigger = CompletionTriggerKind.Insertion) then
                 false
             else
                 // Do not trigger completion if it's not single dot, i.e. range expression
@@ -90,10 +102,10 @@ type internal FSharpCompletionProvider
                 else
                     let documentId, filePath, defines = getInfo()
                     CompletionUtils.shouldProvideCompletion(documentId, filePath, defines, sourceText, triggerPosition) &&
-                    (trigger.Reason = Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionTriggerReason.InvokeAndCommitIfUnique || triggerChar = '.' || (intelliSenseOptions.ShowAfterCharIsTyped && CompletionUtils.isStartingNewWord(sourceText, triggerPosition)))
+                    (triggerChar = '.' || (intelliSenseOptions.ShowAfterCharIsTyped && CompletionUtils.isStartingNewWord(sourceText, triggerPosition)))
 
 
-    static member ProvideCompletionsAsyncAux(completionSource: Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.IAsyncCompletionSource , checker: FSharpChecker, sourceText: SourceText, caretPosition: int, options: FSharpProjectOptions, filePath: string, 
+    static member ProvideCompletionsAsyncAux(checker: FSharpChecker, sourceText: SourceText, caretPosition: int, options: FSharpProjectOptions, filePath: string, 
                                              textVersionHash: int, getAllSymbols: FSharpCheckFileResults -> AssemblySymbol list, languageServicePerformanceOptions: LanguageServicePerformanceOptions, intellisenseOptions: IntelliSenseOptions) = 
         asyncMaybe {
             let! parseResults, _, checkFileResults = checker.ParseAndCheckDocument(filePath, textVersionHash, sourceText, options, languageServicePerformanceOptions, userOpName = userOpName)
@@ -112,7 +124,7 @@ type internal FSharpCompletionProvider
 
             let! declarations = checkFileResults.GetDeclarationListInfo(Some(parseResults), fcsCaretLineNumber, caretLine.ToString(), 
                                                                         partialName, getAllSymbols, userOpName=userOpName) |> liftAsync
-            let results = List<Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionItem>()
+            let results = List<Completion.CompletionItem>()
 
             declarationItems <-
                 declarations.Items
@@ -131,7 +143,6 @@ type internal FSharpCompletionProvider
 
             declarationItems |> Array.iteri (fun number declarationItem ->
                 let glyph = Tokenizer.FSharpGlyphToRoslynGlyph (declarationItem.Glyph, declarationItem.Accessibility)
-                let image = GlyphHelper.getImageId glyph |> ImageElement
                 let name =
                     match declarationItem.NamespaceToOpen with
                     | Some namespaceToOpen -> sprintf "%s (open %s)" declarationItem.Name namespaceToOpen
@@ -145,38 +156,38 @@ type internal FSharpCompletionProvider
                     // We are passing last part of long ident as FilterText.
                     | _, idents -> Array.last idents
 
-                let completionItem =
-                    let item = new Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionItem(name, completionSource, icon = image)
-                    item.Properties.AddProperty(IndexPropName, declarationItem)
-                    item
-
+                let completionItem = 
+                    FSharpCommonCompletionItem.Create(name, null, rules = getRules intellisenseOptions.ShowAfterCharIsTyped, glyph = Nullable glyph, filterText = filterText)
+                                        .AddProperty(FullNamePropName, declarationItem.FullName)
+                        
                 let completionItem =
                     match declarationItem.Kind with
                     | CompletionItemKind.Method (isExtension = true) ->
-                            completionItem//.AddProperty(IsExtensionMemberPropName, "")
+                            completionItem.AddProperty(IsExtensionMemberPropName, "")
                     | _ -> completionItem
 
                 let completionItem =
                     if name <> declarationItem.NameInCode then
-                        completionItem//.AddProperty(NameInCodePropName, declarationItem.NameInCode)
+                        completionItem.AddProperty(NameInCodePropName, declarationItem.NameInCode)
                     else completionItem
 
                 let completionItem =
                     match declarationItem.NamespaceToOpen with
-                    | Some ns -> completionItem//.AddProperty(NamespaceToOpenPropName, ns)
+                    | Some ns -> completionItem.AddProperty(NamespaceToOpenPropName, ns)
                     | None -> completionItem
 
-                let completionItem = completionItem//.AddProperty(IndexPropName, string number)
+                let completionItem = completionItem.AddProperty(IndexPropName, string number)
 
                 let priority = 
                     match mruItems.TryGetValue declarationItem.FullName with
                     | true, hints -> maxHints - hints
                     | _ -> number + maxHints + 1
 
                 let sortText = priority.ToString("D6")
-                let completionItem = completionItem//.WithSortText(sortText)
+                let completionItem = completionItem.WithSortText(sortText)
                 results.Add(completionItem))
 
+            
             if results.Count > 0 && not declarations.IsForType && not declarations.IsError && List.isEmpty partialName.QualifyingIdents then
                 let lineStr = textLines.[caretLinePos.Line].ToString()
 
@@ -185,28 +196,46 @@ type internal FSharpCompletionProvider
                     |> Option.bind (fun parseTree ->
                          UntypedParseImpl.TryGetCompletionContext(Pos.fromZ caretLinePos.Line caretLinePos.Character, parseTree, lineStr))
 
-                let image = GlyphHelper.getImageId Glyph.Keyword |> ImageElement
-
                 match completionContext with
-                | None ->
-                    let keywordItemsWithSource =
-                        keywordCompletionItems
-                        |> Seq.mapi (fun n (keyword, description) ->
-                                new Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionItem
-                                        (keyword, completionSource, image, ImmutableArray<Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionFilter>.Empty, "", keyword, sprintf "%06d" (1000000 + n), keyword, keyword, ImmutableArray<ImageElement>.Empty ))
-
-                    results.AddRange(keywordItemsWithSource)
+                | None -> results.AddRange(keywordCompletionItems)
                 | _ -> ()
-
+            
             return results
         }
 
-    override this.ProvideCompletionsAsync(context: Completion.CompletionContext) =
+    override _.ShouldTriggerCompletion(sourceText: SourceText, caretPosition: int, trigger: CompletionTrigger, _: OptionSet) =
+        use _logBlock = Logger.LogBlock LogEditorFunctionId.Completion_ShouldTrigger
+
+        let getInfo() = 
+            let documentId = workspace.GetDocumentIdInCurrentContext(sourceText.Container)
+            let document = workspace.CurrentSolution.GetDocument(documentId)
+            let defines = projectInfoManager.GetCompilationDefinesForEditingDocument(document)
+            (documentId, document.FilePath, defines)
+
+        FSharpCompletionProvider.ShouldTriggerCompletionAux(sourceText, caretPosition, trigger.Kind, getInfo, settings.IntelliSense)
+        
+    override _.ProvideCompletionsAsync(context: Completion.CompletionContext) =
         asyncMaybe {
-            context.AddItems([])//results)
+            use _logBlock = Logger.LogBlockMessage context.Document.Name LogEditorFunctionId.Completion_ProvideCompletionsAsync
+
+            let document = context.Document
+            let! sourceText = context.Document.GetTextAsync(context.CancellationToken)
+            let defines = projectInfoManager.GetCompilationDefinesForEditingDocument(document)
+            do! Option.guard (CompletionUtils.shouldProvideCompletion(document.Id, document.FilePath, defines, sourceText, context.Position))
+            let! _parsingOptions, projectOptions = projectInfoManager.TryGetOptionsForEditingDocumentOrProject(document, context.CancellationToken)
+            let! textVersion = context.Document.GetTextVersionAsync(context.CancellationToken)
+            let getAllSymbols(fileCheckResults: FSharpCheckFileResults) =
+                if settings.IntelliSense.IncludeSymbolsFromUnopenedNamespacesOrModules
+                then assemblyContentProvider.GetAllEntitiesInProjectAndReferencedAssemblies(fileCheckResults)
+                else []
+            let! results = 
+                FSharpCompletionProvider.ProvideCompletionsAsyncAux(checker, sourceText, context.Position, projectOptions, document.FilePath,
+                                                                    textVersion.GetHashCode(), getAllSymbols, settings.LanguageServicePerformance, settings.IntelliSense)
+            
+            context.AddItems(results)
         } |> Async.Ignore |> RoslynHelpers.StartAsyncUnitAsTask context.CancellationToken
 
-    override this.GetDescriptionAsync(document: Document, completionItem: Completion.CompletionItem, cancellationToken: CancellationToken): Task<CompletionDescription> =
+    override _.GetDescriptionAsync(document: Document, completionItem: Completion.CompletionItem, cancellationToken: CancellationToken): Task<CompletionDescription> =
         async {
             use _logBlock = Logger.LogBlockMessage document.Name LogEditorFunctionId.Completion_GetDescriptionAsync
             match completionItem.Properties.TryGetValue IndexPropName with
@@ -220,27 +249,18 @@ type internal FSharpCompletionProvider
                     // mix main description and xmldoc by using one collector
                     XmlDocumentation.BuildDataTipText(documentationBuilder, collector, collector, collector, collector, collector, description) 
                     return CompletionDescription.Create(documentation.ToImmutableArray())
-                else 
+                else
+                    return CompletionDescription.Empty
+            | _ ->
+                // Try keyword descriptions if they exist
+                match completionItem.Properties.TryGetValue KeywordDescription with
+                | true, keywordDescription ->
+                    return CompletionDescription.FromText(keywordDescription)
+                | false, _ ->
                     return CompletionDescription.Empty
-            | _ -> 
-                return CompletionDescription.Empty
-        } |> RoslynHelpers.StartAsyncAsTask cancellationToken
-
-    member this.GetDescriptionAsync2(textView:  ITextView, completionItem: Microsoft.VisualStudio.Language.Intellisense.AsyncCompletion.Data.CompletionItem, cancellationToken: CancellationToken): Task<CompletionDescription> =
-        async {
-            match completionItem.Properties.TryGetProperty IndexPropName with
-            | true, (declarationItem: FSharpDeclarationListItem) ->
-                let! description = declarationItem.StructuredDescriptionTextAsync
-                let documentation = List()
-                let collector = RoslynHelpers.CollectTaggedText documentation
-                // mix main description and xmldoc by using one collector
-                XmlDocumentation.BuildDataTipText(documentationBuilder, collector, collector, collector, collector, collector, description) 
-                return CompletionDescription.Create(documentation.ToImmutableArray())
-            | _ -> 
-                return CompletionDescription.Empty
         } |> RoslynHelpers.StartAsyncAsTask cancellationToken
 
-    override this.GetChangeAsync(document, item, _, cancellationToken) : Task<CompletionChange> =
+    override _.GetChangeAsync(document, item, _, cancellationToken) : Task<CompletionChange> =
         async {
             use _logBlock = Logger.LogBlockMessage document.Name LogEditorFunctionId.Completion_GetChangeAsync
 
@@ -249,9 +269,8 @@ type internal FSharpCompletionProvider
                 | true, x -> Some x
                 | _ -> None
 
-            // do not add extension members, keywords and not yet resolved symbols to the MRU list
-            if not (item.Properties.ContainsKey NamespaceToOpenPropName) && not (item.Properties.ContainsKey IsExtensionMemberPropName) &&
-               not (item.Properties.ContainsKey IsKeywordPropName) then
+            // do not add extension members and unresolved symbols to the MRU list
+            if not (item.Properties.ContainsKey NamespaceToOpenPropName) && not (item.Properties.ContainsKey IsExtensionMemberPropName) then
                 match fullName with
                 | Some fullName ->
                     match mruItems.TryGetValue fullName with