Merge pull request #1809 from SeeminglyScience/update-member-tooltips

Additional IntelliSense fixes and ToolTip overhaul

Author: andyleejordan

src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs

@@ -127,7 +127,15 @@ await executionService.ExecuteDelegateAsync(
                 .ConfigureAwait(false);
 
             stopwatch.Stop();
-            logger.LogTrace($"IntelliSense completed in {stopwatch.ElapsedMilliseconds}ms: {commandCompletion}");
+            logger.LogTrace(
+                "IntelliSense completed in {elapsed}ms - WordToComplete: \"{word}\" MatchCount: {count}",
+                stopwatch.ElapsedMilliseconds,
+                commandCompletion.ReplacementLength > 0
+                    ? scriptAst.Extent.StartScriptPosition.GetFullScript()?.Substring(
+                        commandCompletion.ReplacementIndex,
+                        commandCompletion.ReplacementLength)
+                    : null,
+                commandCompletion.CompletionMatches.Count);
 
             return commandCompletion;
         }

src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs

@@ -3,6 +3,7 @@
 
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Management.Automation;
 using System.Text;
 using System.Text.RegularExpressions;
@@ -30,6 +31,7 @@ internal class PsesCompletionHandler : CompletionHandlerBase
         private readonly IRunspaceContext _runspaceContext;
         private readonly IInternalPowerShellExecutionService _executionService;
         private readonly WorkspaceService _workspaceService;
