From ec06e5755d9c2a962142e34ee0addb26cf974c24 Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Thu, 4 May 2023 17:17:09 -0700 Subject: [PATCH 1/4] Remove useless `ToArray` of `BreakpointDetails` This can just be an `IEnumerable` throughout instead. --- .../Services/DebugAdapter/DebugService.cs | 10 ++-- .../Handlers/BreakpointHandlers.cs | 8 +-- .../Debugging/DscBreakpointCapability.cs | 17 +++--- .../Debugging/DebugServiceTests.cs | 52 +++++++++---------- 4 files changed, 41 insertions(+), 46 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs index 7ea6ad435..63d8c961c 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs @@ -130,9 +130,9 @@ public DebugService( /// BreakpointDetails for each breakpoint that will be set. /// If true, causes all existing breakpoints to be cleared before setting new ones. /// An awaitable Task that will provide details about the breakpoints that were set. - public async Task SetLineBreakpointsAsync( + public async Task> SetLineBreakpointsAsync( ScriptFile scriptFile, - BreakpointDetails[] breakpoints, + IEnumerable breakpoints, bool clearExisting = true) { DscBreakpointCapability dscBreakpoints = await _debugContext.GetDscBreakpointCapabilityAsync(CancellationToken.None).ConfigureAwait(false); @@ -146,7 +146,7 @@ public async Task SetLineBreakpointsAsync( if (!_remoteFileManager.IsUnderRemoteTempPath(scriptPath)) { _logger.LogTrace($"Could not set breakpoints for local path '{scriptPath}' in a remote session."); - return Array.Empty(); + return Enumerable.Empty(); } scriptPath = _remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace); @@ -154,7 +154,7 @@ public async Task SetLineBreakpointsAsync( else if (temporaryScriptListingPath?.Equals(scriptPath, StringComparison.CurrentCultureIgnoreCase) == true) { _logger.LogTrace($"Could not set breakpoint on temporary script listing path '{scriptPath}'."); - return Array.Empty(); + return Enumerable.Empty(); } // Fix for issue #123 - file paths that contain wildcard chars [ and ] need to @@ -168,7 +168,7 @@ public async Task SetLineBreakpointsAsync( await _breakpointService.RemoveAllBreakpointsAsync(scriptFile.FilePath).ConfigureAwait(false); } - return (await _breakpointService.SetBreakpointsAsync(escapedScriptPath, breakpoints).ConfigureAwait(false)).ToArray(); + return await _breakpointService.SetBreakpointsAsync(escapedScriptPath, breakpoints).ConfigureAwait(false); } return await dscBreakpoints diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs index 03f2841f2..0e3da3942 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. using System; +using System.Collections.Generic; using System.IO; using System.Linq; using System.Threading; @@ -82,18 +83,17 @@ public async Task Handle(SetBreakpointsArguments request } // At this point, the source file has been verified as a PowerShell script. - BreakpointDetails[] breakpointDetails = request.Breakpoints + IEnumerable breakpointDetails = request.Breakpoints .Select((srcBreakpoint) => BreakpointDetails.Create( scriptFile.FilePath, srcBreakpoint.Line, srcBreakpoint.Column, srcBreakpoint.Condition, srcBreakpoint.HitCondition, - srcBreakpoint.LogMessage)) - .ToArray(); + srcBreakpoint.LogMessage)); // If this is a "run without debugging (Ctrl+F5)" session ignore requests to set breakpoints. - BreakpointDetails[] updatedBreakpointDetails = breakpointDetails; + IEnumerable updatedBreakpointDetails = breakpointDetails; if (!_debugStateService.NoDebug) { await _debugStateService.WaitForSetBreakpointHandleAsync().ConfigureAwait(false); diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs index 644eec763..7121e6d75 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs @@ -22,24 +22,19 @@ internal class DscBreakpointCapability { private string[] dscResourceRootPaths = Array.Empty(); - private readonly Dictionary breakpointsPerFile = - new(); + private readonly Dictionary breakpointsPerFile = new(); - public async Task SetLineBreakpointsAsync( + public async Task> SetLineBreakpointsAsync( IInternalPowerShellExecutionService executionService, string scriptPath, - BreakpointDetails[] breakpoints) + IEnumerable breakpoints) { - List resultBreakpointDetails = - new(); - // We always get the latest array of breakpoint line numbers // so store that for future use - if (breakpoints.Length > 0) + if (breakpoints.Any()) { // Set the breakpoints for this scriptPath - breakpointsPerFile[scriptPath] = - breakpoints.Select(b => b.LineNumber).ToArray(); + breakpointsPerFile[scriptPath] = breakpoints.Select(b => b.LineNumber).ToArray(); } else { @@ -72,7 +67,7 @@ await executionService.ExecutePSCommandAsync( breakpoint.Verified = true; } - return breakpoints.ToArray(); + return breakpoints; } public bool IsDscResourcePath(string scriptPath) diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index fa1ed7be8..0e58a0025 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -203,7 +203,7 @@ await debugService.SetCommandBreakpointsAsync( [MemberData(nameof(DebuggerAcceptsScriptArgsTestData))] public async Task DebuggerAcceptsScriptArgs(string[] args) { - BreakpointDetails[] breakpoints = await debugService.SetLineBreakpointsAsync( + IEnumerable breakpoints = await debugService.SetLineBreakpointsAsync( oddPathScriptFile, new[] { BreakpointDetails.Create(oddPathScriptFile.FilePath, 3) }).ConfigureAwait(true); @@ -264,8 +264,8 @@ public async Task DebuggerSetsAndClearsFunctionBreakpoints() }).ConfigureAwait(true); Assert.Equal(2, breakpoints.Length); - Assert.Equal("Write-Host", breakpoints[0].Name); - Assert.Equal("Get-Date", breakpoints[1].Name); + Assert.Equal("Write-Host", breakpoints.ElementAt(0).Name); + Assert.Equal("Get-Date", breakpoints.ElementAt(1).Name); breakpoints = await debugService.SetCommandBreakpointsAsync( new[] { CommandBreakpointDetails.Create("Get-Host") }).ConfigureAwait(true); @@ -311,7 +311,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints() [Fact] public async Task DebuggerSetsAndClearsLineBreakpoints() { - BreakpointDetails[] breakpoints = + IEnumerable breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, new[] { @@ -322,8 +322,8 @@ await debugService.SetLineBreakpointsAsync( IReadOnlyList confirmedBreakpoints = await GetConfirmedBreakpoints(debugScriptFile).ConfigureAwait(true); Assert.Equal(2, confirmedBreakpoints.Count); - Assert.Equal(5, breakpoints[0].LineNumber); - Assert.Equal(10, breakpoints[1].LineNumber); + Assert.Equal(5, breakpoints.ElementAt(0).LineNumber); + Assert.Equal(10, breakpoints.ElementAt(1).LineNumber); breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, @@ -331,7 +331,7 @@ await debugService.SetLineBreakpointsAsync( confirmedBreakpoints = await GetConfirmedBreakpoints(debugScriptFile).ConfigureAwait(true); Assert.Single(confirmedBreakpoints); - Assert.Equal(2, breakpoints[0].LineNumber); + Assert.Equal(2, breakpoints.ElementAt(0).LineNumber); await debugService.SetLineBreakpointsAsync( debugScriptFile, @@ -442,7 +442,7 @@ await debugService.SetLineBreakpointsAsync( [Fact] public async Task DebuggerProvidesMessageForInvalidConditionalBreakpoint() { - BreakpointDetails[] breakpoints = + IEnumerable breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, new[] { @@ -457,20 +457,20 @@ await debugService.SetLineBreakpointsAsync( }).ConfigureAwait(true); Assert.Single(breakpoints); - // Assert.Equal(5, breakpoints[0].LineNumber); - // Assert.True(breakpoints[0].Verified); - // Assert.Null(breakpoints[0].Message); - - Assert.Equal(10, breakpoints[0].LineNumber); - Assert.False(breakpoints[0].Verified); - Assert.NotNull(breakpoints[0].Message); - Assert.Contains("Unexpected token '-ez'", breakpoints[0].Message); + // Assert.Equal(5, breakpoints.ElementAt(0).LineNumber); + // Assert.True(breakpoints.ElementAt(0).Verified); + // Assert.Null(breakpoints.ElementAt(0).Message); + + Assert.Equal(10, breakpoints.ElementAt(0).LineNumber); + Assert.False(breakpoints.ElementAt(0).Verified); + Assert.NotNull(breakpoints.ElementAt(0).Message); + Assert.Contains("Unexpected token '-ez'", breakpoints.ElementAt(0).Message); } [Fact] public async Task DebuggerFindsParsableButInvalidSimpleBreakpointConditions() { - BreakpointDetails[] breakpoints = + IEnumerable breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, new[] { @@ -478,15 +478,15 @@ await debugService.SetLineBreakpointsAsync( BreakpointDetails.Create(debugScriptFile.FilePath, 7, column: null, condition: "$i > 100") }).ConfigureAwait(true); - Assert.Equal(2, breakpoints.Length); - Assert.Equal(5, breakpoints[0].LineNumber); - Assert.False(breakpoints[0].Verified); - Assert.Contains("Use '-eq' instead of '=='", breakpoints[0].Message); - - Assert.Equal(7, breakpoints[1].LineNumber); - Assert.False(breakpoints[1].Verified); - Assert.NotNull(breakpoints[1].Message); - Assert.Contains("Use '-gt' instead of '>'", breakpoints[1].Message); + Assert.Equal(2, breakpoints.Count()); + Assert.Equal(5, breakpoints.ElementAt(0).LineNumber); + Assert.False(breakpoints.ElementAt(0).Verified); + Assert.Contains("Use '-eq' instead of '=='", breakpoints.ElementAt(0).Message); + + Assert.Equal(7, breakpoints.ElementAt(1).LineNumber); + Assert.False(breakpoints.ElementAt(1).Verified); + Assert.NotNull(breakpoints.ElementAt(1).Message); + Assert.Contains("Use '-gt' instead of '>'", breakpoints.ElementAt(1).Message); } [Fact] From 11837b9e4051541276b58be46026cfd585201f3a Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Thu, 4 May 2023 17:53:46 -0700 Subject: [PATCH 2/4] Only try to load DSC module once Instead of trying repeatedly every time we set a breakpoint. --- .../Services/DebugAdapter/DebugService.cs | 2 +- .../Handlers/BreakpointHandlers.cs | 4 +- .../Debugging/DscBreakpointCapability.cs | 120 +++++++----------- .../Debugging/IPowerShellDebugContext.cs | 3 +- .../Debugging/PowerShellDebugContext.cs | 5 +- .../PowerShell/Runspace/RunspaceInfo.cs | 7 +- .../Services/Template/TemplateService.cs | 20 +-- 7 files changed, 58 insertions(+), 103 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs index 63d8c961c..a02775cc5 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs @@ -135,7 +135,7 @@ public async Task> SetLineBreakpointsAsync( IEnumerable breakpoints, bool clearExisting = true) { - DscBreakpointCapability dscBreakpoints = await _debugContext.GetDscBreakpointCapabilityAsync(CancellationToken.None).ConfigureAwait(false); + DscBreakpointCapability dscBreakpoints = await _debugContext.GetDscBreakpointCapabilityAsync().ConfigureAwait(false); string scriptPath = scriptFile.FilePath; diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs index 0e3da3942..4efe9e6f2 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs @@ -52,7 +52,7 @@ public async Task Handle(SetBreakpointsArguments request if (!_workspaceService.TryGetFile(request.Source.Path, out ScriptFile scriptFile)) { string message = _debugStateService.NoDebug ? string.Empty : "Source file could not be accessed, breakpoint not set."; - System.Collections.Generic.IEnumerable srcBreakpoints = request.Breakpoints + IEnumerable srcBreakpoints = request.Breakpoints .Select(srcBkpt => LspDebugUtils.CreateBreakpoint( srcBkpt, request.Source.Path, message, verified: _debugStateService.NoDebug)); @@ -71,7 +71,7 @@ public async Task Handle(SetBreakpointsArguments request string message = _debugStateService.NoDebug ? string.Empty : "Source is not a PowerShell script, breakpoint not set."; - System.Collections.Generic.IEnumerable srcBreakpoints = request.Breakpoints + IEnumerable srcBreakpoints = request.Breakpoints .Select(srcBkpt => LspDebugUtils.CreateBreakpoint( srcBkpt, request.Source.Path, message, verified: _debugStateService.NoDebug)); diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs index 7121e6d75..189fb35b3 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs @@ -1,27 +1,24 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. -using System.Linq; -using System.Threading.Tasks; -using Microsoft.Extensions.Logging; -using Microsoft.PowerShell.EditorServices.Logging; -using Microsoft.PowerShell.EditorServices.Services.DebugAdapter; using System; using System.Collections.Generic; -using System.Collections.ObjectModel; +using System.Linq; using System.Management.Automation; using System.Threading; -using SMA = System.Management.Automation; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Utility; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace; +using System.Threading.Tasks; +using Microsoft.Extensions.Logging; +using Microsoft.PowerShell.EditorServices.Services.DebugAdapter; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; +using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace; namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging { internal class DscBreakpointCapability { + private static bool? isDscInstalled; private string[] dscResourceRootPaths = Array.Empty(); - private readonly Dictionary breakpointsPerFile = new(); public async Task> SetLineBreakpointsAsync( @@ -79,88 +76,57 @@ public bool IsDscResourcePath(string scriptPath) StringComparison.CurrentCultureIgnoreCase)); } - public static Task GetDscCapabilityAsync( + public static async Task GetDscCapabilityAsync( ILogger logger, IRunspaceInfo currentRunspace, - PsesInternalHost psesHost, - CancellationToken cancellationToken) + PsesInternalHost psesHost) { // DSC support is enabled only for Windows PowerShell. if ((currentRunspace.PowerShellVersionDetails.Version.Major >= 6) && (currentRunspace.RunspaceOrigin != RunspaceOrigin.DebuggedRunspace)) { - return Task.FromResult(null); + return null; } - DscBreakpointCapability getDscBreakpointCapabilityFunc(SMA.PowerShell pwsh, CancellationToken _) + if (!isDscInstalled.HasValue) { - PSInvocationSettings invocationSettings = new() - { - AddToHistory = false, - ErrorActionPreference = ActionPreference.Stop - }; - - PSModuleInfo dscModule = null; - try - { - dscModule = pwsh.AddCommand("Import-Module") - .AddArgument(@"C:\Program Files\DesiredStateConfiguration\1.0.0.0\Modules\PSDesiredStateConfiguration\PSDesiredStateConfiguration.psd1") - .AddParameter("PassThru") - .InvokeAndClear(invocationSettings) - .FirstOrDefault(); - } - catch (RuntimeException e) - { - logger.LogException("Could not load the DSC module!", e); - } - - if (dscModule == null) - { - logger.LogTrace("Side-by-side DSC module was not found."); - return null; - } - - logger.LogTrace("Side-by-side DSC module found, gathering DSC resource paths..."); - - // The module was loaded, add the breakpoint capability - DscBreakpointCapability capability = new(); - - pwsh.AddCommand("Microsoft.PowerShell.Utility\\Write-Host") - .AddArgument("Gathering DSC resource paths, this may take a while...") - .InvokeAndClear(invocationSettings); - - Collection resourcePaths = null; - try - { - // Get the list of DSC resource paths - resourcePaths = pwsh.AddCommand("Get-DscResource") - .AddCommand("Select-Object") - .AddParameter("ExpandProperty", "ParentPath") - .InvokeAndClear(invocationSettings); - } - catch (CmdletInvocationException e) - { - logger.LogException("Get-DscResource failed!", e); - } - - if (resourcePaths == null) - { - logger.LogTrace("No DSC resources found."); - return null; - } + PSCommand psCommand = new PSCommand() + .AddCommand("Import-Module") + .AddArgument(@"C:\Program Files\DesiredStateConfiguration\1.0.0.0\Modules\PSDesiredStateConfiguration\PSDesiredStateConfiguration.psd1") + .AddParameter("PassThru"); + + IReadOnlyList dscModule = + await psesHost.ExecutePSCommandAsync( + psCommand, + CancellationToken.None, + new PowerShellExecutionOptions { ThrowOnError = false }).ConfigureAwait(false); + + isDscInstalled = dscModule.Count > 0; + logger.LogTrace("Side-by-side DSC module found: " + isDscInstalled.Value); + } - capability.dscResourceRootPaths = resourcePaths.ToArray(); + if (isDscInstalled.Value) + { + PSCommand psCommand = new PSCommand() + .AddCommand("Get-DscResource") + .AddCommand("Select-Object") + .AddParameter("ExpandProperty", "ParentPath"); + + IReadOnlyList resourcePaths = + await psesHost.ExecutePSCommandAsync( + psCommand, + CancellationToken.None, + new PowerShellExecutionOptions { ThrowOnError = false } + ).ConfigureAwait(false); logger.LogTrace($"DSC resources found: {resourcePaths.Count}"); - - return capability; + return new DscBreakpointCapability + { + dscResourceRootPaths = resourcePaths.ToArray() + }; } - return psesHost.ExecuteDelegateAsync( - nameof(getDscBreakpointCapabilityFunc), - executionOptions: null, - getDscBreakpointCapabilityFunc, - cancellationToken); + return null; } } } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/IPowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/IPowerShellDebugContext.cs index 173e5992b..506109b7d 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/IPowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/IPowerShellDebugContext.cs @@ -3,7 +3,6 @@ using System; using System.Management.Automation; -using System.Threading; using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging @@ -34,6 +33,6 @@ internal interface IPowerShellDebugContext void Abort(); - Task GetDscBreakpointCapabilityAsync(CancellationToken cancellationToken); + Task GetDscBreakpointCapabilityAsync(); } } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs index 1d45c435d..f88aa38d5 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/PowerShellDebugContext.cs @@ -3,7 +3,6 @@ using System; using System.Management.Automation; -using System.Threading; using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Context; @@ -81,10 +80,10 @@ public PowerShellDebugContext( public event Action DebuggerResuming; public event Action BreakpointUpdated; - public Task GetDscBreakpointCapabilityAsync(CancellationToken cancellationToken) + public Task GetDscBreakpointCapabilityAsync() { _psesHost.Runspace.ThrowCancelledIfUnusable(); - return _psesHost.CurrentRunspace.GetDscBreakpointCapabilityAsync(_logger, _psesHost, cancellationToken); + return _psesHost.CurrentRunspace.GetDscBreakpointCapabilityAsync(_logger, _psesHost); } // This is required by the PowerShell API so that remote debugging works. Without it, a diff --git a/src/PowerShellEditorServices/Services/PowerShell/Runspace/RunspaceInfo.cs b/src/PowerShellEditorServices/Services/PowerShell/Runspace/RunspaceInfo.cs index d35d0f8a7..c955bac4c 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Runspace/RunspaceInfo.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Runspace/RunspaceInfo.cs @@ -5,7 +5,6 @@ using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging; using Microsoft.Extensions.Logging; using System.Threading.Tasks; -using System.Threading; using System; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host; @@ -86,14 +85,12 @@ public RunspaceInfo( public async Task GetDscBreakpointCapabilityAsync( ILogger logger, - PsesInternalHost psesHost, - CancellationToken cancellationToken) + PsesInternalHost psesHost) { return _dscBreakpointCapability ??= await DscBreakpointCapability.GetDscCapabilityAsync( logger, this, - psesHost, - cancellationToken) + psesHost) .ConfigureAwait(false); } } diff --git a/src/PowerShellEditorServices/Services/Template/TemplateService.cs b/src/PowerShellEditorServices/Services/Template/TemplateService.cs index f6dc76fa6..4de0de520 100644 --- a/src/PowerShellEditorServices/Services/Template/TemplateService.cs +++ b/src/PowerShellEditorServices/Services/Template/TemplateService.cs @@ -72,14 +72,12 @@ public async Task ImportPlasterIfInstalledAsync() _logger.LogTrace("Checking if Plaster is installed..."); - PSObject moduleObject = (await _executionService.ExecutePSCommandAsync(psCommand, CancellationToken.None).ConfigureAwait(false))[0]; + IReadOnlyList moduleObject = + await _executionService.ExecutePSCommandAsync( + psCommand, CancellationToken.None).ConfigureAwait(false); - isPlasterInstalled = moduleObject != null; - string installedQualifier = - isPlasterInstalled.Value - ? string.Empty : "not "; - - _logger.LogTrace($"Plaster is {installedQualifier}installed!"); + isPlasterInstalled = moduleObject.Count > 0; + _logger.LogTrace("Plaster installed: " + isPlasterInstalled.Value); // Attempt to load plaster if (isPlasterInstalled.Value && !isPlasterLoaded) @@ -89,17 +87,13 @@ public async Task ImportPlasterIfInstalledAsync() psCommand = new PSCommand(); psCommand .AddCommand("Import-Module") - .AddParameter("ModuleInfo", (PSModuleInfo)moduleObject.ImmediateBaseObject) + .AddParameter("ModuleInfo", (PSModuleInfo)moduleObject[0].ImmediateBaseObject) .AddParameter("PassThru"); IReadOnlyList importResult = await _executionService.ExecutePSCommandAsync(psCommand, CancellationToken.None).ConfigureAwait(false); isPlasterLoaded = importResult.Count > 0; - string loadedQualifier = - isPlasterInstalled.Value - ? "was" : "could not be"; - - _logger.LogTrace($"Plaster {loadedQualifier} loaded successfully!"); + _logger.LogTrace("Plaster loaded: " + isPlasterLoaded); } } From 67905945b9459f0fcffd0fe619e85b5a60933a1c Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Fri, 5 May 2023 12:32:59 -0700 Subject: [PATCH 3/4] Fix Emacs end-to-end test The way the class properties are accessed changed. --- test/emacs-test.el | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/emacs-test.el b/test/emacs-test.el index d703ff22d..378c792fc 100644 --- a/test/emacs-test.el +++ b/test/emacs-test.el @@ -59,9 +59,8 @@ (should (apply #'eglot--connect (eglot--guess-contact))) (should (eglot-current-server)) (let ((lsp (eglot-current-server))) - (should (string= (oref lsp project-nickname) "PowerShellEditorServices")) - (should (member 'powershell-mode (oref lsp major-modes))) - (should (string= (oref lsp language-id) "powershell"))) + (should (string= (eglot--project-nickname lsp) "PowerShellEditorServices")) + (should (member (cons 'powershell-mode "powershell") (eglot--languages lsp)))) (sleep-for 5) ; TODO: Wait for "textDocument/publishDiagnostics" instead (flymake-start) (goto-char (point-min)) From aa73a2162564e5533f0b53dde5686e9b00f5b871 Mon Sep 17 00:00:00 2001 From: Andy Jordan Date: Mon, 8 May 2023 12:06:08 -0700 Subject: [PATCH 4/4] Move to `IReadOnlyList` from `IEnumerable` to prevent double enumerations Seems to be the easiest "clean and safe" way to handle this. --- .../DebugAdapter/BreakpointService.cs | 12 ++-- .../Services/DebugAdapter/DebugService.cs | 25 ++++----- .../Handlers/BreakpointHandlers.cs | 21 +++---- .../Debugging/DscBreakpointCapability.cs | 9 +-- .../Debugging/DebugServiceTests.cs | 56 +++++++++---------- 5 files changed, 56 insertions(+), 67 deletions(-) diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs index 4f806ce39..748e21c6d 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/BreakpointService.cs @@ -40,7 +40,7 @@ public BreakpointService( _debugStateService = debugStateService; } - public async Task> GetBreakpointsAsync() + public async Task> GetBreakpointsAsync() { if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace)) { @@ -52,14 +52,12 @@ public async Task> GetBreakpointsAsync() // Legacy behavior PSCommand psCommand = new PSCommand().AddCommand(@"Microsoft.PowerShell.Utility\Get-PSBreakpoint"); - IEnumerable breakpoints = await _executionService + return await _executionService .ExecutePSCommandAsync(psCommand, CancellationToken.None) .ConfigureAwait(false); - - return breakpoints.ToList(); } - public async Task> SetBreakpointsAsync(string escapedScriptPath, IEnumerable breakpoints) + public async Task> SetBreakpointsAsync(string escapedScriptPath, IReadOnlyList breakpoints) { if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace)) { @@ -147,7 +145,7 @@ public async Task> SetBreakpointsAsync(string esc return configuredBreakpoints; } - public async Task> SetCommandBreakpointsAsync(IEnumerable breakpoints) + public async Task> SetCommandBreakpointsAsync(IReadOnlyList breakpoints) { if (BreakpointApiUtils.SupportsBreakpointApis(_editorServicesHost.CurrentRunspace)) { @@ -216,7 +214,7 @@ public async Task> SetCommandBreakpointsAs // If no PSCommand was created then there are no breakpoints to set. if (psCommand is not null) { - IEnumerable setBreakpoints = await _executionService + IReadOnlyList setBreakpoints = await _executionService .ExecutePSCommandAsync(psCommand, CancellationToken.None) .ConfigureAwait(false); configuredBreakpoints.AddRange(setBreakpoints.Select(CommandBreakpointDetails.Create)); diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs index a02775cc5..a92f05eed 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs @@ -130,9 +130,9 @@ public DebugService( /// BreakpointDetails for each breakpoint that will be set. /// If true, causes all existing breakpoints to be cleared before setting new ones. /// An awaitable Task that will provide details about the breakpoints that were set. - public async Task> SetLineBreakpointsAsync( + public async Task> SetLineBreakpointsAsync( ScriptFile scriptFile, - IEnumerable breakpoints, + IReadOnlyList breakpoints, bool clearExisting = true) { DscBreakpointCapability dscBreakpoints = await _debugContext.GetDscBreakpointCapabilityAsync().ConfigureAwait(false); @@ -146,7 +146,7 @@ public async Task> SetLineBreakpointsAsync( if (!_remoteFileManager.IsUnderRemoteTempPath(scriptPath)) { _logger.LogTrace($"Could not set breakpoints for local path '{scriptPath}' in a remote session."); - return Enumerable.Empty(); + return Array.Empty(); } scriptPath = _remoteFileManager.GetMappedPath(scriptPath, _psesHost.CurrentRunspace); @@ -154,7 +154,7 @@ public async Task> SetLineBreakpointsAsync( else if (temporaryScriptListingPath?.Equals(scriptPath, StringComparison.CurrentCultureIgnoreCase) == true) { _logger.LogTrace($"Could not set breakpoint on temporary script listing path '{scriptPath}'."); - return Enumerable.Empty(); + return Array.Empty(); } // Fix for issue #123 - file paths that contain wildcard chars [ and ] need to @@ -182,25 +182,20 @@ public async Task> SetLineBreakpointsAsync( /// CommandBreakpointDetails for each command breakpoint that will be set. /// If true, causes all existing function breakpoints to be cleared before setting new ones. /// An awaitable Task that will provide details about the breakpoints that were set. - public async Task SetCommandBreakpointsAsync( - CommandBreakpointDetails[] breakpoints, + public async Task> SetCommandBreakpointsAsync( + IReadOnlyList breakpoints, bool clearExisting = true) { - CommandBreakpointDetails[] resultBreakpointDetails = null; - if (clearExisting) { // Flatten dictionary values into one list and remove them all. - IEnumerable existingBreakpoints = await _breakpointService.GetBreakpointsAsync().ConfigureAwait(false); + IReadOnlyList existingBreakpoints = await _breakpointService.GetBreakpointsAsync().ConfigureAwait(false); await _breakpointService.RemoveBreakpointsAsync(existingBreakpoints.OfType()).ConfigureAwait(false); } - if (breakpoints.Length > 0) - { - resultBreakpointDetails = (await _breakpointService.SetCommandBreakpointsAsync(breakpoints).ConfigureAwait(false)).ToArray(); - } - - return resultBreakpointDetails ?? Array.Empty(); + return breakpoints.Count > 0 + ? await _breakpointService.SetCommandBreakpointsAsync(breakpoints).ConfigureAwait(false) + : Array.Empty(); } /// diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs index 4efe9e6f2..4c99ff747 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/BreakpointHandlers.cs @@ -83,17 +83,17 @@ public async Task Handle(SetBreakpointsArguments request } // At this point, the source file has been verified as a PowerShell script. - IEnumerable breakpointDetails = request.Breakpoints + IReadOnlyList breakpointDetails = request.Breakpoints .Select((srcBreakpoint) => BreakpointDetails.Create( scriptFile.FilePath, srcBreakpoint.Line, srcBreakpoint.Column, srcBreakpoint.Condition, srcBreakpoint.HitCondition, - srcBreakpoint.LogMessage)); + srcBreakpoint.LogMessage)).ToList(); // If this is a "run without debugging (Ctrl+F5)" session ignore requests to set breakpoints. - IEnumerable updatedBreakpointDetails = breakpointDetails; + IReadOnlyList updatedBreakpointDetails = breakpointDetails; if (!_debugStateService.NoDebug) { await _debugStateService.WaitForSetBreakpointHandleAsync().ConfigureAwait(false); @@ -125,23 +125,20 @@ await _debugService.SetLineBreakpointsAsync( public async Task Handle(SetFunctionBreakpointsArguments request, CancellationToken cancellationToken) { - CommandBreakpointDetails[] breakpointDetails = request.Breakpoints + IReadOnlyList breakpointDetails = request.Breakpoints .Select((funcBreakpoint) => CommandBreakpointDetails.Create( funcBreakpoint.Name, - funcBreakpoint.Condition)) - .ToArray(); + funcBreakpoint.Condition)).ToList(); // If this is a "run without debugging (Ctrl+F5)" session ignore requests to set breakpoints. - CommandBreakpointDetails[] updatedBreakpointDetails = breakpointDetails; + IReadOnlyList updatedBreakpointDetails = breakpointDetails; if (!_debugStateService.NoDebug) { await _debugStateService.WaitForSetBreakpointHandleAsync().ConfigureAwait(false); try { - updatedBreakpointDetails = - await _debugService.SetCommandBreakpointsAsync( - breakpointDetails).ConfigureAwait(false); + updatedBreakpointDetails = await _debugService.SetCommandBreakpointsAsync(breakpointDetails).ConfigureAwait(false); } catch (Exception e) { @@ -156,9 +153,7 @@ await _debugService.SetCommandBreakpointsAsync( return new SetFunctionBreakpointsResponse { - Breakpoints = updatedBreakpointDetails - .Select(LspDebugUtils.CreateBreakpoint) - .ToArray() + Breakpoints = updatedBreakpointDetails.Select(LspDebugUtils.CreateBreakpoint).ToList() }; } diff --git a/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs b/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs index 189fb35b3..d75274fa6 100644 --- a/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs +++ b/src/PowerShellEditorServices/Services/PowerShell/Debugging/DscBreakpointCapability.cs @@ -21,17 +21,18 @@ internal class DscBreakpointCapability private string[] dscResourceRootPaths = Array.Empty(); private readonly Dictionary breakpointsPerFile = new(); - public async Task> SetLineBreakpointsAsync( + public async Task> SetLineBreakpointsAsync( IInternalPowerShellExecutionService executionService, string scriptPath, - IEnumerable breakpoints) + IReadOnlyList breakpoints) { // We always get the latest array of breakpoint line numbers // so store that for future use - if (breakpoints.Any()) + int[] lineNumbers = breakpoints.Select(b => b.LineNumber).ToArray(); + if (lineNumbers.Length > 0) { // Set the breakpoints for this scriptPath - breakpointsPerFile[scriptPath] = breakpoints.Select(b => b.LineNumber).ToArray(); + breakpointsPerFile[scriptPath] = lineNumbers; } else { diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index 0e58a0025..68db85f36 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -203,7 +203,7 @@ await debugService.SetCommandBreakpointsAsync( [MemberData(nameof(DebuggerAcceptsScriptArgsTestData))] public async Task DebuggerAcceptsScriptArgs(string[] args) { - IEnumerable breakpoints = await debugService.SetLineBreakpointsAsync( + IReadOnlyList breakpoints = await debugService.SetLineBreakpointsAsync( oddPathScriptFile, new[] { BreakpointDetails.Create(oddPathScriptFile.FilePath, 3) }).ConfigureAwait(true); @@ -257,15 +257,15 @@ public async Task DebuggerAcceptsScriptArgs(string[] args) [Fact] public async Task DebuggerSetsAndClearsFunctionBreakpoints() { - CommandBreakpointDetails[] breakpoints = await debugService.SetCommandBreakpointsAsync( + IReadOnlyList breakpoints = await debugService.SetCommandBreakpointsAsync( new[] { CommandBreakpointDetails.Create("Write-Host"), CommandBreakpointDetails.Create("Get-Date") }).ConfigureAwait(true); - Assert.Equal(2, breakpoints.Length); - Assert.Equal("Write-Host", breakpoints.ElementAt(0).Name); - Assert.Equal("Get-Date", breakpoints.ElementAt(1).Name); + Assert.Equal(2, breakpoints.Count); + Assert.Equal("Write-Host", breakpoints[0].Name); + Assert.Equal("Get-Date", breakpoints[1].Name); breakpoints = await debugService.SetCommandBreakpointsAsync( new[] { CommandBreakpointDetails.Create("Get-Host") }).ConfigureAwait(true); @@ -281,7 +281,7 @@ public async Task DebuggerSetsAndClearsFunctionBreakpoints() [Fact] public async Task DebuggerStopsOnFunctionBreakpoints() { - CommandBreakpointDetails[] breakpoints = await debugService.SetCommandBreakpointsAsync( + IReadOnlyList breakpoints = await debugService.SetCommandBreakpointsAsync( new[] { CommandBreakpointDetails.Create("Write-Host") }).ConfigureAwait(true); Task _ = ExecuteDebugFileAsync(); @@ -311,7 +311,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints() [Fact] public async Task DebuggerSetsAndClearsLineBreakpoints() { - IEnumerable breakpoints = + IReadOnlyList breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, new[] { @@ -322,8 +322,8 @@ await debugService.SetLineBreakpointsAsync( IReadOnlyList confirmedBreakpoints = await GetConfirmedBreakpoints(debugScriptFile).ConfigureAwait(true); Assert.Equal(2, confirmedBreakpoints.Count); - Assert.Equal(5, breakpoints.ElementAt(0).LineNumber); - Assert.Equal(10, breakpoints.ElementAt(1).LineNumber); + Assert.Equal(5, breakpoints[0].LineNumber); + Assert.Equal(10, breakpoints[1].LineNumber); breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, @@ -331,7 +331,7 @@ await debugService.SetLineBreakpointsAsync( confirmedBreakpoints = await GetConfirmedBreakpoints(debugScriptFile).ConfigureAwait(true); Assert.Single(confirmedBreakpoints); - Assert.Equal(2, breakpoints.ElementAt(0).LineNumber); + Assert.Equal(2, breakpoints[0].LineNumber); await debugService.SetLineBreakpointsAsync( debugScriptFile, @@ -442,7 +442,7 @@ await debugService.SetLineBreakpointsAsync( [Fact] public async Task DebuggerProvidesMessageForInvalidConditionalBreakpoint() { - IEnumerable breakpoints = + IReadOnlyList breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, new[] { @@ -457,20 +457,20 @@ await debugService.SetLineBreakpointsAsync( }).ConfigureAwait(true); Assert.Single(breakpoints); - // Assert.Equal(5, breakpoints.ElementAt(0).LineNumber); - // Assert.True(breakpoints.ElementAt(0).Verified); - // Assert.Null(breakpoints.ElementAt(0).Message); - - Assert.Equal(10, breakpoints.ElementAt(0).LineNumber); - Assert.False(breakpoints.ElementAt(0).Verified); - Assert.NotNull(breakpoints.ElementAt(0).Message); - Assert.Contains("Unexpected token '-ez'", breakpoints.ElementAt(0).Message); + // Assert.Equal(5, breakpoints[0].LineNumber); + // Assert.True(breakpoints[0].Verified); + // Assert.Null(breakpoints[0].Message); + + Assert.Equal(10, breakpoints[0].LineNumber); + Assert.False(breakpoints[0].Verified); + Assert.NotNull(breakpoints[0].Message); + Assert.Contains("Unexpected token '-ez'", breakpoints[0].Message); } [Fact] public async Task DebuggerFindsParsableButInvalidSimpleBreakpointConditions() { - IEnumerable breakpoints = + IReadOnlyList breakpoints = await debugService.SetLineBreakpointsAsync( debugScriptFile, new[] { @@ -478,15 +478,15 @@ await debugService.SetLineBreakpointsAsync( BreakpointDetails.Create(debugScriptFile.FilePath, 7, column: null, condition: "$i > 100") }).ConfigureAwait(true); - Assert.Equal(2, breakpoints.Count()); - Assert.Equal(5, breakpoints.ElementAt(0).LineNumber); - Assert.False(breakpoints.ElementAt(0).Verified); - Assert.Contains("Use '-eq' instead of '=='", breakpoints.ElementAt(0).Message); + Assert.Equal(2, breakpoints.Count); + Assert.Equal(5, breakpoints[0].LineNumber); + Assert.False(breakpoints[0].Verified); + Assert.Contains("Use '-eq' instead of '=='", breakpoints[0].Message); - Assert.Equal(7, breakpoints.ElementAt(1).LineNumber); - Assert.False(breakpoints.ElementAt(1).Verified); - Assert.NotNull(breakpoints.ElementAt(1).Message); - Assert.Contains("Use '-gt' instead of '>'", breakpoints.ElementAt(1).Message); + Assert.Equal(7, breakpoints[1].LineNumber); + Assert.False(breakpoints[1].Verified); + Assert.NotNull(breakpoints[1].Message); + Assert.Contains("Use '-gt' instead of '>'", breakpoints[1].Message); } [Fact]