From 5aafa682a95e48574e50de36a437733651db8ab9 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Mon, 7 Mar 2022 16:48:33 -0800 Subject: [PATCH 1/7] Improve completion logic This makes the deduced `CompletionItemKind` enum more accurate, which results in better icons for the user when viewing completions in an editor. Also cleaned up the file as there were remnants of dead code. --- .../TextDocument/CompletionResults.cs | 67 +++---- .../Handlers/CompletionHandler.cs | 171 ++++++++---------- 2 files changed, 98 insertions(+), 140 deletions(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs b/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs index 8f35af586..91ff21259 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs @@ -147,7 +147,17 @@ internal enum CompletionType /// /// Identifies a completion for a provider path (like a file system path) to a container. /// - Folder + Folder, + + /// + /// Identifies a completion for history. + /// + History, + + /// + /// Identifies a completion for just text. + /// + Text } /// @@ -277,46 +287,23 @@ public override int GetHashCode() private static CompletionType ConvertCompletionResultType( CompletionResultType completionResultType) { - switch (completionResultType) + return completionResultType switch { - case CompletionResultType.Command: - return CompletionType.Command; - - case CompletionResultType.Method: - return CompletionType.Method; - - case CompletionResultType.ParameterName: - return CompletionType.ParameterName; - - case CompletionResultType.ParameterValue: - return CompletionType.ParameterValue; - - case CompletionResultType.Property: - return CompletionType.Property; - - case CompletionResultType.Variable: - return CompletionType.Variable; - - case CompletionResultType.Namespace: - return CompletionType.Namespace; - - case CompletionResultType.Type: - return CompletionType.Type; - - case CompletionResultType.Keyword: - case CompletionResultType.DynamicKeyword: - return CompletionType.Keyword; - - case CompletionResultType.ProviderContainer: - return CompletionType.Folder; - - case CompletionResultType.ProviderItem: - return CompletionType.File; - - default: - // TODO: Trace the unsupported CompletionResultType - return CompletionType.Unknown; - } + CompletionResultType.Command => CompletionType.Command, + CompletionResultType.Method => CompletionType.Method, + CompletionResultType.ParameterName => CompletionType.ParameterName, + CompletionResultType.ParameterValue => CompletionType.ParameterValue, + CompletionResultType.Property => CompletionType.Property, + CompletionResultType.Variable => CompletionType.Variable, + CompletionResultType.Namespace => CompletionType.Namespace, + CompletionResultType.Type => CompletionType.Type, + CompletionResultType.Keyword or CompletionResultType.DynamicKeyword => CompletionType.Keyword, + CompletionResultType.ProviderContainer => CompletionType.Folder, + CompletionResultType.ProviderItem => CompletionType.File, + CompletionResultType.History => CompletionType.History, + CompletionResultType.Text => CompletionType.Text, + _ => CompletionType.Unknown, + }; } private static string ExtractSymbolTypeNameFromToolTip(string toolTipText) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 2581e2a6e..7b9815fbd 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -24,17 +24,15 @@ namespace Microsoft.PowerShell.EditorServices.Handlers // TODO: Use ABCs. internal class PsesCompletionHandler : ICompletionHandler, ICompletionResolveHandler { - const int DefaultWaitTimeoutMilliseconds = 5000; + private const int DefaultWaitTimeoutMilliseconds = 5000; private readonly ILogger _logger; private readonly IRunspaceContext _runspaceContext; private readonly IInternalPowerShellExecutionService _executionService; private readonly WorkspaceService _workspaceService; - private CompletionResults _mostRecentCompletions; - private int _mostRecentRequestLine; - private int _mostRecentRequestOffest; - private string _mostRecentRequestFile; private CompletionCapability _capability; private readonly Guid _id = Guid.NewGuid(); + private static readonly Regex _regex = new(@"^(\[.+\])"); + Guid ICanBeIdentifiedHandler.Id => _id; public PsesCompletionHandler( @@ -49,7 +47,7 @@ public PsesCompletionHandler( _workspaceService = workspaceService; } - public CompletionRegistrationOptions GetRegistrationOptions(CompletionCapability capability, ClientCapabilities clientCapabilities) => new CompletionRegistrationOptions + public CompletionRegistrationOptions GetRegistrationOptions(CompletionCapability capability, ClientCapabilities clientCapabilities) => new() { DocumentSelector = LspUtils.PowerShellDocumentSelector, ResolveProvider = true, @@ -69,13 +67,9 @@ public async Task Handle(CompletionParams request, CancellationT return Array.Empty(); } - CompletionResults completionResults = - await GetCompletionsInFileAsync( - scriptFile, - cursorLine, - cursorColumn).ConfigureAwait(false); + CompletionResults completionResults = await GetCompletionsInFileAsync(scriptFile, cursorLine, cursorColumn).ConfigureAwait(false); - if (completionResults == null) + if (completionResults is null) { return Array.Empty(); } @@ -110,13 +104,9 @@ public async Task Handle(CompletionItem request, CancellationTok } // Get the documentation for the function - CommandInfo commandInfo = - await CommandHelpers.GetCommandInfoAsync( - request.Label, - _runspaceContext.CurrentRunspace, - _executionService).ConfigureAwait(false); + CommandInfo commandInfo = await CommandHelpers.GetCommandInfoAsync(request.Label, _runspaceContext.CurrentRunspace, _executionService).ConfigureAwait(false); - if (commandInfo != null) + if (commandInfo is not null) { request = request with { @@ -158,13 +148,10 @@ public async Task GetCompletionsInFileAsync( // Get the offset at the specified position. This method // will also validate the given position. - int fileOffset = - scriptFile.GetOffsetAtPosition( - lineNumber, - columnNumber); + int fileOffset = scriptFile.GetOffsetAtPosition(lineNumber, columnNumber); CommandCompletion commandCompletion = null; - using (var cts = new CancellationTokenSource(DefaultWaitTimeoutMilliseconds)) + using (CancellationTokenSource cts = new(DefaultWaitTimeoutMilliseconds)) { commandCompletion = await AstOperations.GetCompletionsAsync( @@ -176,33 +163,20 @@ await AstOperations.GetCompletionsAsync( cts.Token).ConfigureAwait(false); } - if (commandCompletion == null) + if (commandCompletion is null) { return new CompletionResults(); } try { - CompletionResults completionResults = - CompletionResults.Create( - scriptFile, - commandCompletion); - - // save state of most recent completion - _mostRecentCompletions = completionResults; - _mostRecentRequestFile = scriptFile.Id; - _mostRecentRequestLine = lineNumber; - _mostRecentRequestOffest = columnNumber; - - return completionResults; + return CompletionResults.Create(scriptFile, commandCompletion); } catch (ArgumentException e) { // Bad completion results could return an invalid // replacement range, catch that here - _logger.LogError( - $"Caught exception while trying to create CompletionResults:\n\n{e.ToString()}"); - + _logger.LogError($"Caught exception while trying to create CompletionResults:\n\n{e.ToString()}"); return new CompletionResults(); } } @@ -212,57 +186,65 @@ private static CompletionItem CreateCompletionItem( BufferRange completionRange, int sortIndex) { - string detailString = null; - string documentationString = null; + string toolTipText = completionDetails.ToolTipText; string completionText = completionDetails.CompletionText; + CompletionItemKind kind = MapCompletionKind(completionDetails.CompletionType); InsertTextFormat insertTextFormat = InsertTextFormat.PlainText; switch (completionDetails.CompletionType) { - case CompletionType.Type: case CompletionType.Namespace: case CompletionType.ParameterValue: case CompletionType.Method: case CompletionType.Property: - detailString = completionDetails.ToolTipText; - break; + case CompletionType.Keyword: + case CompletionType.File: + case CompletionType.History: + case CompletionType.Text: case CompletionType.Variable: + case CompletionType.Unknown: + break; + case CompletionType.Type: + // Custom classes come through as types but the PowerShell completion tooltip + // will start with "Class ", so we can more accurately display its icon. + if (toolTipText.StartsWith("Class ", StringComparison.Ordinal)) + { + kind = CompletionItemKind.Class; + } + break; case CompletionType.ParameterName: // Look for type encoded in the tooltip for parameters and variables. // Display PowerShell type names in [] to be consistent with PowerShell syntax // and how the debugger displays type names. - var matches = Regex.Matches(completionDetails.ToolTipText, @"^(\[.+\])"); + MatchCollection matches = _regex.Matches(toolTipText); if ((matches.Count > 0) && (matches[0].Groups.Count > 1)) { - detailString = matches[0].Groups[1].Value; + toolTipText = matches[0].Groups[1].Value; } - // The comparison operators (-eq, -not, -gt, etc) are unfortunately fall into ParameterName - // but they don't have a type associated to them. This allows those tooltips to show up. - else if (!string.IsNullOrEmpty(completionDetails.ToolTipText)) + // The comparison operators (-eq, -not, -gt, etc) unfortunately come across as + // ParameterName types but they don't have a type associated to them, so we can + // deduce its an operator. + else { - detailString = completionDetails.ToolTipText; + kind = CompletionItemKind.Operator; } break; case CompletionType.Command: - // For Commands, let's extract the resolved command or the path for an exe - // from the ToolTipText - if there is any ToolTipText. - if (completionDetails.ToolTipText != null) + // For commands, let's extract the resolved command or the path for an + // executable from the tooltip. + if (!string.IsNullOrEmpty(toolTipText)) { // Fix for #240 - notepad++.exe in tooltip text caused regex parser to throw. - string escapedToolTipText = Regex.Escape(completionDetails.ToolTipText); - - // Don't display ToolTipText if it is the same as the ListItemText. - // Reject command syntax ToolTipText - it's too much to display as a detailString. - if (!completionDetails.ListItemText.Equals( - completionDetails.ToolTipText, - StringComparison.OrdinalIgnoreCase) && - !Regex.IsMatch(completionDetails.ToolTipText, - @"^\s*" + escapedToolTipText + @"\s+\[")) + string escapedToolTipText = Regex.Escape(toolTipText); + + // Don't display tooltip if it is the same as the ListItemText. Don't + // display command syntax tooltip because it's too much. + if (completionDetails.ListItemText.Equals(toolTipText, StringComparison.OrdinalIgnoreCase) + || Regex.IsMatch(toolTipText, @"^\s*" + escapedToolTipText + @"\s+\[")) { - detailString = completionDetails.ToolTipText; + toolTipText = string.Empty; } } - break; case CompletionType.Folder: // Insert a final "tab stop" as identified by $0 in the snippet provided for completion. @@ -275,14 +257,13 @@ private static CompletionItem CreateCompletionItem( // Since we want to use a "tab stop" we need to escape a few things for Textmate to render properly. if (EndsWithQuote(completionText)) { - var sb = new StringBuilder(completionDetails.CompletionText) + StringBuilder sb = new StringBuilder(completionText) .Replace(@"\", @"\\") .Replace(@"}", @"\}") .Replace(@"$", @"\$"); completionText = sb.Insert(sb.Length - 1, "$0").ToString(); insertTextFormat = InsertTextFormat.Snippet; } - break; } @@ -291,16 +272,16 @@ private static CompletionItem CreateCompletionItem( // make sure the default order also be the lexicographical order // which we do by prefixing the ListItemText with a leading 0's // four digit index. - var sortText = $"{sortIndex:D4}{completionDetails.ListItemText}"; + string sortText = $"{sortIndex:D4}{completionDetails.ListItemText}"; return new CompletionItem { InsertText = completionText, InsertTextFormat = insertTextFormat, Label = completionDetails.ListItemText, - Kind = MapCompletionKind(completionDetails.CompletionType), - Detail = detailString, - Documentation = documentationString, + Kind = kind, + Detail = toolTipText, + Documentation = string.Empty, // TODO: Fill this in agin. SortText = sortText, FilterText = completionDetails.CompletionText, TextEdit = new TextEdit @@ -323,43 +304,33 @@ private static CompletionItem CreateCompletionItem( }; } + // TODO: Unwrap this, it's confusing as it doesn't cover all cases and we conditionally + // override it when it's inaccurate. private static CompletionItemKind MapCompletionKind(CompletionType completionType) { - switch (completionType) + return completionType switch { - case CompletionType.Command: - return CompletionItemKind.Function; - - case CompletionType.Property: - return CompletionItemKind.Property; - - case CompletionType.Method: - return CompletionItemKind.Method; - - case CompletionType.Variable: - case CompletionType.ParameterName: - return CompletionItemKind.Variable; - - case CompletionType.File: - return CompletionItemKind.File; - - case CompletionType.Folder: - return CompletionItemKind.Folder; - - default: - return CompletionItemKind.Text; - } + CompletionType.Unknown => CompletionItemKind.Text, + CompletionType.Command => CompletionItemKind.Function, + CompletionType.Method => CompletionItemKind.Method, + CompletionType.ParameterName => CompletionItemKind.Variable, + CompletionType.ParameterValue => CompletionItemKind.Value, + CompletionType.Property => CompletionItemKind.Property, + CompletionType.Variable => CompletionItemKind.Variable, + CompletionType.Namespace => CompletionItemKind.Module, + CompletionType.Type => CompletionItemKind.TypeParameter, + CompletionType.Keyword => CompletionItemKind.Keyword, + CompletionType.File => CompletionItemKind.File, + CompletionType.Folder => CompletionItemKind.Folder, + CompletionType.History => CompletionItemKind.Reference, + CompletionType.Text => CompletionItemKind.Text, + _ => throw new ArgumentOutOfRangeException(nameof(completionType)), + }; } private static bool EndsWithQuote(string text) { - if (string.IsNullOrEmpty(text)) - { - return false; - } - - char lastChar = text[text.Length - 1]; - return lastChar == '"' || lastChar == '\''; + return !string.IsNullOrEmpty(text) && text[text.Length - 1] is '"' or '\''; } } } From 3b22c4fd0c3ec6ebdbb52939168684022e4b8ee3 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Tue, 8 Mar 2022 17:06:36 -0800 Subject: [PATCH 2/7] Remove superfluous `CompletionResults.cs` file (and fix tooltips) This removes an abstraction that sat between `SMA.CompletionResults` and `OmniSharp.CompletionItems`. It most likely existed out of necessity before a prior refactor that introduced OmniSharp. Now we can skip the middle layer and instead convert directly, massively simplifying the code (and fixing tooltips). --- .../TextDocument/CompletionResults.cs | 326 ------------------ .../Handlers/CompletionHandler.cs | 244 +++++++------ 2 files changed, 119 insertions(+), 451 deletions(-) delete mode 100644 src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs diff --git a/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs b/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs deleted file mode 100644 index 91ff21259..000000000 --- a/src/PowerShellEditorServices/Services/TextDocument/CompletionResults.cs +++ /dev/null @@ -1,326 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; -using System.Management.Automation; -using System.Text.RegularExpressions; -using Microsoft.PowerShell.EditorServices.Utility; - -namespace Microsoft.PowerShell.EditorServices.Services.TextDocument -{ - /// - /// Provides the results of a single code completion request. - /// - internal sealed class CompletionResults - { - #region Properties - - /// - /// Gets the completions that were found during the - /// completion request. - /// - public CompletionDetails[] Completions { get; private set; } - - /// - /// Gets the range in the buffer that should be replaced by this - /// completion result. - /// - public BufferRange ReplacedRange { get; private set; } - - #endregion - - #region Constructors - - /// - /// Creates an empty CompletionResults instance. - /// - public CompletionResults() - { - this.Completions = Array.Empty(); - this.ReplacedRange = new BufferRange(0, 0, 0, 0); - } - - internal static CompletionResults Create( - ScriptFile scriptFile, - CommandCompletion commandCompletion) - { - BufferRange replacedRange = null; - - // Only calculate the replacement range if there are completion results - if (commandCompletion.CompletionMatches.Count > 0) - { - replacedRange = - scriptFile.GetRangeBetweenOffsets( - commandCompletion.ReplacementIndex, - commandCompletion.ReplacementIndex + commandCompletion.ReplacementLength); - } - - return new CompletionResults - { - Completions = GetCompletionsArray(commandCompletion), - ReplacedRange = replacedRange - }; - } - - #endregion - - #region Private Methods - - private static CompletionDetails[] GetCompletionsArray( - CommandCompletion commandCompletion) - { - IEnumerable completionList = - commandCompletion.CompletionMatches.Select( - CompletionDetails.Create); - - return completionList.ToArray(); - } - - #endregion - } - - /// - /// Enumerates the completion types that may be returned. - /// - internal enum CompletionType - { - /// - /// Completion type is unknown, either through being uninitialized or - /// having been created from an unsupported CompletionResult that was - /// returned by the PowerShell engine. - /// - Unknown = 0, - - /// - /// Identifies a completion for a command. - /// - Command, - - /// - /// Identifies a completion for a .NET method. - /// - Method, - - /// - /// Identifies a completion for a command parameter name. - /// - ParameterName, - - /// - /// Identifies a completion for a command parameter value. - /// - ParameterValue, - - /// - /// Identifies a completion for a .NET property. - /// - Property, - - /// - /// Identifies a completion for a variable name. - /// - Variable, - - /// - /// Identifies a completion for a namespace. - /// - Namespace, - - /// - /// Identifies a completion for a .NET type name. - /// - Type, - - /// - /// Identifies a completion for a PowerShell language keyword. - /// - Keyword, - - /// - /// Identifies a completion for a provider path (like a file system path) to a leaf item. - /// - File, - - /// - /// Identifies a completion for a provider path (like a file system path) to a container. - /// - Folder, - - /// - /// Identifies a completion for history. - /// - History, - - /// - /// Identifies a completion for just text. - /// - Text - } - - /// - /// Provides the details about a single completion result. - /// - [DebuggerDisplay("CompletionType = {CompletionType.ToString()}, CompletionText = {CompletionText}")] - internal sealed class CompletionDetails - { - #region Properties - - /// - /// Gets the text that will be used to complete the statement - /// at the requested file offset. - /// - public string CompletionText { get; private set; } - - /// - /// Gets the text that should be dispayed in a drop-down completion list. - /// - public string ListItemText { get; private set; } - - /// - /// Gets the text that can be used to display a tooltip for - /// the statement at the requested file offset. - /// - public string ToolTipText { get; private set; } - - /// - /// Gets the name of the type which this symbol represents. - /// If the symbol doesn't have an inherent type, null will - /// be returned. - /// - public string SymbolTypeName { get; private set; } - - /// - /// Gets the CompletionType which identifies the type of this completion. - /// - public CompletionType CompletionType { get; private set; } - - #endregion - - #region Constructors - - internal static CompletionDetails Create(CompletionResult completionResult) - { - Validate.IsNotNull("completionResult", completionResult); - - // Some tooltips may have newlines or whitespace for unknown reasons - string toolTipText = completionResult.ToolTip; - if (toolTipText != null) - { - toolTipText = toolTipText.Trim(); - } - - return new CompletionDetails - { - CompletionText = completionResult.CompletionText, - ListItemText = completionResult.ListItemText, - ToolTipText = toolTipText, - SymbolTypeName = ExtractSymbolTypeNameFromToolTip(completionResult.ToolTip), - CompletionType = - ConvertCompletionResultType( - completionResult.ResultType) - }; - } - - internal static CompletionDetails Create( - string completionText, - CompletionType completionType, - string toolTipText = null, - string symbolTypeName = null, - string listItemText = null) - { - return new CompletionDetails - { - CompletionText = completionText, - CompletionType = completionType, - ListItemText = listItemText, - ToolTipText = toolTipText, - SymbolTypeName = symbolTypeName - }; - } - - #endregion - - #region Public Methods - - /// - /// Compares two CompletionResults instances for equality. - /// - /// The potential CompletionResults instance to compare. - /// True if the CompletionResults instances have the same details. - public override bool Equals(object obj) - { - if (!(obj is CompletionDetails otherDetails)) - { - return false; - } - - return - string.Equals(this.CompletionText, otherDetails.CompletionText) && - this.CompletionType == otherDetails.CompletionType && - string.Equals(this.ToolTipText, otherDetails.ToolTipText) && - string.Equals(this.SymbolTypeName, otherDetails.SymbolTypeName); - } - - /// - /// Returns the hash code for this CompletionResults instance. - /// - /// The hash code for this CompletionResults instance. - public override int GetHashCode() - { - return - string.Format( - "{0}{1}{2}{3}{4}", - this.CompletionText, - this.CompletionType, - this.ListItemText, - this.ToolTipText, - this.SymbolTypeName).GetHashCode(); - } - - #endregion - - #region Private Methods - - private static CompletionType ConvertCompletionResultType( - CompletionResultType completionResultType) - { - return completionResultType switch - { - CompletionResultType.Command => CompletionType.Command, - CompletionResultType.Method => CompletionType.Method, - CompletionResultType.ParameterName => CompletionType.ParameterName, - CompletionResultType.ParameterValue => CompletionType.ParameterValue, - CompletionResultType.Property => CompletionType.Property, - CompletionResultType.Variable => CompletionType.Variable, - CompletionResultType.Namespace => CompletionType.Namespace, - CompletionResultType.Type => CompletionType.Type, - CompletionResultType.Keyword or CompletionResultType.DynamicKeyword => CompletionType.Keyword, - CompletionResultType.ProviderContainer => CompletionType.Folder, - CompletionResultType.ProviderItem => CompletionType.File, - CompletionResultType.History => CompletionType.History, - CompletionResultType.Text => CompletionType.Text, - _ => CompletionType.Unknown, - }; - } - - private static string ExtractSymbolTypeNameFromToolTip(string toolTipText) - { - // Tooltips returned from PowerShell contain the symbol type in - // brackets. Attempt to extract such strings for further processing. - var matches = Regex.Matches(toolTipText, @"^\[(.+)\]"); - - if (matches.Count > 0 && matches[0].Groups.Count > 1) - { - // Return the symbol type name - return matches[0].Groups[1].Value; - } - - return null; - } - - #endregion - } -} diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 7b9815fbd..483402ccb 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.Management.Automation; using System.Text; using System.Text.RegularExpressions; @@ -31,7 +32,7 @@ internal class PsesCompletionHandler : ICompletionHandler, ICompletionResolveHan private readonly WorkspaceService _workspaceService; private CompletionCapability _capability; private readonly Guid _id = Guid.NewGuid(); - private static readonly Regex _regex = new(@"^(\[.+\])"); + private static readonly Regex _typeRegex = new(@"^(\[.+\])"); Guid ICanBeIdentifiedHandler.Id => _id; @@ -67,20 +68,9 @@ public async Task Handle(CompletionParams request, CancellationT return Array.Empty(); } - CompletionResults completionResults = await GetCompletionsInFileAsync(scriptFile, cursorLine, cursorColumn).ConfigureAwait(false); + IEnumerable completionResults = await GetCompletionsInFileAsync(scriptFile, cursorLine, cursorColumn).ConfigureAwait(false); - if (completionResults is null) - { - return Array.Empty(); - } - - CompletionItem[] completionItems = new CompletionItem[completionResults.Completions.Length]; - for (int i = 0; i < completionItems.Length; i++) - { - completionItems[i] = CreateCompletionItem(completionResults.Completions[i], completionResults.ReplacedRange, i + 1); - } - - return completionItems; + return new CompletionList(completionResults); } public static bool CanResolve(CompletionItem value) @@ -139,84 +129,88 @@ public void SetCapability(CompletionCapability capability, ClientCapabilities cl /// /// A CommandCompletion instance completions for the identified statement. /// - public async Task GetCompletionsInFileAsync( + public async Task> GetCompletionsInFileAsync( ScriptFile scriptFile, int lineNumber, int columnNumber) { Validate.IsNotNull(nameof(scriptFile), scriptFile); - // Get the offset at the specified position. This method - // will also validate the given position. - int fileOffset = scriptFile.GetOffsetAtPosition(lineNumber, columnNumber); - - CommandCompletion commandCompletion = null; + CommandCompletion result = null; using (CancellationTokenSource cts = new(DefaultWaitTimeoutMilliseconds)) { - commandCompletion = - await AstOperations.GetCompletionsAsync( - scriptFile.ScriptAst, - scriptFile.ScriptTokens, - fileOffset, - _executionService, - _logger, - cts.Token).ConfigureAwait(false); + result = await AstOperations.GetCompletionsAsync( + scriptFile.ScriptAst, + scriptFile.ScriptTokens, + scriptFile.GetOffsetAtPosition(lineNumber, columnNumber), + _executionService, + _logger, + cts.Token).ConfigureAwait(false); } - if (commandCompletion is null) + // Only calculate the replacement range if there are completions. + BufferRange replacedRange = new(0, 0, 0, 0); + if (result.CompletionMatches.Count > 0) { - return new CompletionResults(); + replacedRange = scriptFile.GetRangeBetweenOffsets( + result.ReplacementIndex, + result.ReplacementIndex + result.ReplacementLength); } - try + // Create OmniSharp CompletionItems from PowerShell CompletionResults. We use a for loop + // because the index is used for sorting. + CompletionItem[] completionItems = new CompletionItem[result.CompletionMatches.Count]; + for (int i = 0; i < result.CompletionMatches.Count; i++) { - return CompletionResults.Create(scriptFile, commandCompletion); - } - catch (ArgumentException e) - { - // Bad completion results could return an invalid - // replacement range, catch that here - _logger.LogError($"Caught exception while trying to create CompletionResults:\n\n{e.ToString()}"); - return new CompletionResults(); + completionItems[i] = CreateCompletionItem(result.CompletionMatches[i], replacedRange, i + 1); } + return completionItems; } - private static CompletionItem CreateCompletionItem( - CompletionDetails completionDetails, + internal static CompletionItem CreateCompletionItem( + CompletionResult completion, BufferRange completionRange, int sortIndex) { - string toolTipText = completionDetails.ToolTipText; - string completionText = completionDetails.CompletionText; - CompletionItemKind kind = MapCompletionKind(completionDetails.CompletionType); + Validate.IsNotNull(nameof(completion), completion); + + // Some tooltips may have newlines or whitespace for unknown reasons. + string toolTipText = completion.ToolTip?.Trim(); + + string completionText = completion.CompletionText; InsertTextFormat insertTextFormat = InsertTextFormat.PlainText; + CompletionItemKind kind; + + // Force the client to maintain the sort order in which the original completion results + // were returned. We just need to make sure the default order also be the + // lexicographical order which we do by prefixing the ListItemText with a leading 0's + // four digit index. + string sortText = $"{sortIndex:D4}{completion.ListItemText}"; - switch (completionDetails.CompletionType) + switch (completion.ResultType) { - case CompletionType.Namespace: - case CompletionType.ParameterValue: - case CompletionType.Method: - case CompletionType.Property: - case CompletionType.Keyword: - case CompletionType.File: - case CompletionType.History: - case CompletionType.Text: - case CompletionType.Variable: - case CompletionType.Unknown: + case CompletionResultType.Command: + kind = CompletionItemKind.Function; break; - case CompletionType.Type: - // Custom classes come through as types but the PowerShell completion tooltip - // will start with "Class ", so we can more accurately display its icon. - if (toolTipText.StartsWith("Class ", StringComparison.Ordinal)) - { - kind = CompletionItemKind.Class; - } + case CompletionResultType.History: + kind = CompletionItemKind.Reference; + break; + case CompletionResultType.Keyword: + case CompletionResultType.DynamicKeyword: + kind = CompletionItemKind.Keyword; + break; + case CompletionResultType.Method: + kind = CompletionItemKind.Method; + break; + case CompletionResultType.Namespace: + kind = CompletionItemKind.Module; break; - case CompletionType.ParameterName: + case CompletionResultType.ParameterName: + kind = CompletionItemKind.Variable; // Look for type encoded in the tooltip for parameters and variables. // Display PowerShell type names in [] to be consistent with PowerShell syntax // and how the debugger displays type names. - MatchCollection matches = _regex.Matches(toolTipText); + MatchCollection matches = _typeRegex.Matches(toolTipText); if ((matches.Count > 0) && (matches[0].Groups.Count > 1)) { toolTipText = matches[0].Groups[1].Value; @@ -229,32 +223,24 @@ private static CompletionItem CreateCompletionItem( kind = CompletionItemKind.Operator; } break; - case CompletionType.Command: - // For commands, let's extract the resolved command or the path for an - // executable from the tooltip. - if (!string.IsNullOrEmpty(toolTipText)) - { - // Fix for #240 - notepad++.exe in tooltip text caused regex parser to throw. - string escapedToolTipText = Regex.Escape(toolTipText); - - // Don't display tooltip if it is the same as the ListItemText. Don't - // display command syntax tooltip because it's too much. - if (completionDetails.ListItemText.Equals(toolTipText, StringComparison.OrdinalIgnoreCase) - || Regex.IsMatch(toolTipText, @"^\s*" + escapedToolTipText + @"\s+\[")) - { - toolTipText = string.Empty; - } - } + case CompletionResultType.ParameterValue: + kind = CompletionItemKind.Value; + break; + case CompletionResultType.Property: + kind = CompletionItemKind.Property; break; - case CompletionType.Folder: - // Insert a final "tab stop" as identified by $0 in the snippet provided for completion. - // For folder paths, we take the path returned by PowerShell e.g. 'C:\Program Files' and insert - // the tab stop marker before the closing quote char e.g. 'C:\Program Files$0'. - // This causes the editing cursor to be placed *before* the final quote after completion, - // which makes subsequent path completions work. See this part of the LSP spec for details: + case CompletionResultType.ProviderContainer: + kind = CompletionItemKind.Folder; + // Insert a final "tab stop" as identified by $0 in the snippet provided for + // completion. For folder paths, we take the path returned by PowerShell e.g. + // 'C:\Program Files' and insert the tab stop marker before the closing quote + // char e.g. 'C:\Program Files$0'. This causes the editing cursor to be placed + // *before* the final quote after completion, which makes subsequent path + // completions work. See this part of the LSP spec for details: // https://microsoft.github.io/language-server-protocol/specification#textDocument_completion - // Since we want to use a "tab stop" we need to escape a few things for Textmate to render properly. + // Since we want to use a "tab stop" we need to escape a few things for Textmate + // to render properly. if (EndsWithQuote(completionText)) { StringBuilder sb = new StringBuilder(completionText) @@ -265,25 +251,49 @@ private static CompletionItem CreateCompletionItem( insertTextFormat = InsertTextFormat.Snippet; } break; + case CompletionResultType.ProviderItem: + kind = CompletionItemKind.File; + break; + case CompletionResultType.Text: + kind = CompletionItemKind.Text; + break; + case CompletionResultType.Type: + kind = CompletionItemKind.TypeParameter; + // Custom classes come through as types but the PowerShell completion tooltip + // will start with "Class ", so we can more accurately display its icon. + if (toolTipText.StartsWith("Class ", StringComparison.Ordinal)) + { + kind = CompletionItemKind.Class; + } + break; + case CompletionResultType.Variable: + kind = CompletionItemKind.Variable; + // Look for type encoded in the tooltip for parameters and variables. + // Display PowerShell type names in [] to be consistent with PowerShell syntax + // and how the debugger displays type names. + matches = _typeRegex.Matches(toolTipText); + if ((matches.Count > 0) && (matches[0].Groups.Count > 1)) + { + toolTipText = matches[0].Groups[1].Value; + } + break; + default: + throw new ArgumentOutOfRangeException(nameof(completion)); } - // Force the client to maintain the sort order in which the - // original completion results were returned. We just need to - // make sure the default order also be the lexicographical order - // which we do by prefixing the ListItemText with a leading 0's - // four digit index. - string sortText = $"{sortIndex:D4}{completionDetails.ListItemText}"; + // Don't display tooltip if it is the same as the ListItemText. + if (completion.ListItemText.Equals(toolTipText, StringComparison.OrdinalIgnoreCase)) + { + toolTipText = string.Empty; + } + Validate.IsNotNull(nameof(CompletionItemKind), kind); + + // TODO: We used to extract the symbol type from the tooltip using a regex, but it + // wasn't actually used. return new CompletionItem { - InsertText = completionText, - InsertTextFormat = insertTextFormat, - Label = completionDetails.ListItemText, Kind = kind, - Detail = toolTipText, - Documentation = string.Empty, // TODO: Fill this in agin. - SortText = sortText, - FilterText = completionDetails.CompletionText, TextEdit = new TextEdit { NewText = completionText, @@ -300,31 +310,15 @@ private static CompletionItem CreateCompletionItem( Character = completionRange.End.Column - 1 } } - } - }; - } - - // TODO: Unwrap this, it's confusing as it doesn't cover all cases and we conditionally - // override it when it's inaccurate. - private static CompletionItemKind MapCompletionKind(CompletionType completionType) - { - return completionType switch - { - CompletionType.Unknown => CompletionItemKind.Text, - CompletionType.Command => CompletionItemKind.Function, - CompletionType.Method => CompletionItemKind.Method, - CompletionType.ParameterName => CompletionItemKind.Variable, - CompletionType.ParameterValue => CompletionItemKind.Value, - CompletionType.Property => CompletionItemKind.Property, - CompletionType.Variable => CompletionItemKind.Variable, - CompletionType.Namespace => CompletionItemKind.Module, - CompletionType.Type => CompletionItemKind.TypeParameter, - CompletionType.Keyword => CompletionItemKind.Keyword, - CompletionType.File => CompletionItemKind.File, - CompletionType.Folder => CompletionItemKind.Folder, - CompletionType.History => CompletionItemKind.Reference, - CompletionType.Text => CompletionItemKind.Text, - _ => throw new ArgumentOutOfRangeException(nameof(completionType)), + }, + InsertTextFormat = insertTextFormat, + InsertText = completionText, + FilterText = completion.CompletionText, + SortText = sortText, + // TODO: Documentation + Detail = toolTipText, + Label = completion.ListItemText, + // TODO: Command }; } From d29d0ec142bed3faf7c85f884f66b8b8a73a5121 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Wed, 9 Mar 2022 16:13:08 -0800 Subject: [PATCH 3/7] Update `CompletionHandlerTests.cs` --- .../Completion/CompleteAttributeValue.cs | 86 +++++++++++--- .../Completion/CompleteCommandFromModule.cs | 54 +++++---- .../Completion/CompleteCommandInFile.cs | 46 +++++--- .../Completion/CompleteFilePath.cs | 36 +++--- .../Completion/CompleteNamespace.cs | 47 +++++--- .../Completion/CompleteTypeName.cs | 47 +++++--- .../Completion/CompleteVariableInFile.cs | 46 +++++--- .../Language/CompletionHandlerTests.cs | 106 ++++++------------ 8 files changed, 276 insertions(+), 192 deletions(-) diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs index 63cf69911..d4460ef67 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs @@ -2,26 +2,80 @@ // Licensed under the MIT License. using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteAttributeValue + internal static class CompleteAttributeValue { - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 16, - startColumnNumber: 38, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 16, + startColumnNumber: 38, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly BufferRange ExpectedRange = - new BufferRange( - new BufferPosition(16, 33), - new BufferPosition(16, 38)); + public static readonly CompletionItem ExpectedCompletion1 = new() + { + Kind = CompletionItemKind.Property, + Detail = "System.Boolean ValueFromPipeline", + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "ValueFromPipeline", + FilterText = "ValueFromPipeline", + Label = "ValueFromPipeline", + SortText = "0001ValueFromPipeline", + TextEdit = new TextEdit + { + NewText = "ValueFromPipeline", + Range = new Range + { + Start = new Position { Line = 15, Character = 32 }, + End = new Position { Line = 15, Character = 37 } + } + } + }; + + public static readonly CompletionItem ExpectedCompletion2 = new() + { + Kind = CompletionItemKind.Property, + Detail = "System.Boolean ValueFromPipelineByPropertyName", + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "ValueFromPipelineByPropertyName", + FilterText = "ValueFromPipelineByPropertyName", + Label = "ValueFromPipelineByPropertyName", + SortText = "0002ValueFromPipelineByPropertyName", + TextEdit = new TextEdit + { + NewText = "ValueFromPipelineByPropertyName", + Range = new Range + { + Start = new Position { Line = 15, Character = 32 }, + End = new Position { Line = 15, Character = 37 } + } + } + }; + + public static readonly CompletionItem ExpectedCompletion3 = new() + { + Kind = CompletionItemKind.Property, + Detail = "System.Boolean ValueFromRemainingArguments", + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "ValueFromRemainingArguments", + FilterText = "ValueFromRemainingArguments", + Label = "ValueFromRemainingArguments", + SortText = "0003ValueFromRemainingArguments", + TextEdit = new TextEdit + { + NewText = "ValueFromRemainingArguments", + Range = new Range + { + Start = new Position { Line = 15, Character = 32 }, + End = new Position { Line = 15, Character = 37 } + } + } + }; } } - diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs index e7b07ddb3..2736a96f8 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs @@ -3,33 +3,43 @@ using System; using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteCommandFromModule + internal static class CompleteCommandFromModule { - private static readonly string[] s_getRandomParamSets = { - "Get-Random [[-Maximum] ] [-SetSeed ] [-Minimum ] []", - "Get-Random [-InputObject] [-SetSeed ] [-Count ] []" - }; + public static readonly string GetRandomDetail = + "Get-Random [[-Maximum] ] [-SetSeed ] [-Minimum ]"; - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 13, - startColumnNumber: 8, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 13, + startColumnNumber: 8, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly CompletionDetails ExpectedCompletion = - CompletionDetails.Create( - "Get-Random", - CompletionType.Command, - string.Join(Environment.NewLine + Environment.NewLine, s_getRandomParamSets), - listItemText: "Get-Random" - ); + public static readonly CompletionItem ExpectedCompletion = new() + { + Kind = CompletionItemKind.Function, + Detail = "", // OS-dependent, checked separately. + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "Get-Random", + FilterText = "Get-Random", + Label = "Get-Random", + SortText = "0001Get-Random", + TextEdit = new TextEdit + { + NewText = "Get-Random", + Range = new Range + { + Start = new Position { Line = 12, Character = 0 }, + End = new Position { Line = 12, Character = 8 } + } + } + }; } } diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs index dbd06fc10..e0db02625 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs @@ -2,26 +2,40 @@ // Licensed under the MIT License. using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteCommandInFile + internal static class CompleteCommandInFile { - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 8, - startColumnNumber: 7, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 8, + startColumnNumber: 7, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly CompletionDetails ExpectedCompletion = - CompletionDetails.Create( - "Get-Something", - CompletionType.Command, - "Get-Something"); + public static readonly CompletionItem ExpectedCompletion = new() + { + Kind = CompletionItemKind.Function, + Detail = "", + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "Get-Something", + FilterText = "Get-Something", + Label = "Get-Something", + SortText = "0001Get-Something", + TextEdit = new TextEdit + { + NewText = "Get-Something", + Range = new Range + { + Start = new Position { Line = 7, Character = 0 }, + End = new Position { Line = 7, Character = 6 } + } + } + }; } } diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteFilePath.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteFilePath.cs index d821e3b6e..a3a899f7e 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteFilePath.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteFilePath.cs @@ -2,26 +2,30 @@ // Licensed under the MIT License. using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteFilePath + internal static class CompleteFilePath { - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 19, - startColumnNumber: 15, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 19, + startColumnNumber: 15, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly BufferRange ExpectedRange = - new BufferRange( - new BufferPosition(19, 15), - new BufferPosition(19, 25)); + public static readonly TextEdit ExpectedEdit = new() + { + NewText = "", + Range = new Range + { + Start = new Position { Line = 18, Character = 14 }, + End = new Position { Line = 18, Character = 14 } + } + }; } } - diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs index d1356898e..4477befc7 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs @@ -2,27 +2,40 @@ // Licensed under the MIT License. using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteNamespace + internal static class CompleteNamespace { - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 22, - startColumnNumber: 15, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 22, + startColumnNumber: 15, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly CompletionDetails ExpectedCompletion = - CompletionDetails.Create( - "System.Collections", - CompletionType.Namespace, - "System.Collections" - ); + public static readonly CompletionItem ExpectedCompletion = new() + { + Kind = CompletionItemKind.Module, + Detail = "Namespace System.Collections", + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "System.Collections", + FilterText = "System.Collections", + Label = "Collections", + SortText = "0001Collections", + TextEdit = new TextEdit + { + NewText = "System.Collections", + Range = new Range + { + Start = new Position { Line = 21, Character = 1 }, + End = new Position { Line = 21, Character = 15 } + } + } + }; } } diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs index 084411a76..432167f25 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs @@ -2,27 +2,40 @@ // Licensed under the MIT License. using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteTypeName + internal static class CompleteTypeName { - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 21, - startColumnNumber: 25, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 21, + startColumnNumber: 25, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly CompletionDetails ExpectedCompletion = - CompletionDetails.Create( - "System.Collections.ArrayList", - CompletionType.Type, - "System.Collections.ArrayList" - ); + public static readonly CompletionItem ExpectedCompletion = new() + { + Kind = CompletionItemKind.TypeParameter, + Detail = "System.Collections.ArrayList", + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "System.Collections.ArrayList", + FilterText = "System.Collections.ArrayList", + Label = "ArrayList", + SortText = "0001ArrayList", + TextEdit = new TextEdit + { + NewText = "System.Collections.ArrayList", + Range = new Range + { + Start = new Position { Line = 20, Character = 1 }, + End = new Position { Line = 20, Character = 29 } + } + } + }; } } diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs index 2654cd31e..34376e954 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs @@ -2,26 +2,40 @@ // Licensed under the MIT License. using Microsoft.PowerShell.EditorServices.Services.TextDocument; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; namespace Microsoft.PowerShell.EditorServices.Test.Shared.Completion { - internal class CompleteVariableInFile + internal static class CompleteVariableInFile { - public static readonly ScriptRegion SourceDetails = - new ScriptRegion( - file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), - text: string.Empty, - startLineNumber: 10, - startColumnNumber: 9, - startOffset: 0, - endLineNumber: 0, - endColumnNumber: 0, - endOffset: 0); + public static readonly ScriptRegion SourceDetails = new( + file: TestUtilities.NormalizePath("Completion/CompletionExamples.psm1"), + text: string.Empty, + startLineNumber: 10, + startColumnNumber: 9, + startOffset: 0, + endLineNumber: 0, + endColumnNumber: 0, + endOffset: 0); - public static readonly CompletionDetails ExpectedCompletion = - CompletionDetails.Create( - "$testVar1", - CompletionType.Variable, - "testVar1"); + public static readonly CompletionItem ExpectedCompletion = new() + { + Kind = CompletionItemKind.Variable, + Detail = "", // Same as label, so not shown. + InsertTextFormat = InsertTextFormat.PlainText, + InsertText = "$testVar1", + FilterText = "$testVar1", + Label = "testVar1", + SortText = "0001testVar1", + TextEdit = new TextEdit + { + NewText = "$testVar1", + Range = new Range + { + Start = new Position { Line = 9, Character = 0 }, + End = new Position { Line = 9, Character = 8 } + } + } + }; } } diff --git a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs index b83689cb3..f0257eb6f 100644 --- a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs +++ b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs @@ -2,6 +2,8 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; +using System.Linq; using System.Threading.Tasks; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.PowerShell.EditorServices.Handlers; @@ -11,6 +13,7 @@ using Microsoft.PowerShell.EditorServices.Test.Shared; using Microsoft.PowerShell.EditorServices.Test.Shared.Completion; using Microsoft.PowerShell.EditorServices.Utility; +using OmniSharp.Extensions.LanguageServer.Protocol.Models; using Xunit; namespace Microsoft.PowerShell.EditorServices.Test.Language @@ -31,44 +34,36 @@ public CompletionHandlerTests() public void Dispose() { - psesHost.StopAsync().GetAwaiter().GetResult(); + psesHost.StopAsync().Wait(); GC.SuppressFinalize(this); } private ScriptFile GetScriptFile(ScriptRegion scriptRegion) => workspace.GetFile(TestUtilities.GetSharedPath(scriptRegion.File)); - private async Task GetCompletionResults(ScriptRegion scriptRegion) + private Task> GetCompletionResultsAsync(ScriptRegion scriptRegion) { - return await completionHandler.GetCompletionsInFileAsync( + return completionHandler.GetCompletionsInFileAsync( GetScriptFile(scriptRegion), scriptRegion.StartLineNumber, - scriptRegion.StartColumnNumber).ConfigureAwait(true); + scriptRegion.StartColumnNumber); } [Fact] public async Task CompletesCommandInFile() { - CompletionResults completionResults = await GetCompletionResults(CompleteCommandInFile.SourceDetails).ConfigureAwait(true); - Assert.NotEmpty(completionResults.Completions); - Assert.Equal(CompleteCommandInFile.ExpectedCompletion, completionResults.Completions[0]); + IEnumerable results = await GetCompletionResultsAsync(CompleteCommandInFile.SourceDetails).ConfigureAwait(true); + CompletionItem actual = Assert.Single(results); + Assert.Equal(CompleteCommandInFile.ExpectedCompletion, actual); } [Fact] public async Task CompletesCommandFromModule() { - CompletionResults completionResults = await GetCompletionResults(CompleteCommandFromModule.SourceDetails).ConfigureAwait(true); - - Assert.NotEmpty(completionResults.Completions); - - Assert.Equal( - CompleteCommandFromModule.ExpectedCompletion.CompletionText, - completionResults.Completions[0].CompletionText); - - Assert.Equal( - CompleteCommandFromModule.ExpectedCompletion.CompletionType, - completionResults.Completions[0].CompletionType); - - Assert.NotNull(completionResults.Completions[0].ToolTipText); + IEnumerable results = await GetCompletionResultsAsync(CompleteCommandFromModule.SourceDetails).ConfigureAwait(true); + CompletionItem actual = Assert.Single(results); + // NOTE: The tooltip varies across PowerShell and OS versions, so we ignore it. + Assert.Equal(CompleteCommandFromModule.ExpectedCompletion, actual with { Detail = "" }); + Assert.StartsWith(CompleteCommandFromModule.GetRandomDetail, actual.Detail); } [SkippableFact] @@ -78,19 +73,9 @@ public async Task CompletesTypeName() !VersionUtils.IsNetCore, "In Windows PowerShell the CommandCompletion fails in the test harness, but works manually."); - CompletionResults completionResults = await GetCompletionResults(CompleteTypeName.SourceDetails).ConfigureAwait(true); - - Assert.NotEmpty(completionResults.Completions); - - Assert.Equal( - CompleteTypeName.ExpectedCompletion.CompletionText, - completionResults.Completions[0].CompletionText); - - Assert.Equal( - CompleteTypeName.ExpectedCompletion.CompletionType, - completionResults.Completions[0].CompletionType); - - Assert.NotNull(completionResults.Completions[0].ToolTipText); + IEnumerable results = await GetCompletionResultsAsync(CompleteTypeName.SourceDetails).ConfigureAwait(true); + CompletionItem actual = Assert.Single(results); + Assert.Equal(CompleteTypeName.ExpectedCompletion, actual); } [Trait("Category", "Completions")] @@ -101,60 +86,37 @@ public async Task CompletesNamespace() !VersionUtils.IsNetCore, "In Windows PowerShell the CommandCompletion fails in the test harness, but works manually."); - CompletionResults completionResults = await GetCompletionResults(CompleteNamespace.SourceDetails).ConfigureAwait(true); - - Assert.NotEmpty(completionResults.Completions); - - Assert.Equal( - CompleteNamespace.ExpectedCompletion.CompletionText, - completionResults.Completions[0].CompletionText); - - Assert.Equal( - CompleteNamespace.ExpectedCompletion.CompletionType, - completionResults.Completions[0].CompletionType); - - Assert.NotNull(completionResults.Completions[0].ToolTipText); + IEnumerable results = await GetCompletionResultsAsync(CompleteNamespace.SourceDetails).ConfigureAwait(true); + CompletionItem actual = Assert.Single(results); + Assert.Equal(CompleteNamespace.ExpectedCompletion, actual); } [Fact] public async Task CompletesVariableInFile() { - CompletionResults completionResults = await GetCompletionResults(CompleteVariableInFile.SourceDetails).ConfigureAwait(true); - - Assert.Single(completionResults.Completions); - - Assert.Equal( - CompleteVariableInFile.ExpectedCompletion, - completionResults.Completions[0]); + IEnumerable results = await GetCompletionResultsAsync(CompleteVariableInFile.SourceDetails).ConfigureAwait(true); + CompletionItem actual = Assert.Single(results); + Assert.Equal(CompleteVariableInFile.ExpectedCompletion, actual); } [Fact] public async Task CompletesAttributeValue() { - CompletionResults completionResults = await GetCompletionResults(CompleteAttributeValue.SourceDetails).ConfigureAwait(true); - - Assert.NotEmpty(completionResults.Completions); - - Assert.Equal( - CompleteAttributeValue.ExpectedRange, - completionResults.ReplacedRange); + IEnumerable results = await GetCompletionResultsAsync(CompleteAttributeValue.SourceDetails).ConfigureAwait(true); + Assert.Collection(results, + actual => Assert.Equal(actual, CompleteAttributeValue.ExpectedCompletion1), + acutal => Assert.Equal(acutal, CompleteAttributeValue.ExpectedCompletion2), + actual => Assert.Equal(actual, CompleteAttributeValue.ExpectedCompletion3)); } [Fact] public async Task CompletesFilePath() { - CompletionResults completionResults = await GetCompletionResults(CompleteFilePath.SourceDetails).ConfigureAwait(true); - - Assert.NotEmpty(completionResults.Completions); - - // TODO: Since this is a path completion, this test will need to be - // platform specific. Probably something like: - // - Windows: C:\Program - // - macOS: /User - // - Linux: /hom - //Assert.Equal( - // CompleteFilePath.ExpectedRange, - // completionResults.ReplacedRange); + IEnumerable results = await GetCompletionResultsAsync(CompleteFilePath.SourceDetails).ConfigureAwait(true); + Assert.NotEmpty(results); + CompletionItem actual = results.First(); + Assert.Equal(actual.TextEdit.TextEdit with { NewText = "" }, CompleteFilePath.ExpectedEdit); + Assert.All(results, r => Assert.True(r.Kind is CompletionItemKind.File or CompletionItemKind.Folder)); } } } From 7734d2918c1e41a43f82155f4ed188493d5e7685 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 10 Mar 2022 10:31:47 -0800 Subject: [PATCH 4/7] Convert `CreateCompletionItem` to use `switch` expression Makes it a thousand times more readable, and therefore less error-prone. Also verified fields against the spec and so finished the TODOs. --- .../Handlers/CompletionHandler.cs | 244 ++++++++---------- .../Completion/CompleteAttributeValue.cs | 6 - .../Completion/CompleteCommandFromModule.cs | 2 - .../Completion/CompleteCommandInFile.cs | 2 - .../Completion/CompleteNamespace.cs | 2 - .../Completion/CompleteTypeName.cs | 2 - .../Completion/CompleteVariableInFile.cs | 2 - .../Language/CompletionHandlerTests.cs | 1 + 8 files changed, 107 insertions(+), 154 deletions(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 483402ccb..dc951b555 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -32,7 +32,6 @@ internal class PsesCompletionHandler : ICompletionHandler, ICompletionResolveHan private readonly WorkspaceService _workspaceService; private CompletionCapability _capability; private readonly Guid _id = Guid.NewGuid(); - private static readonly Regex _typeRegex = new(@"^(\[.+\])"); Guid ICanBeIdentifiedHandler.Id => _id; @@ -168,163 +167,132 @@ public async Task> GetCompletionsInFileAsync( } internal static CompletionItem CreateCompletionItem( - CompletionResult completion, + CompletionResult result, BufferRange completionRange, int sortIndex) { - Validate.IsNotNull(nameof(completion), completion); + Validate.IsNotNull(nameof(result), result); - // Some tooltips may have newlines or whitespace for unknown reasons. - string toolTipText = completion.ToolTip?.Trim(); + TextEdit textEdit = new() + { + NewText = result.CompletionText, + Range = new Range + { + Start = new Position + { + Line = completionRange.Start.Line - 1, + Character = completionRange.Start.Column - 1 + }, + End = new Position + { + Line = completionRange.End.Line - 1, + Character = completionRange.End.Column - 1 + } + } + }; - string completionText = completion.CompletionText; - InsertTextFormat insertTextFormat = InsertTextFormat.PlainText; - CompletionItemKind kind; + // Some tooltips may have newlines or whitespace for unknown reasons. + string detail = result.ToolTip?.Trim(); - // Force the client to maintain the sort order in which the original completion results - // were returned. We just need to make sure the default order also be the - // lexicographical order which we do by prefixing the ListItemText with a leading 0's - // four digit index. - string sortText = $"{sortIndex:D4}{completion.ListItemText}"; + CompletionItem item = new() + { + Label = result.ListItemText, + Detail = result.ListItemText.Equals(detail, StringComparison.CurrentCulture) + ? string.Empty : detail, // Don't repeat label. + // Retain PowerShell's sort order with the given index. + SortText = $"{sortIndex:D4}{result.ListItemText}", + FilterText = result.CompletionText, + TextEdit = textEdit // Used instead of InsertText. + }; - switch (completion.ResultType) + return result.ResultType switch { - case CompletionResultType.Command: - kind = CompletionItemKind.Function; - break; - case CompletionResultType.History: - kind = CompletionItemKind.Reference; - break; - case CompletionResultType.Keyword: - case CompletionResultType.DynamicKeyword: - kind = CompletionItemKind.Keyword; - break; - case CompletionResultType.Method: - kind = CompletionItemKind.Method; - break; - case CompletionResultType.Namespace: - kind = CompletionItemKind.Module; - break; - case CompletionResultType.ParameterName: - kind = CompletionItemKind.Variable; - // Look for type encoded in the tooltip for parameters and variables. - // Display PowerShell type names in [] to be consistent with PowerShell syntax - // and how the debugger displays type names. - MatchCollection matches = _typeRegex.Matches(toolTipText); - if ((matches.Count > 0) && (matches[0].Groups.Count > 1)) + 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 { - toolTipText = matches[0].Groups[1].Value; + 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.ParameterName => TryExtractType(detail, out string type) + ? item with { Kind = CompletionItemKind.Variable, Detail = type } // The comparison operators (-eq, -not, -gt, etc) unfortunately come across as // ParameterName types but they don't have a type associated to them, so we can - // deduce its an operator. - else - { - kind = CompletionItemKind.Operator; - } - break; - case CompletionResultType.ParameterValue: - kind = CompletionItemKind.Value; - break; - case CompletionResultType.Property: - kind = CompletionItemKind.Property; - break; - case CompletionResultType.ProviderContainer: - kind = CompletionItemKind.Folder; - // Insert a final "tab stop" as identified by $0 in the snippet provided for - // completion. For folder paths, we take the path returned by PowerShell e.g. - // 'C:\Program Files' and insert the tab stop marker before the closing quote - // char e.g. 'C:\Program Files$0'. This causes the editing cursor to be placed - // *before* the final quote after completion, which makes subsequent path - // completions work. See this part of the LSP spec for details: - // https://microsoft.github.io/language-server-protocol/specification#textDocument_completion - - // Since we want to use a "tab stop" we need to escape a few things for Textmate - // to render properly. - if (EndsWithQuote(completionText)) - { - StringBuilder sb = new StringBuilder(completionText) - .Replace(@"\", @"\\") - .Replace(@"}", @"\}") - .Replace(@"$", @"\$"); - completionText = sb.Insert(sb.Length - 1, "$0").ToString(); - insertTextFormat = InsertTextFormat.Snippet; - } - break; - case CompletionResultType.ProviderItem: - kind = CompletionItemKind.File; - break; - case CompletionResultType.Text: - kind = CompletionItemKind.Text; - break; - case CompletionResultType.Type: - kind = CompletionItemKind.TypeParameter; + // deduce it is an operator. + : item with { Kind = CompletionItemKind.Operator }, + CompletionResultType.ParameterValue => item with { Kind = CompletionItemKind.Value }, + CompletionResultType.Variable => TryExtractType(detail, out string type) + ? item with { Kind = CompletionItemKind.Variable, Detail = type } + : item with { Kind = CompletionItemKind.Variable }, + CompletionResultType.Namespace => item with { Kind = CompletionItemKind.Module }, + 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. - if (toolTipText.StartsWith("Class ", StringComparison.Ordinal)) - { - kind = CompletionItemKind.Class; - } - break; - case CompletionResultType.Variable: - kind = CompletionItemKind.Variable; - // Look for type encoded in the tooltip for parameters and variables. - // Display PowerShell type names in [] to be consistent with PowerShell syntax - // and how the debugger displays type names. - matches = _typeRegex.Matches(toolTipText); - if ((matches.Count > 0) && (matches[0].Groups.Count > 1)) - { - toolTipText = matches[0].Groups[1].Value; - } - break; - default: - throw new ArgumentOutOfRangeException(nameof(completion)); - } - - // Don't display tooltip if it is the same as the ListItemText. - if (completion.ListItemText.Equals(toolTipText, StringComparison.OrdinalIgnoreCase)) - { - toolTipText = string.Empty; - } + ? item with { Kind = CompletionItemKind.Class } + : item with { Kind = CompletionItemKind.TypeParameter }, + CompletionResultType.Keyword or CompletionResultType.DynamicKeyword => + item with { Kind = CompletionItemKind.Keyword }, + _ => throw new ArgumentOutOfRangeException(nameof(result)) + }; + } - Validate.IsNotNull(nameof(CompletionItemKind), kind); + private static readonly Regex s_typeRegex = new(@"^(\[.+\])", RegexOptions.Compiled); - // TODO: We used to extract the symbol type from the tooltip using a regex, but it - // wasn't actually used. - return new CompletionItem + /// + /// Look for type encoded in the tooltip for parameters and variables. Display PowerShell + /// type names in [] to be consistent with PowerShell syntax and how the debugger displays + /// type names. + /// + /// + /// + /// Whether or not the type was found. + private static bool TryExtractType(string toolTipText, out string type) + { + MatchCollection matches = s_typeRegex.Matches(toolTipText); + type = string.Empty; + if ((matches.Count > 0) && (matches[0].Groups.Count > 1)) { - Kind = kind, - TextEdit = new TextEdit - { - NewText = completionText, - Range = new Range - { - Start = new Position - { - Line = completionRange.Start.Line - 1, - Character = completionRange.Start.Column - 1 - }, - End = new Position - { - Line = completionRange.End.Line - 1, - Character = completionRange.End.Column - 1 - } - } - }, - InsertTextFormat = insertTextFormat, - InsertText = completionText, - FilterText = completion.CompletionText, - SortText = sortText, - // TODO: Documentation - Detail = toolTipText, - Label = completion.ListItemText, - // TODO: Command - }; + type = matches[0].Groups[1].Value; + return true; + } + return false; } - private static bool EndsWithQuote(string text) + /// + /// Insert a final "tab stop" as identified by $0 in the snippet provided for completion. + /// For folder paths, we take the path returned by PowerShell e.g. 'C:\Program Files' and + /// insert the tab stop marker before the closing quote char e.g. 'C:\Program Files$0'. This + /// causes the editing cursor to be placed *before* the final quote after completion, which + /// makes subsequent path completions work. See this part of the LSP spec for details: + /// https://microsoft.github.io/language-server-protocol/specification#textDocument_completion + /// + /// + /// + /// + /// Whether or not the completion ended with a quote and so was a snippet. + /// + private static bool TryBuildSnippet(string completionText, out string snippet) { - return !string.IsNullOrEmpty(text) && text[text.Length - 1] is '"' or '\''; + snippet = string.Empty; + if (!string.IsNullOrEmpty(completionText) + && completionText[completionText.Length - 1] is '"' or '\'') + { + // Since we want to use a "tab stop" we need to escape a few things. + StringBuilder sb = new StringBuilder(completionText) + .Replace(@"\", @"\\") + .Replace(@"}", @"\}") + .Replace(@"$", @"\$"); + snippet = sb.Insert(sb.Length - 1, "$0").ToString(); + return true; + } + return false; } } } diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs index d4460ef67..317cc6277 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteAttributeValue.cs @@ -22,8 +22,6 @@ internal static class CompleteAttributeValue { Kind = CompletionItemKind.Property, Detail = "System.Boolean ValueFromPipeline", - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "ValueFromPipeline", FilterText = "ValueFromPipeline", Label = "ValueFromPipeline", SortText = "0001ValueFromPipeline", @@ -42,8 +40,6 @@ internal static class CompleteAttributeValue { Kind = CompletionItemKind.Property, Detail = "System.Boolean ValueFromPipelineByPropertyName", - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "ValueFromPipelineByPropertyName", FilterText = "ValueFromPipelineByPropertyName", Label = "ValueFromPipelineByPropertyName", SortText = "0002ValueFromPipelineByPropertyName", @@ -62,8 +58,6 @@ internal static class CompleteAttributeValue { Kind = CompletionItemKind.Property, Detail = "System.Boolean ValueFromRemainingArguments", - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "ValueFromRemainingArguments", FilterText = "ValueFromRemainingArguments", Label = "ValueFromRemainingArguments", SortText = "0003ValueFromRemainingArguments", diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs index 2736a96f8..184e1699a 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandFromModule.cs @@ -26,8 +26,6 @@ internal static class CompleteCommandFromModule { Kind = CompletionItemKind.Function, Detail = "", // OS-dependent, checked separately. - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "Get-Random", FilterText = "Get-Random", Label = "Get-Random", SortText = "0001Get-Random", diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs index e0db02625..cf8bb6855 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteCommandInFile.cs @@ -22,8 +22,6 @@ internal static class CompleteCommandInFile { Kind = CompletionItemKind.Function, Detail = "", - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "Get-Something", FilterText = "Get-Something", Label = "Get-Something", SortText = "0001Get-Something", diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs index 4477befc7..66c111871 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteNamespace.cs @@ -22,8 +22,6 @@ internal static class CompleteNamespace { Kind = CompletionItemKind.Module, Detail = "Namespace System.Collections", - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "System.Collections", FilterText = "System.Collections", Label = "Collections", SortText = "0001Collections", diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs index 432167f25..867f067b4 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteTypeName.cs @@ -22,8 +22,6 @@ internal static class CompleteTypeName { Kind = CompletionItemKind.TypeParameter, Detail = "System.Collections.ArrayList", - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "System.Collections.ArrayList", FilterText = "System.Collections.ArrayList", Label = "ArrayList", SortText = "0001ArrayList", diff --git a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs index 34376e954..394962379 100644 --- a/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs +++ b/test/PowerShellEditorServices.Test.Shared/Completion/CompleteVariableInFile.cs @@ -22,8 +22,6 @@ internal static class CompleteVariableInFile { Kind = CompletionItemKind.Variable, Detail = "", // Same as label, so not shown. - InsertTextFormat = InsertTextFormat.PlainText, - InsertText = "$testVar1", FilterText = "$testVar1", Label = "testVar1", SortText = "0001testVar1", diff --git a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs index f0257eb6f..f159740a1 100644 --- a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs +++ b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs @@ -115,6 +115,7 @@ public async Task CompletesFilePath() IEnumerable results = await GetCompletionResultsAsync(CompleteFilePath.SourceDetails).ConfigureAwait(true); Assert.NotEmpty(results); CompletionItem actual = results.First(); + // Paths are system dependent so we ignore the text and just check the type and range. Assert.Equal(actual.TextEdit.TextEdit with { NewText = "" }, CompleteFilePath.ExpectedEdit); Assert.All(results, r => Assert.True(r.Kind is CompletionItemKind.File or CompletionItemKind.Folder)); } From d2038e8603e77aefeeff8959f4949018cbfc0f8b Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 10 Mar 2022 10:46:51 -0800 Subject: [PATCH 5/7] Make `completionItem/resolve` cancellable --- .../PowerShell/Utility/CommandHelpers.cs | 10 ++++--- .../Handlers/CompletionHandler.cs | 26 ++++++++++--------- 2 files changed, 20 insertions(+), 16 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs index fffb0c1c9..d65503167 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs @@ -64,7 +64,8 @@ internal static class CommandHelpers public static async Task GetCommandInfoAsync( string commandName, IRunspaceInfo currentRunspace, - IInternalPowerShellExecutionService executionService) + IInternalPowerShellExecutionService executionService, + CancellationToken cancellationToken = default) { // This mechanism only works in-process if (currentRunspace.RunspaceOrigin != RunspaceOrigin.Local) @@ -98,7 +99,7 @@ public static async Task GetCommandInfoAsync( .AddParameter("ErrorAction", "Ignore"); IReadOnlyList results = await executionService - .ExecutePSCommandAsync(command, CancellationToken.None) + .ExecutePSCommandAsync(command, cancellationToken) .ConfigureAwait(false); CommandInfo commandInfo = results.Count > 0 ? results[0] : null; @@ -120,7 +121,8 @@ public static async Task GetCommandInfoAsync( /// The synopsis. public static async Task GetCommandSynopsisAsync( CommandInfo commandInfo, - IInternalPowerShellExecutionService executionService) + IInternalPowerShellExecutionService executionService, + CancellationToken cancellationToken = default) { Validate.IsNotNull(nameof(commandInfo), commandInfo); Validate.IsNotNull(nameof(executionService), executionService); @@ -151,7 +153,7 @@ public static async Task GetCommandSynopsisAsync( .AddParameter("ErrorAction", "Ignore"); IReadOnlyList results = await executionService - .ExecutePSCommandAsync(command, CancellationToken.None) + .ExecutePSCommandAsync(command, cancellationToken) .ConfigureAwait(false); // Extract the synopsis string from the object diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index dc951b555..11219b7ea 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -80,30 +80,32 @@ public static bool CanResolve(CompletionItem value) // Handler for "completionItem/resolve". In VSCode this is fired when a completion item is highlighted in the completion list. public async Task Handle(CompletionItem request, CancellationToken cancellationToken) { - // We currently only support this request for anything that returns a CommandInfo: functions, cmdlets, aliases. - if (request.Kind != CompletionItemKind.Function) - { - return request; - } - - // No details means the module hasn't been imported yet and Intellisense shouldn't import the module to get this info. - if (request.Detail is null) + // 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. + if (request.Kind is not CompletionItemKind.Function || request.Detail is null) { return request; } // Get the documentation for the function - CommandInfo commandInfo = await CommandHelpers.GetCommandInfoAsync(request.Label, _runspaceContext.CurrentRunspace, _executionService).ConfigureAwait(false); + CommandInfo commandInfo = await CommandHelpers.GetCommandInfoAsync( + request.Label, + _runspaceContext.CurrentRunspace, + _executionService, + cancellationToken).ConfigureAwait(false); if (commandInfo is not null) { - request = request with + return request with { - Documentation = await CommandHelpers.GetCommandSynopsisAsync(commandInfo, _executionService).ConfigureAwait(false) + Documentation = await CommandHelpers.GetCommandSynopsisAsync( + commandInfo, + _executionService, + cancellationToken).ConfigureAwait(false) }; } - // Send back the updated CompletionItem return request; } From 7fdef2089d763c40889e6abac26a7c4ef9e24c96 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 10 Mar 2022 12:04:31 -0800 Subject: [PATCH 6/7] Enable skipped Windows PowerShell completion tests --- .../Language/CompletionHandlerTests.cs | 26 +++++++++++-------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs index f159740a1..58aa40a61 100644 --- a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs +++ b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs @@ -66,26 +66,30 @@ public async Task CompletesCommandFromModule() Assert.StartsWith(CompleteCommandFromModule.GetRandomDetail, actual.Detail); } - [SkippableFact] + [Fact] public async Task CompletesTypeName() { - Skip.If( - !VersionUtils.IsNetCore, - "In Windows PowerShell the CommandCompletion fails in the test harness, but works manually."); - IEnumerable results = await GetCompletionResultsAsync(CompleteTypeName.SourceDetails).ConfigureAwait(true); CompletionItem actual = Assert.Single(results); - Assert.Equal(CompleteTypeName.ExpectedCompletion, actual); + if (VersionUtils.IsNetCore) + { + Assert.Equal(CompleteTypeName.ExpectedCompletion, actual); + } + else + { + // Windows PowerShell shows ArrayList as a Class. + Assert.Equal(CompleteTypeName.ExpectedCompletion with + { + Kind = CompletionItemKind.Class, + Detail = "Class System.Collections.ArrayList" + }, actual); + } } [Trait("Category", "Completions")] - [SkippableFact] + [Fact] public async Task CompletesNamespace() { - Skip.If( - !VersionUtils.IsNetCore, - "In Windows PowerShell the CommandCompletion fails in the test harness, but works manually."); - IEnumerable results = await GetCompletionResultsAsync(CompleteNamespace.SourceDetails).ConfigureAwait(true); CompletionItem actual = Assert.Single(results); Assert.Equal(CompleteNamespace.ExpectedCompletion, actual); From 912392e81efb6b12d245747349203b1725f8d523 Mon Sep 17 00:00:00 2001 From: Andrew Schwartzmeyer Date: Thu, 10 Mar 2022 12:15:02 -0800 Subject: [PATCH 7/7] Replace 5 second cancellation token with handler's token --- .../Handlers/CompletionHandler.cs | 35 ++++++++----------- .../Language/CompletionHandlerTests.cs | 4 ++- 2 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 11219b7ea..bd3a45713 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -25,7 +25,6 @@ namespace Microsoft.PowerShell.EditorServices.Handlers // TODO: Use ABCs. internal class PsesCompletionHandler : ICompletionHandler, ICompletionResolveHandler { - private const int DefaultWaitTimeoutMilliseconds = 5000; private readonly ILogger _logger; private readonly IRunspaceContext _runspaceContext; private readonly IInternalPowerShellExecutionService _executionService; @@ -60,14 +59,11 @@ public async Task Handle(CompletionParams request, CancellationT int cursorColumn = request.Position.Character + 1; ScriptFile scriptFile = _workspaceService.GetFile(request.TextDocument.Uri); - - if (cancellationToken.IsCancellationRequested) - { - _logger.LogDebug("Completion request canceled for file: {0}", request.TextDocument.Uri); - return Array.Empty(); - } - - IEnumerable completionResults = await GetCompletionsInFileAsync(scriptFile, cursorLine, cursorColumn).ConfigureAwait(false); + IEnumerable completionResults = await GetCompletionsInFileAsync( + scriptFile, + cursorLine, + cursorColumn, + cancellationToken).ConfigureAwait(false); return new CompletionList(completionResults); } @@ -133,21 +129,18 @@ public void SetCapability(CompletionCapability capability, ClientCapabilities cl public async Task> GetCompletionsInFileAsync( ScriptFile scriptFile, int lineNumber, - int columnNumber) + int columnNumber, + CancellationToken cancellationToken) { Validate.IsNotNull(nameof(scriptFile), scriptFile); - CommandCompletion result = null; - using (CancellationTokenSource cts = new(DefaultWaitTimeoutMilliseconds)) - { - result = await AstOperations.GetCompletionsAsync( - scriptFile.ScriptAst, - scriptFile.ScriptTokens, - scriptFile.GetOffsetAtPosition(lineNumber, columnNumber), - _executionService, - _logger, - cts.Token).ConfigureAwait(false); - } + CommandCompletion result = await AstOperations.GetCompletionsAsync( + scriptFile.ScriptAst, + scriptFile.ScriptTokens, + scriptFile.GetOffsetAtPosition(lineNumber, columnNumber), + _executionService, + _logger, + cancellationToken).ConfigureAwait(false); // Only calculate the replacement range if there are completions. BufferRange replacedRange = new(0, 0, 0, 0); diff --git a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs index 58aa40a61..9681b7dc6 100644 --- a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs +++ b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.PowerShell.EditorServices.Handlers; @@ -45,7 +46,8 @@ private Task> GetCompletionResultsAsync(ScriptRegion return completionHandler.GetCompletionsInFileAsync( GetScriptFile(scriptRegion), scriptRegion.StartLineNumber, - scriptRegion.StartColumnNumber); + scriptRegion.StartColumnNumber, + CancellationToken.None); } [Fact]