From 5df33bb948565c72fe668544a6148d6accccfe74 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 12 May 2022 13:50:05 -0400 Subject: [PATCH 1/8] Mark completion requests as Serial This stops completion from being cancelled whenever a DidChangeTextDocument notification is sent. Also lets completion cancel some other expensive requests like code lens. --- .../Server/PsesLanguageServer.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs index 204ae88d9..c3fc81bd1 100644 --- a/src/PowerShellEditorServices/Server/PsesLanguageServer.cs +++ b/src/PowerShellEditorServices/Server/PsesLanguageServer.cs @@ -12,6 +12,7 @@ using Microsoft.PowerShell.EditorServices.Services.Extension; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; using Microsoft.PowerShell.EditorServices.Services.Template; +using OmniSharp.Extensions.JsonRpc; using OmniSharp.Extensions.LanguageServer.Protocol.Server; using OmniSharp.Extensions.LanguageServer.Server; using Serilog; @@ -102,7 +103,14 @@ public async Task StartAsync() .WithHandler() .WithHandler() .WithHandler() - .WithHandler() + // If PsesCompletionHandler is not marked as serial, then DidChangeTextDocument + // notifications will end up cancelling completion. So quickly typing `Get-` + // would result in no completions. + // + // This also lets completion requests interrupt time consuming background tasks + // like the references code lens. + .WithHandler( + new JsonRpcHandlerOptions() { RequestProcessType = RequestProcessType.Serial }) .WithHandler() .WithHandler() .WithHandler() From a991bebd1bcfa24e94adedc16ac680200c80357d Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 12 May 2022 14:39:28 -0400 Subject: [PATCH 2/8] Fix tasks never completing if cancelled quickly We were exiting SynchronousTask.ExecuteSynchronously early if the cancellation was already requested. This was not setting our tcs state correctly, causing the caller to just fall off without warning. --- .../Services/PowerShell/Execution/SynchronousTask.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousTask.cs b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousTask.cs index a3554fa7e..900ba4b1e 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousTask.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Execution/SynchronousTask.cs @@ -35,7 +35,7 @@ protected SynchronousTask( CancellationToken cancellationToken) { Logger = logger; - _taskCompletionSource = new TaskCompletionSource(); + _taskCompletionSource = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); _taskRequesterCancellationToken = cancellationToken; _executionCanceled = false; } @@ -76,6 +76,7 @@ public void ExecuteSynchronously(CancellationToken executorCancellationToken) { if (IsCanceled) { + SetCanceled(); return; } From 591c77e2d2c8ea05d14ca7b29c49c87ceb219a2f Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 12 May 2022 14:41:52 -0400 Subject: [PATCH 3/8] Make alias discovery possible on idle The use of `Runspace.SessionStateProxy` while the pipeline thread is busy (even if you are **on** the pipeline thread) results in an exception. This also allows us to cancel `GetAliasesAsync`. --- .../PowerShell/Utility/CommandHelpers.cs | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs b/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs index 67547a0e1..560a6524f 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Utility/CommandHelpers.cs @@ -16,6 +16,10 @@ namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility /// internal static class CommandHelpers { + public record struct AliasMap( + Dictionary> CmdletToAliases, + Dictionary AliasToCmdlets); + private static readonly HashSet s_nounExclusionList = new() { // PowerShellGet v2 nouns @@ -180,19 +184,21 @@ not CommandTypes.Function and /// Gets all aliases found in the runspace /// /// - public static async Task<(Dictionary>, Dictionary)> GetAliasesAsync(IInternalPowerShellExecutionService executionService) + /// + public static async Task GetAliasesAsync( + IInternalPowerShellExecutionService executionService, + CancellationToken cancellationToken = default) { Validate.IsNotNull(nameof(executionService), executionService); - IEnumerable aliases = await executionService.ExecuteDelegateAsync( - nameof(GetAliasesAsync), - executionOptions: null, - (pwsh, _) => - { - CommandInvocationIntrinsics invokeCommand = pwsh.Runspace.SessionStateProxy.InvokeCommand; - return invokeCommand.GetCommands("*", CommandTypes.Alias, nameIsPattern: true); - }, - CancellationToken.None).ConfigureAwait(false); + // Need to execute a PSCommand here as Runspace.SessionStateProxy cannot be used from + // our PSRL on idle handler. + IReadOnlyList aliases = await executionService.ExecutePSCommandAsync( + new PSCommand() + .AddCommand("Microsoft.PowerShell.Core\\Get-Command") + .AddParameter("ListImported", true) + .AddParameter("CommandType", CommandTypes.Alias), + cancellationToken).ConfigureAwait(false); foreach (AliasInfo aliasInfo in aliases) { @@ -206,7 +212,8 @@ not CommandTypes.Function and s_aliasToCmdletCache.TryAdd(aliasInfo.Name, aliasInfo.Definition); } - return (new Dictionary>(s_cmdletToAliasCache), + return new AliasMap( + new Dictionary>(s_cmdletToAliasCache), new Dictionary(s_aliasToCmdletCache)); } } From bdb8343feb4eff8eb2c28381f5fccb12eedb81cd Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 12 May 2022 14:45:06 -0400 Subject: [PATCH 4/8] Make a lot more things cancellable - Cancellation tokens are now propagated to all code lens providers. - Provider code that was heavy in synchronous code has some cancellation checks and `Task.Yield` calls to help keep thread pool threads more flexible. --- .../Services/CodeLens/ICodeLensProvider.cs | 8 +++-- .../CodeLens/PesterCodeLensProvider.cs | 8 +++-- .../CodeLens/ReferencesCodeLensProvider.cs | 30 +++++++++++++------ .../Services/Symbols/SymbolsService.cs | 19 ++++++++++-- .../TextDocument/Handlers/CodeLensHandlers.cs | 13 ++++---- .../Handlers/ReferencesHandler.cs | 3 +- .../Handlers/WorkspaceSymbolsHandler.cs | 9 ++++-- 7 files changed, 65 insertions(+), 25 deletions(-) diff --git a/src/PowerShellEditorServices/Services/CodeLens/ICodeLensProvider.cs b/src/PowerShellEditorServices/Services/CodeLens/ICodeLensProvider.cs index ff4ba33e1..a6255a41d 100644 --- a/src/PowerShellEditorServices/Services/CodeLens/ICodeLensProvider.cs +++ b/src/PowerShellEditorServices/Services/CodeLens/ICodeLensProvider.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Threading; using System.Threading.Tasks; using Microsoft.PowerShell.EditorServices.Services.TextDocument; using OmniSharp.Extensions.LanguageServer.Protocol.Models; @@ -25,8 +26,9 @@ internal interface ICodeLensProvider /// /// The document for which CodeLenses should be provided. /// + /// /// An array of CodeLenses. - CodeLens[] ProvideCodeLenses(ScriptFile scriptFile); + CodeLens[] ProvideCodeLenses(ScriptFile scriptFile, CancellationToken cancellationToken); /// /// Resolves a CodeLens that was created without a Command. @@ -37,11 +39,13 @@ internal interface ICodeLensProvider /// /// The ScriptFile to resolve it in (sometimes unused). /// + /// /// /// A Task which returns the resolved CodeLens when completed. /// Task ResolveCodeLens( CodeLens codeLens, - ScriptFile scriptFile); + ScriptFile scriptFile, + CancellationToken cancellationToken); } } diff --git a/src/PowerShellEditorServices/Services/CodeLens/PesterCodeLensProvider.cs b/src/PowerShellEditorServices/Services/CodeLens/PesterCodeLensProvider.cs index c37a1fa0a..386fb83a8 100644 --- a/src/PowerShellEditorServices/Services/CodeLens/PesterCodeLensProvider.cs +++ b/src/PowerShellEditorServices/Services/CodeLens/PesterCodeLensProvider.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; using System.Collections.Generic; +using System.Threading; using System.Threading.Tasks; using Microsoft.PowerShell.EditorServices.Services; using Microsoft.PowerShell.EditorServices.Services.Symbols; @@ -99,8 +100,9 @@ private static CodeLens[] GetPesterLens(PesterSymbolReference pesterSymbol, Scri /// Get all Pester CodeLenses for a given script file. /// /// The script file to get Pester CodeLenses for. + /// /// All Pester CodeLenses for the given script file. - public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) + public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile, CancellationToken cancellationToken) { // Don't return anything if codelens setting is disabled if (!_configurationService.CurrentSettings.Pester.CodeLens) @@ -116,6 +118,7 @@ public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) continue; } + cancellationToken.ThrowIfCancellationRequested(); if (_configurationService.CurrentSettings.Pester.UseLegacyCodeLens && pesterSymbol.Command != PesterCommandType.Describe) { @@ -133,8 +136,9 @@ public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) /// /// The code lens to resolve. /// The script file. + /// /// The given CodeLens, wrapped in a task. - public Task ResolveCodeLens(CodeLens codeLens, ScriptFile scriptFile) => + public Task ResolveCodeLens(CodeLens codeLens, ScriptFile scriptFile, CancellationToken cancellationToken) => // This provider has no specific behavior for // resolving CodeLenses. Task.FromResult(codeLens); diff --git a/src/PowerShellEditorServices/Services/CodeLens/ReferencesCodeLensProvider.cs b/src/PowerShellEditorServices/Services/CodeLens/ReferencesCodeLensProvider.cs index 099591e5d..c55613178 100644 --- a/src/PowerShellEditorServices/Services/CodeLens/ReferencesCodeLensProvider.cs +++ b/src/PowerShellEditorServices/Services/CodeLens/ReferencesCodeLensProvider.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Text; +using System.Threading; using System.Threading.Tasks; using Microsoft.PowerShell.EditorServices.Services; using Microsoft.PowerShell.EditorServices.Services.Symbols; @@ -53,12 +54,14 @@ public ReferencesCodeLensProvider(WorkspaceService workspaceService, SymbolsServ /// Get all reference code lenses for a given script file. /// /// The PowerShell script file to get code lenses for. + /// /// An array of CodeLenses describing all functions in the given script file. - public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) + public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile, CancellationToken cancellationToken) { List acc = new(); foreach (SymbolReference sym in _symbolProvider.ProvideDocumentSymbols(scriptFile)) { + cancellationToken.ThrowIfCancellationRequested(); if (sym.SymbolType == SymbolType.Function) { acc.Add(new CodeLens @@ -68,7 +71,7 @@ public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) Uri = scriptFile.DocumentUri, ProviderId = nameof(ReferencesCodeLensProvider) }, LspSerializer.Instance.JsonSerializer), - Range = sym.ScriptRegion.ToRange() + Range = sym.ScriptRegion.ToRange(), }); } } @@ -81,8 +84,12 @@ public CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) /// /// The old code lens to get updated references for. /// + /// /// A new code lens object describing the same data as the old one but with updated references. - public async Task ResolveCodeLens(CodeLens codeLens, ScriptFile scriptFile) + public async Task ResolveCodeLens( + CodeLens codeLens, + ScriptFile scriptFile, + CancellationToken cancellationToken) { ScriptFile[] references = _workspaceService.ExpandScriptReferences( scriptFile); @@ -93,9 +100,10 @@ public async Task ResolveCodeLens(CodeLens codeLens, ScriptFile script codeLens.Range.Start.Character + 1); List referencesResult = await _symbolsService.FindReferencesOfSymbol( - foundSymbol, - references, - _workspaceService).ConfigureAwait(false); + foundSymbol, + references, + _workspaceService, + cancellationToken).ConfigureAwait(false); Location[] referenceLocations; if (referencesResult == null) @@ -107,6 +115,10 @@ public async Task ResolveCodeLens(CodeLens codeLens, ScriptFile script List acc = new(); foreach (SymbolReference foundReference in referencesResult) { + // This async method is pretty dense with synchronous code + // so it's helpful to add some yields. + await Task.Yield(); + cancellationToken.ThrowIfCancellationRequested(); if (IsReferenceDefinition(foundSymbol, foundReference)) { continue; @@ -140,9 +152,9 @@ public async Task ResolveCodeLens(CodeLens codeLens, ScriptFile script Title = GetReferenceCountHeader(referenceLocations.Length), Arguments = JArray.FromObject(new object[] { - scriptFile.DocumentUri, - codeLens.Range.Start, - referenceLocations + scriptFile.DocumentUri, + codeLens.Range.Start, + referenceLocations }, LspSerializer.Instance.JsonSerializer) } diff --git a/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs b/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs index f80570b1c..34009a88d 100644 --- a/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs +++ b/src/PowerShellEditorServices/Services/Symbols/SymbolsService.cs @@ -11,6 +11,7 @@ using System.Management.Automation.Language; using System.Runtime.InteropServices; using System.Text.RegularExpressions; +using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.CodeLenses; @@ -160,18 +161,25 @@ public static SymbolReference FindSymbolAtLocation( /// The symbol to find all references for /// An array of scriptFiles too search for references in /// The workspace that will be searched for symbols + /// /// FindReferencesResult public async Task> FindReferencesOfSymbol( SymbolReference foundSymbol, ScriptFile[] referencedFiles, - WorkspaceService workspace) + WorkspaceService workspace, + CancellationToken cancellationToken = default) { if (foundSymbol == null) { return null; } - (Dictionary> cmdletToAliases, Dictionary aliasToCmdlets) = await CommandHelpers.GetAliasesAsync(_executionService).ConfigureAwait(false); + CommandHelpers.AliasMap aliases = await CommandHelpers.GetAliasesAsync( + _executionService, + cancellationToken).ConfigureAwait(false); + + Dictionary> cmdletToAliases = aliases.CmdletToAliases; + Dictionary aliasToCmdlets = aliases.AliasToCmdlets; // We want to look for references first in referenced files, hence we use ordered dictionary // TODO: File system case-sensitivity is based on filesystem not OS, but OS is a much cheaper heuristic @@ -188,6 +196,10 @@ public async Task> FindReferencesOfSymbol( { if (!fileMap.Contains(filePath)) { + // This async method is pretty dense with synchronous code + // so it's helpful to add some yields. + await Task.Yield(); + cancellationToken.ThrowIfCancellationRequested(); if (!workspace.TryGetFile(filePath, out ScriptFile scriptFile)) { // If we can't access the file for some reason, just ignore it @@ -223,6 +235,9 @@ public async Task> FindReferencesOfSymbol( reference.FilePath = file.FilePath; symbolReferences.Add(reference); } + + await Task.Yield(); + cancellationToken.ThrowIfCancellationRequested(); } return symbolReferences; diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs index 108dace6c..a83a440fc 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CodeLensHandlers.cs @@ -41,11 +41,11 @@ public PsesCodeLensHandlers(ILoggerFactory factory, SymbolsService symbolsServic public override Task Handle(CodeLensParams request, CancellationToken cancellationToken) { ScriptFile scriptFile = _workspaceService.GetFile(request.TextDocument.Uri); - CodeLens[] codeLensResults = ProvideCodeLenses(scriptFile); + CodeLens[] codeLensResults = ProvideCodeLenses(scriptFile, cancellationToken); return Task.FromResult(new CodeLensContainer(codeLensResults)); } - public override Task Handle(CodeLens request, CancellationToken cancellationToken) + public override async Task Handle(CodeLens request, CancellationToken cancellationToken) { // TODO: Catch deserializtion exception on bad object CodeLensData codeLensData = request.Data.ToObject(); @@ -55,18 +55,19 @@ public override Task Handle(CodeLens request, CancellationToken cancel .FirstOrDefault(provider => provider.ProviderId.Equals(codeLensData.ProviderId, StringComparison.Ordinal)); ScriptFile scriptFile = _workspaceService.GetFile(codeLensData.Uri); - - return originalProvider.ResolveCodeLens(request, scriptFile); + return await originalProvider.ResolveCodeLens(request, scriptFile, cancellationToken) + .ConfigureAwait(false); } /// /// Get all the CodeLenses for a given script file. /// /// The PowerShell script file to get CodeLenses for. + /// /// All generated CodeLenses for the given script file. - private CodeLens[] ProvideCodeLenses(ScriptFile scriptFile) + private CodeLens[] ProvideCodeLenses(ScriptFile scriptFile, CancellationToken cancellationToken) { - return InvokeProviders(provider => provider.ProvideCodeLenses(scriptFile)) + return InvokeProviders(provider => provider.ProvideCodeLenses(scriptFile, cancellationToken)) .SelectMany(codeLens => codeLens) .ToArray(); } diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/ReferencesHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/ReferencesHandler.cs index 7600341b9..d76aa7c66 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/ReferencesHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/ReferencesHandler.cs @@ -45,7 +45,8 @@ public override async Task Handle(ReferenceParams request, Ca await _symbolsService.FindReferencesOfSymbol( foundSymbol, _workspaceService.ExpandScriptReferences(scriptFile), - _workspaceService).ConfigureAwait(false); + _workspaceService, + cancellationToken).ConfigureAwait(false); List locations = new(); diff --git a/src/PowerShellEditorServices/Services/Workspace/Handlers/WorkspaceSymbolsHandler.cs b/src/PowerShellEditorServices/Services/Workspace/Handlers/WorkspaceSymbolsHandler.cs index ad36d25fc..04572f431 100644 --- a/src/PowerShellEditorServices/Services/Workspace/Handlers/WorkspaceSymbolsHandler.cs +++ b/src/PowerShellEditorServices/Services/Workspace/Handlers/WorkspaceSymbolsHandler.cs @@ -32,7 +32,7 @@ public PsesWorkspaceSymbolsHandler(ILoggerFactory loggerFactory, SymbolsService protected override WorkspaceSymbolRegistrationOptions CreateRegistrationOptions(WorkspaceSymbolCapability capability, ClientCapabilities clientCapabilities) => new() { }; - public override Task> Handle(WorkspaceSymbolParams request, CancellationToken cancellationToken) + public override async Task> Handle(WorkspaceSymbolParams request, CancellationToken cancellationToken) { List symbols = new(); @@ -47,6 +47,10 @@ public override Task> Handle(WorkspaceSymbolParams foreach (SymbolReference foundOccurrence in foundSymbols) { + // This async method is pretty dense with synchronous code + // so it's helpful to add some yields. + await Task.Yield(); + cancellationToken.ThrowIfCancellationRequested(); if (!IsQueryMatch(request.Query, foundOccurrence.SymbolName)) { continue; @@ -68,8 +72,7 @@ public override Task> Handle(WorkspaceSymbolParams } } _logger.LogWarning("Logging in a handler works now."); - - return Task.FromResult(new Container(symbols)); + return new Container(symbols); } #region private Methods From 687026f85fecd9d981e4979da934e15644cb6277 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 12 May 2022 14:49:31 -0400 Subject: [PATCH 5/8] Add space to completion trigger characters Adding space to completion triggers lets us automatically give completion results for parameter values. This also required building in some support for marking completion results as "Incomplete". This means that the client will not cache results when standard identifier characters are typed. Mainly, this solves the scenario where you type space, get file completion results, and then type `-`. You expect to then get parameter names, but without marking "Incomplete", it will instead filter the file results. --- .../Handlers/CompletionHandler.cs | 26 ++++++++++++++----- 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index 4bf174cac..e1dee04d3 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -46,7 +46,7 @@ public PsesCompletionHandler( // TODO: What do we do with the arguments? DocumentSelector = LspUtils.PowerShellDocumentSelector, ResolveProvider = true, - TriggerCharacters = new[] { ".", "-", ":", "\\", "$" } + TriggerCharacters = new[] { ".", "-", ":", "\\", "$", " " } }; public override async Task Handle(CompletionParams request, CancellationToken cancellationToken) @@ -55,13 +55,16 @@ public override async Task Handle(CompletionParams request, Canc int cursorColumn = request.Position.Character + 1; ScriptFile scriptFile = _workspaceService.GetFile(request.TextDocument.Uri); - IEnumerable completionResults = await GetCompletionsInFileAsync( + (bool isIncomplete, IReadOnlyList completionResults) = await GetCompletionsInFileAsync( scriptFile, cursorLine, cursorColumn, cancellationToken).ConfigureAwait(false); - return new CompletionList(completionResults); + // Treat completions trigged by space as incomplete so that `gci ` + // and then typing `-` doesn't just filter the list of parameter values + // (typically files) returned by the space completion + return new CompletionList(completionResults, isIncomplete || request.Context.TriggerCharacter is " "); } // Handler for "completionItem/resolve". In VSCode this is fired when a completion item is highlighted in the completion list. @@ -113,7 +116,7 @@ public override async Task Handle(CompletionItem request, Cancel /// /// A CommandCompletion instance completions for the identified statement. /// - internal async Task> GetCompletionsInFileAsync( + internal async Task<(bool isIncomplete, IReadOnlyList matches)> GetCompletionsInFileAsync( ScriptFile scriptFile, int lineNumber, int columnNumber, @@ -131,21 +134,32 @@ internal async Task> GetCompletionsInFileAsync( if (result.CompletionMatches.Count == 0) { - return Array.Empty(); + return (true, Array.Empty()); } BufferRange replacedRange = scriptFile.GetRangeBetweenOffsets( result.ReplacementIndex, 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. CompletionItem[] completionItems = new CompletionItem[result.CompletionMatches.Count]; for (int i = 0; i < result.CompletionMatches.Count; i++) { + CompletionResult completionMatch = result.CompletionMatches[i]; + + // If a completion result is a variable scope like `$script:` we want to + // mark as incomplete so on typing `:` completion changes. + if (completionMatch.ResultType is CompletionResultType.Variable + && completionMatch.CompletionText.EndsWith(":")) + { + isIncomplete = true; + } + completionItems[i] = CreateCompletionItem(result.CompletionMatches[i], replacedRange, i + 1); } - return completionItems; + return (isIncomplete, completionItems); } internal static CompletionItem CreateCompletionItem( From f31c1c242fb3da14e310f65505b8db6ad3125004 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Thu, 12 May 2022 14:53:38 -0400 Subject: [PATCH 6/8] Make completion requests cancellable That way any completion results from custom argument completers can be cancelled if they take too long and the user keeps typing. --- .../Services/Symbols/Vistors/AstOperations.cs | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs b/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs index ee364107c..54ddb646a 100644 --- a/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs +++ b/src/PowerShellEditorServices/Services/Symbols/Vistors/AstOperations.cs @@ -14,6 +14,7 @@ using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services.PowerShell; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; namespace Microsoft.PowerShell.EditorServices.Services.Symbols { @@ -92,6 +93,34 @@ await executionService.ExecuteDelegateAsync( (pwsh, _) => { stopwatch.Start(); + + // If the current runspace is not out of process, then we call TabExpansion2 so + // that we have the ability to issue pipeline stop requests on cancellation. + if (executionService is PsesInternalHost psesInternalHost + && !psesInternalHost.Runspace.RunspaceIsRemote) + { + IReadOnlyList completionResults = new SynchronousPowerShellTask( + logger, + psesInternalHost, + new PSCommand() + .AddCommand("TabExpansion2") + .AddParameter("ast", scriptAst) + .AddParameter("tokens", currentTokens) + .AddParameter("positionOfCursor", cursorPosition), + executionOptions: null, + cancellationToken) + .ExecuteAndGetResult(cancellationToken); + + if (completionResults is { Count: > 0 }) + { + commandCompletion = completionResults[0]; + } + + return; + } + + // If the current runspace is out of process, we can't call TabExpansion2 + // because the output will be serialized. commandCompletion = CommandCompletion.CompleteInput( scriptAst, currentTokens, From 8da0b7986857fb0021175eecdb45219d6a9f2fc7 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Fri, 13 May 2022 14:41:52 -0400 Subject: [PATCH 7/8] Fix test and use record instead of tuple --- .../TextDocument/Handlers/CompletionHandler.cs | 8 +++++--- .../Language/CompletionHandlerTests.cs | 16 ++++++++-------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index e1dee04d3..da2f5cf13 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -22,6 +22,8 @@ namespace Microsoft.PowerShell.EditorServices.Handlers { + internal record CompletionResults(bool IsIncomplete, IReadOnlyList Matches); + internal class PsesCompletionHandler : CompletionHandlerBase { private readonly ILogger _logger; @@ -116,7 +118,7 @@ public override async Task Handle(CompletionItem request, Cancel /// /// A CommandCompletion instance completions for the identified statement. /// - internal async Task<(bool isIncomplete, IReadOnlyList matches)> GetCompletionsInFileAsync( + internal async Task GetCompletionsInFileAsync( ScriptFile scriptFile, int lineNumber, int columnNumber, @@ -134,7 +136,7 @@ public override async Task Handle(CompletionItem request, Cancel if (result.CompletionMatches.Count == 0) { - return (true, Array.Empty()); + return new CompletionResults(IsIncomplete: true, Array.Empty()); } BufferRange replacedRange = scriptFile.GetRangeBetweenOffsets( @@ -159,7 +161,7 @@ public override async Task Handle(CompletionItem request, Cancel completionItems[i] = CreateCompletionItem(result.CompletionMatches[i], replacedRange, i + 1); } - return (isIncomplete, completionItems); + return new CompletionResults(isIncomplete, completionItems); } internal static CompletionItem CreateCompletionItem( diff --git a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs index 1e87127b1..d8094799c 100644 --- a/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs +++ b/test/PowerShellEditorServices.Test/Language/CompletionHandlerTests.cs @@ -41,7 +41,7 @@ public void Dispose() private ScriptFile GetScriptFile(ScriptRegion scriptRegion) => workspace.GetFile(TestUtilities.GetSharedPath(scriptRegion.File)); - private Task> GetCompletionResultsAsync(ScriptRegion scriptRegion) + private Task GetCompletionResultsAsync(ScriptRegion scriptRegion) { return completionHandler.GetCompletionsInFileAsync( GetScriptFile(scriptRegion), @@ -53,7 +53,7 @@ private Task> GetCompletionResultsAsync(ScriptRegion [Fact] public async Task CompletesCommandInFile() { - IEnumerable results = await GetCompletionResultsAsync(CompleteCommandInFile.SourceDetails).ConfigureAwait(true); + (_, IEnumerable results) = await GetCompletionResultsAsync(CompleteCommandInFile.SourceDetails).ConfigureAwait(true); CompletionItem actual = Assert.Single(results); Assert.Equal(CompleteCommandInFile.ExpectedCompletion, actual); } @@ -61,7 +61,7 @@ public async Task CompletesCommandInFile() [Fact] public async Task CompletesCommandFromModule() { - IEnumerable results = await GetCompletionResultsAsync(CompleteCommandFromModule.SourceDetails).ConfigureAwait(true); + (_, 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 = "" }); @@ -71,7 +71,7 @@ public async Task CompletesCommandFromModule() [Fact] public async Task CompletesTypeName() { - IEnumerable results = await GetCompletionResultsAsync(CompleteTypeName.SourceDetails).ConfigureAwait(true); + (_, IEnumerable results) = await GetCompletionResultsAsync(CompleteTypeName.SourceDetails).ConfigureAwait(true); CompletionItem actual = Assert.Single(results); if (VersionUtils.IsNetCore) { @@ -92,7 +92,7 @@ public async Task CompletesTypeName() [Fact] public async Task CompletesNamespace() { - IEnumerable results = await GetCompletionResultsAsync(CompleteNamespace.SourceDetails).ConfigureAwait(true); + (_, IEnumerable results) = await GetCompletionResultsAsync(CompleteNamespace.SourceDetails).ConfigureAwait(true); CompletionItem actual = Assert.Single(results); Assert.Equal(CompleteNamespace.ExpectedCompletion, actual); } @@ -100,7 +100,7 @@ public async Task CompletesNamespace() [Fact] public async Task CompletesVariableInFile() { - IEnumerable results = await GetCompletionResultsAsync(CompleteVariableInFile.SourceDetails).ConfigureAwait(true); + (_, IEnumerable results) = await GetCompletionResultsAsync(CompleteVariableInFile.SourceDetails).ConfigureAwait(true); CompletionItem actual = Assert.Single(results); Assert.Equal(CompleteVariableInFile.ExpectedCompletion, actual); } @@ -108,7 +108,7 @@ public async Task CompletesVariableInFile() [Fact] public async Task CompletesAttributeValue() { - IEnumerable results = await GetCompletionResultsAsync(CompleteAttributeValue.SourceDetails).ConfigureAwait(true); + (_, IEnumerable results) = await GetCompletionResultsAsync(CompleteAttributeValue.SourceDetails).ConfigureAwait(true); Assert.Collection(results.OrderBy(c => c.SortText), actual => Assert.Equal(actual, CompleteAttributeValue.ExpectedCompletion1), actual => Assert.Equal(actual, CompleteAttributeValue.ExpectedCompletion2), @@ -118,7 +118,7 @@ public async Task CompletesAttributeValue() [Fact] public async Task CompletesFilePath() { - IEnumerable results = await GetCompletionResultsAsync(CompleteFilePath.SourceDetails).ConfigureAwait(true); + (_, 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. From 220dd1a28fd1f9d7efcc6617c14cf3b4966cce99 Mon Sep 17 00:00:00 2001 From: Patrick Meinecke Date: Tue, 17 May 2022 13:08:02 -0400 Subject: [PATCH 8/8] Fix NRE in tests --- .../Services/TextDocument/Handlers/CompletionHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs index da2f5cf13..6173f3fa9 100644 --- a/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs +++ b/src/PowerShellEditorServices/Services/TextDocument/Handlers/CompletionHandler.cs @@ -66,7 +66,7 @@ public override async Task Handle(CompletionParams request, Canc // Treat completions trigged by space as incomplete so that `gci ` // and then typing `-` doesn't just filter the list of parameter values // (typically files) returned by the space completion - return new CompletionList(completionResults, isIncomplete || request.Context.TriggerCharacter is " "); + return new CompletionList(completionResults, isIncomplete || request?.Context?.TriggerCharacter is " "); } // Handler for "completionItem/resolve". In VSCode this is fired when a completion item is highlighted in the completion list.