+        private CompletionCapability _completionCapability;
 
         public PsesCompletionHandler(
             ILoggerFactory factory,
@@ -43,13 +45,23 @@ public PsesCompletionHandler(
             _workspaceService = workspaceService;
         }
 
-        protected override CompletionRegistrationOptions CreateRegistrationOptions(CompletionCapability capability, ClientCapabilities clientCapabilities) => new()
+        protected override CompletionRegistrationOptions CreateRegistrationOptions(CompletionCapability capability, ClientCapabilities clientCapabilities)
         {
-            // TODO: What do we do with the arguments?
-            DocumentSelector = LspUtils.PowerShellDocumentSelector,
-            ResolveProvider = true,
-            TriggerCharacters = new[] { ".", "-", ":", "\\", "$", " " }
-        };
+            _completionCapability = capability;
+            return new CompletionRegistrationOptions()
+            {
+                // TODO: What do we do with the arguments?
+                DocumentSelector = LspUtils.PowerShellDocumentSelector,
+                ResolveProvider = true,
+                TriggerCharacters = new[] { ".", "-", ":", "\\", "$", " " },
+            };
+        }
+
+        public bool SupportsSnippets => _completionCapability?.CompletionItem?.SnippetSupport is true;
+
+        public bool SupportsCommitCharacters => _completionCapability?.CompletionItem?.CommitCharactersSupport is true;
+
+        public bool SupportsMarkdown => _completionCapability?.CompletionItem?.DocumentationFormat?.Contains(MarkupKind.Markdown) is true;
 
         public override async Task<CompletionList> Handle(CompletionParams request, CancellationToken cancellationToken)
         {
@@ -72,6 +84,61 @@ public override async Task<CompletionList> Handle(CompletionParams request, Canc
         // Handler for "completionItem/resolve". In VSCode this is fired when a completion item is highlighted in the completion list.
         public override async Task<CompletionItem> Handle(CompletionItem request, CancellationToken cancellationToken)
         {
+            if (SupportsMarkdown)
+            {
+                if (request.Kind is CompletionItemKind.Method)
+                {
+                    string documentation = FormatUtils.GetMethodDocumentation(
+                        _logger,
+                        request.Data.ToString(),
+                        out MarkupKind kind);
+
+                    return request with
+                    {
+                        Documentation = new MarkupContent()
+                        {
+                            Kind = kind,
+                            Value = documentation,
+                        },
+                    };
+                }
+
+                if (request.Kind is CompletionItemKind.Class or CompletionItemKind.TypeParameter or CompletionItemKind.Enum)
+                {
+                    string documentation = FormatUtils.GetTypeDocumentation(
+                        _logger,
+                        request.Detail,
+                        out MarkupKind kind);
+
+                    return request with
+                    {
+                        Detail = null,
+                        Documentation = new MarkupContent()
+                        {
+                            Kind = kind,
+                            Value = documentation,
+                        },
+                    };
+                }
+
+                if (request.Kind is CompletionItemKind.EnumMember or CompletionItemKind.Property or CompletionItemKind.Field)
+                {
+                    string documentation = FormatUtils.GetPropertyDocumentation(
+                        _logger,
+                        request.Data.ToString(),
+                        out MarkupKind kind);
+
+                    return request with
+                    {
+                        Documentation = new MarkupContent()
+                        {
+                            Kind = kind,
+                            Value = documentation,
+                        },
+                    };
+                }
+            }
+
             // We currently only support this request for anything that returns a CommandInfo:
             // functions, cmdlets, aliases. No detail means the module hasn't been imported yet and
             // IntelliSense shouldn't import the module to get this info.
@@ -143,6 +210,14 @@ internal async Task<CompletionResults> GetCompletionsInFileAsync(
                 result.ReplacementIndex,
                 result.ReplacementIndex + result.ReplacementLength);
 
+            string textToBeReplaced = string.Empty;
+            if (result.ReplacementLength is not 0)
+            {
+                textToBeReplaced = scriptFile.Contents.Substring(
+                    result.ReplacementIndex,
+                    result.ReplacementLength);
+            }
+
             bool isIncomplete = false;
             // Create OmniSharp CompletionItems from PowerShell CompletionResults. We use a for loop
             // because the index is used for sorting.
@@ -159,16 +234,25 @@ internal async Task<CompletionResults> GetCompletionsInFileAsync(
                     isIncomplete = true;
                 }
 
-                completionItems[i] = CreateCompletionItem(result.CompletionMatches[i], replacedRange, i + 1);
+                completionItems[i] = CreateCompletionItem(
+                    result.CompletionMatches[i],
+                    replacedRange,
+                    i + 1,
+                    textToBeReplaced,
+                    scriptFile);
+
                 _logger.LogTrace("Created completion item: " + completionItems[i] + " with " + completionItems[i].TextEdit);
             }
+
             return new CompletionResults(isIncomplete, completionItems);
         }
 
-        internal static CompletionItem CreateCompletionItem(
+        internal CompletionItem CreateCompletionItem(
             CompletionResult result,
             BufferRange completionRange,
-            int sortIndex)
+            int sortIndex,
+            string textToBeReplaced,
+            ScriptFile scriptFile)
         {
             Validate.IsNotNull(nameof(result), result);
 
@@ -200,7 +284,9 @@ internal static CompletionItem CreateCompletionItem(
                     ? string.Empty : detail, // Don't repeat label.
                 // Retain PowerShell's sort order with the given index.
                 SortText = $"{sortIndex:D4}{result.ListItemText}",
-                FilterText = result.CompletionText,
+                FilterText = result.ResultType is CompletionResultType.Type
+                    ? GetTypeFilterText(textToBeReplaced, result.CompletionText)
+                    : result.CompletionText,
                 // Used instead of Label when TextEdit is unsupported
                 InsertText = result.CompletionText,
                 // Used instead of InsertText when possible
@@ -212,17 +298,21 @@ internal static CompletionItem CreateCompletionItem(
                 CompletionResultType.Text => item with { Kind = CompletionItemKind.Text },
                 CompletionResultType.History => item with { Kind = CompletionItemKind.Reference },
                 CompletionResultType.Command => item with { Kind = CompletionItemKind.Function },
-                CompletionResultType.ProviderItem => item with { Kind = CompletionItemKind.File },
-                CompletionResultType.ProviderContainer => TryBuildSnippet(result.CompletionText, out string snippet)
-                    ? item with
-                    {
-                        Kind = CompletionItemKind.Folder,
-                        InsertTextFormat = InsertTextFormat.Snippet,
-                        TextEdit = textEdit with { NewText = snippet }
-                    }
-                    : item with { Kind = CompletionItemKind.Folder },
-                CompletionResultType.Property => item with { Kind = CompletionItemKind.Property },
-                CompletionResultType.Method => item with { Kind = CompletionItemKind.Method },
+                CompletionResultType.ProviderItem or CompletionResultType.ProviderContainer
+                    => CreateProviderItemCompletion(item, result, scriptFile, textToBeReplaced),
+                CompletionResultType.Property => item with
+                {
+                    Kind = CompletionItemKind.Property,
+                    Detail = SupportsMarkdown ? null : detail,
+                    Data = SupportsMarkdown ? detail : null,
+                    CommitCharacters = MaybeAddCommitCharacters("."),
+                },
+                CompletionResultType.Method => item with
+                {
+                    Kind = CompletionItemKind.Method,
+                    Data = item.Detail,
+                    Detail = SupportsMarkdown ? null : item.Detail,
+                },
                 CompletionResultType.ParameterName => TryExtractType(detail, out string type)
                     ? item with { Kind = CompletionItemKind.Variable, Detail = type }
                     // The comparison operators (-eq, -not, -gt, etc) unfortunately come across as
@@ -237,14 +327,109 @@ internal static CompletionItem CreateCompletionItem(
                 CompletionResultType.Type => detail.StartsWith("Class ", StringComparison.CurrentCulture)
                     // Custom classes come through as types but the PowerShell completion tooltip
                     // will start with "Class ", so we can more accurately display its icon.
-                    ? item with { Kind = CompletionItemKind.Class }
-                    : item with { Kind = CompletionItemKind.TypeParameter },
+                    ? item with { Kind = CompletionItemKind.Class, Detail = detail.Substring("Class ".Length) }
+                    : detail.StartsWith("Enum ", StringComparison.CurrentCulture)
+                        ? item with { Kind = CompletionItemKind.Enum, Detail = detail.Substring("Enum ".Length) }
+                        : item with { Kind = CompletionItemKind.TypeParameter },
                 CompletionResultType.Keyword or CompletionResultType.DynamicKeyword =>
                     item with { Kind = CompletionItemKind.Keyword },
                 _ => throw new ArgumentOutOfRangeException(nameof(result))
             };
         }
 
+        private CompletionItem CreateProviderItemCompletion(
+            CompletionItem item,
+            CompletionResult result,
+            ScriptFile scriptFile,
+            string textToBeReplaced)
+        {
+            // TODO: Work out a way to do this generally instead of special casing PSScriptRoot.
+            //
+            // This code relies on PowerShell/PowerShell#17376. Until that makes it into a release
+            // no matches will be returned anyway.
+            const string PSScriptRootVariable = "$PSScriptRoot";
+            string completionText = result.CompletionText;
+            if (textToBeReplaced.IndexOf(PSScriptRootVariable, StringComparison.OrdinalIgnoreCase) is int variableIndex and not -1
+                && System.IO.Path.GetDirectoryName(scriptFile.FilePath) is string scriptFolder and not ""
+                && completionText.IndexOf(scriptFolder, StringComparison.OrdinalIgnoreCase) is int pathIndex and not -1
+                && !scriptFile.IsInMemory)
+            {
+                completionText = completionText
+                    .Remove(pathIndex, scriptFolder.Length)
+                    .Insert(variableIndex, textToBeReplaced.Substring(variableIndex, PSScriptRootVariable.Length));
+            }
+
+            InsertTextFormat insertFormat;
+            TextEdit edit;
+            CompletionItemKind itemKind;
+            if (result.ResultType is CompletionResultType.ProviderContainer
+                && SupportsSnippets
+                && TryBuildSnippet(completionText, out string snippet))
+            {
+                edit = item.TextEdit.TextEdit with { NewText = snippet };
+                insertFormat = InsertTextFormat.Snippet;
+                itemKind = CompletionItemKind.Folder;
+            }
+            else
+            {
+                edit = item.TextEdit.TextEdit with { NewText = completionText };
+                insertFormat = default;
+                itemKind = CompletionItemKind.File;
+            }
+
+            return item with
+            {
+                Kind = itemKind,
+                TextEdit = edit,
+                InsertText = completionText,
+                FilterText = completionText,
+                InsertTextFormat = insertFormat,
+                CommitCharacters = MaybeAddCommitCharacters("\\", "/", "'", "\""),
+            };
+        }
+
+        private Container<string> MaybeAddCommitCharacters(params string[] characters)
+            => SupportsCommitCharacters ? new Container<string>(characters) : null;
+
+        private static string GetTypeFilterText(string textToBeReplaced, string completionText)
+        {
+            // FilterText for a type name with using statements gets a little complicated. Consider
+            // this script:
+            //
+            // using namespace System.Management.Automation
+            // [System.Management.Automation.Tracing.]
+            //
+            // Since we're emitting an edit that replaces `System.Management.Automation.Tracing.` with
+            // `Tracing.NullWriter` (for example), we can't use CompletionText as the filter. If we
+            // do, we won't find any matches because it's trying to filter `Tracing.NullWriter` with
+            // `System.Management.Automation.Tracing.` which is too different. So we prepend each
+            // namespace that exists in our original text but does not in our completion text.
+            if (!textToBeReplaced.Contains('.'))
+            {
+                return completionText;
+            }
+
+            string[] oldTypeParts = textToBeReplaced.Split('.');
+            string[] newTypeParts = completionText.Split('.');
+
+            StringBuilder newFilterText = new(completionText);
+
+            int newPartsIndex = newTypeParts.Length - 2;
+            for (int i = oldTypeParts.Length - 2; i >= 0; i--)
+            {
+                if (newPartsIndex is >= 0
+                    && newTypeParts[newPartsIndex].Equals(oldTypeParts[i], StringComparison.OrdinalIgnoreCase))
+                {
+                    newPartsIndex--;
+                    continue;
+                }
+
+                newFilterText.Insert(0, '.').Insert(0, oldTypeParts[i]);
+            }
+
+            return newFilterText.ToString();
+        }
+
         private static readonly Regex s_typeRegex = new(@"^(\[.+\])", RegexOptions.Compiled);
 
         /// <summary>

src/PowerShellEditorServices/Utility/Extensions.cs

@@ -5,6 +5,7 @@
 using System.Linq;
 using System.Collections.Generic;
 using System.Management.Automation.Language;
+using System.Text;
 
 namespace Microsoft.PowerShell.EditorServices.Utility
 {
@@ -144,5 +145,21 @@ public static bool Contains(this IScriptExtent scriptExtent, int line, int colum
 
             return true;
         }
+
+        /// <summary>
+        /// Same as <see cref="StringBuilder.AppendLine()" /> but never CRLF. Use this when building
+        /// formatting for clients that may not render CRLF correctly.
+        /// </summary>
+        /// <param name="self"></param>
+        public static StringBuilder AppendLineLF(this StringBuilder self) => self.Append('\n');
+
+        /// <summary>
+        /// Same as <see cref="StringBuilder.AppendLine(string)" /> but never CRLF. Use this when building
+        /// formatting for clients that may not render CRLF correctly.
+        /// </summary>
+        /// <param name="self"></param>
+        /// <param name="value"></param>
+        public static StringBuilder AppendLineLF(this StringBuilder self, string value)
+            => self.Append(value).Append('\n');
     }
 }