diff --git a/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs b/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs index ff827646e..59e87b4be 100644 --- a/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs +++ b/src/PowerShellEditorServices/Hosting/EditorServicesServerFactory.cs @@ -119,7 +119,6 @@ public PsesDebugServer CreateDebugServerWithLanguageServer( inputStream, outputStream, languageServer.LanguageServer.Services, - useTempSession: false, usePSReadLine); } @@ -144,7 +143,6 @@ public PsesDebugServer RecreateDebugServer( inputStream, outputStream, debugServer.ServiceProvider, - useTempSession: false, usePSReadLine); } @@ -184,7 +182,6 @@ public PsesDebugServer CreateDebugServerForTempSession( inputStream, outputStream, serviceProvider, - useTempSession: true, usePSReadLine: hostStartupInfo.ConsoleReplEnabled && !hostStartupInfo.UsesLegacyReadLine); } diff --git a/src/PowerShellEditorServices/Server/PsesDebugServer.cs b/src/PowerShellEditorServices/Server/PsesDebugServer.cs index 0b1bf209b..89bdaeb3c 100644 --- a/src/PowerShellEditorServices/Server/PsesDebugServer.cs +++ b/src/PowerShellEditorServices/Server/PsesDebugServer.cs @@ -23,7 +23,6 @@ internal class PsesDebugServer : IDisposable { private readonly Stream _inputStream; private readonly Stream _outputStream; - private readonly bool _useTempSession; private readonly bool _usePSReadLine; private readonly TaskCompletionSource _serverStopped; @@ -40,14 +39,12 @@ public PsesDebugServer( Stream inputStream, Stream outputStream, IServiceProvider serviceProvider, - bool useTempSession, bool usePSReadLine) { _loggerFactory = factory; _inputStream = inputStream; _outputStream = outputStream; ServiceProvider = serviceProvider; - _useTempSession = useTempSession; _serverStopped = new TaskCompletionSource(); _usePSReadLine = usePSReadLine; } @@ -74,7 +71,7 @@ public async Task StartAsync() serviceCollection .AddLogging() .AddOptions() - .AddPsesDebugServices(ServiceProvider, this, _useTempSession)) + .AddPsesDebugServices(ServiceProvider, this)) // TODO: Consider replacing all WithHandler with AddSingleton .WithHandler() .WithHandler() diff --git a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs index 55f440a07..74d454a2d 100644 --- a/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs +++ b/src/PowerShellEditorServices/Server/PsesServiceCollectionExtensions.cs @@ -59,8 +59,7 @@ public static IServiceCollection AddPsesLanguageServices( public static IServiceCollection AddPsesDebugServices( this IServiceCollection collection, IServiceProvider languageServiceProvider, - PsesDebugServer psesDebugServer, - bool useTempSession) + PsesDebugServer psesDebugServer) { PsesInternalHost internalHost = languageServiceProvider.GetService(); @@ -74,10 +73,7 @@ public static IServiceCollection AddPsesDebugServices( .AddSingleton(psesDebugServer) .AddSingleton() .AddSingleton() - .AddSingleton(new DebugStateService - { - OwnsEditorSession = useTempSession - }) + .AddSingleton() .AddSingleton(); } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs index a4adaf194..f1bf3199e 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/DebugStateService.cs @@ -25,8 +25,6 @@ internal class DebugStateService internal string ScriptToLaunch { get; set; } - internal bool OwnsEditorSession { get; set; } - internal bool ExecutionCompleted { get; set; } internal bool IsInteractiveDebugSession { get; set; } @@ -39,14 +37,8 @@ internal class DebugStateService // This gets set at the end of the Launch/Attach handler which set debug state. internal TaskCompletionSource ServerStarted { get; set; } - internal void ReleaseSetBreakpointHandle() - { - _setBreakpointInProgressHandle.Release(); - } + internal int ReleaseSetBreakpointHandle() => _setBreakpointInProgressHandle.Release(); - internal async Task WaitForSetBreakpointHandleAsync() - { - await _setBreakpointInProgressHandle.WaitAsync().ConfigureAwait(false); - } + internal Task WaitForSetBreakpointHandleAsync() => _setBreakpointInProgressHandle.WaitAsync(); } } diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs index f66ff519b..01bf70190 100644 --- a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs +++ b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs @@ -1,22 +1,18 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT License. +using System.Threading; +using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Microsoft.PowerShell.EditorServices.Services; -using Microsoft.PowerShell.EditorServices.Services.DebugAdapter; using Microsoft.PowerShell.EditorServices.Services.PowerShell; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging; using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution; -using Microsoft.PowerShell.EditorServices.Services.PowerShell.Runspace; using Microsoft.PowerShell.EditorServices.Services.TextDocument; using Microsoft.PowerShell.EditorServices.Utility; using OmniSharp.Extensions.DebugAdapter.Protocol.Events; using OmniSharp.Extensions.DebugAdapter.Protocol.Requests; using OmniSharp.Extensions.DebugAdapter.Protocol.Server; -using System.Management.Automation; -using System.Management.Automation.Language; -using System.Threading; -using System.Threading.Tasks; namespace Microsoft.PowerShell.EditorServices.Handlers { @@ -38,9 +34,7 @@ internal class ConfigurationDoneHandler : IConfigurationDoneHandler private readonly DebugEventHandlerService _debugEventHandlerService; private readonly IInternalPowerShellExecutionService _executionService; private readonly WorkspaceService _workspaceService; - private readonly IPowerShellDebugContext _debugContext; - private readonly IRunspaceContext _runspaceContext; public ConfigurationDoneHandler( ILoggerFactory loggerFactory, @@ -50,8 +44,7 @@ public ConfigurationDoneHandler( DebugEventHandlerService debugEventHandlerService, IInternalPowerShellExecutionService executionService, WorkspaceService workspaceService, - IPowerShellDebugContext debugContext, - IRunspaceContext runspaceContext) + IPowerShellDebugContext debugContext) { _logger = loggerFactory.CreateLogger(); _debugAdapterServer = debugAdapterServer; @@ -61,23 +54,18 @@ public ConfigurationDoneHandler( _executionService = executionService; _workspaceService = workspaceService; _debugContext = debugContext; - _runspaceContext = runspaceContext; } public Task Handle(ConfigurationDoneArguments request, CancellationToken cancellationToken) { _debugService.IsClientAttached = true; - if (_debugStateService.OwnsEditorSession) - { - // TODO: If this is a debug-only session, we need to start the command loop manually - // - //_powerShellContextService.ConsoleReader.StartCommandLoop(); - } - if (!string.IsNullOrEmpty(_debugStateService.ScriptToLaunch)) { - LaunchScriptAsync(_debugStateService.ScriptToLaunch).HandleErrorsAsync(_logger); + // NOTE: This is an unawaited task because responding to "configuration done" means + // setting up the debugger, and in our case that means starting the script but not + // waiting for it to finish. + Task _ = LaunchScriptAsync(_debugStateService.ScriptToLaunch).HandleErrorsAsync(_logger); } if (_debugStateService.IsInteractiveDebugSession && _debugService.IsDebuggerStopped) @@ -102,48 +90,18 @@ public Task Handle(ConfigurationDoneArguments request private async Task LaunchScriptAsync(string scriptToLaunch) { - // Is this an untitled script? - if (ScriptFile.IsUntitledPath(scriptToLaunch)) - { - ScriptFile untitledScript = _workspaceService.GetFile(scriptToLaunch); - - if (BreakpointApiUtils.SupportsBreakpointApis(_runspaceContext.CurrentRunspace)) - { - // Parse untitled files with their `Untitled:` URI as the file name which will cache the URI & contents within the PowerShell parser. - // By doing this, we light up the ability to debug Untitled files with breakpoints. - // This is only possible via the direct usage of the breakpoint APIs in PowerShell because - // Set-PSBreakpoint validates that paths are actually on the filesystem. - ScriptBlockAst ast = Parser.ParseInput(untitledScript.Contents, untitledScript.DocumentUri.ToString(), out Token[] tokens, out ParseError[] errors); - - // This seems to be the simplest way to invoke a script block (which contains breakpoint information) via the PowerShell API. - // - // TODO: Fix this so the added script doesn't show up. - var cmd = new PSCommand().AddScript(". $args[0]").AddArgument(ast.GetScriptBlock()); - await _executionService - .ExecutePSCommandAsync(cmd, CancellationToken.None, s_debuggerExecutionOptions) - .ConfigureAwait(false); - } - else - { - await _executionService - .ExecutePSCommandAsync( - new PSCommand().AddScript(untitledScript.Contents), - CancellationToken.None, - s_debuggerExecutionOptions) - .ConfigureAwait(false); - } - } - else - { - // TODO: Fix this so the added script doesn't show up. - await _executionService - .ExecutePSCommandAsync( - PSCommandHelpers.BuildCommandFromArguments(scriptToLaunch, _debugStateService.Arguments), - CancellationToken.None, - s_debuggerExecutionOptions) - .ConfigureAwait(false); - } + // TODO: Theoretically we can make PowerShell respect line breakpoints in untitled + // files, but the previous method was a hack that conflicted with correct passing of + // arguments to the debugged script. We are prioritizing the latter over the former, as + // command breakpoints and `Wait-Debugger` work fine. + string command = ScriptFile.IsUntitledPath(scriptToLaunch) + ? string.Concat("{ ", _workspaceService.GetFile(scriptToLaunch).Contents, " }") + : string.Concat('"', scriptToLaunch, '"'); + await _executionService.ExecutePSCommandAsync( + PSCommandHelpers.BuildCommandFromArguments(command, _debugStateService.Arguments), + CancellationToken.None, + s_debuggerExecutionOptions).ConfigureAwait(false); _debugAdapterServer.SendNotification(EventNames.Terminated); } } diff --git a/src/PowerShellEditorServices/Utility/ArgumentUtils.cs b/src/PowerShellEditorServices/Utility/ArgumentUtils.cs deleted file mode 100644 index dd73f4d52..000000000 --- a/src/PowerShellEditorServices/Utility/ArgumentUtils.cs +++ /dev/null @@ -1,40 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using System.Text; -using System.Management.Automation.Language; - -namespace Microsoft.PowerShell.EditorServices.Utility -{ - internal static class ArgumentEscaping - { - /// - /// Escape a PowerShell argument while still making it able to be evaluated in AddScript. - /// - /// NOTE: This does not "sanitize" parameters, e.g., a pipe in one argument might affect another argument. - /// This is intentional to give flexibility to specifying arguments. - /// It also does not try to fix invalid PowerShell syntax, e.g., a single quote in a string literal. - /// - public static string Escape(string Arg) - { - // if argument is a scriptblock return as-is - if (Arg.StartsWith("{") && Arg.EndsWith("}")) - { - return Arg; - } - - // If argument has a space enclose it in quotes unless it is already quoted - if (Arg.Contains(" ")) - { - if (Arg.StartsWith("\"") && Arg.EndsWith("\"") || Arg.StartsWith("'") && Arg.EndsWith("'")) - { - return Arg; - } - - return "\"" + Arg + "\""; - } - - return Arg; - } - } -} diff --git a/src/PowerShellEditorServices/Utility/PSCommandExtensions.cs b/src/PowerShellEditorServices/Utility/PSCommandExtensions.cs index bc5a53d05..1d5186184 100644 --- a/src/PowerShellEditorServices/Utility/PSCommandExtensions.cs +++ b/src/PowerShellEditorServices/Utility/PSCommandExtensions.cs @@ -129,25 +129,11 @@ private static StringBuilder AddCommandText(this StringBuilder sb, Command comma return sb; } - public static PSCommand BuildCommandFromArguments(string command, IReadOnlyList arguments) + public static PSCommand BuildCommandFromArguments(string command, IEnumerable arguments) { // HACK: We use AddScript instead of AddArgument/AddParameter to reuse Powershell parameter binding logic. - // We quote the command parameter so that expressions can still be used in the arguments. - var sb = new StringBuilder() - .Append('.') - .Append(' ') - .Append('"') - .Append(command) - .Append('"'); - - foreach (string arg in arguments ?? System.Linq.Enumerable.Empty()) - { - sb - .Append(' ') - .Append(ArgumentEscaping.Escape(arg)); - } - - return new PSCommand().AddScript(sb.ToString()); + string script = string.Concat(". ", command, " ", string.Join(" ", arguments ?? Array.Empty())); + return new PSCommand().AddScript(script); } } } diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs index 00996db96..05a98c7b0 100644 --- a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs +++ b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs @@ -96,7 +96,7 @@ private VariableDetailsBase[] GetVariables(string scopeName) private Task ExecutePowerShellCommand(string command, params string[] args) { return psesHost.ExecutePSCommandAsync( - PSCommandHelpers.BuildCommandFromArguments(command, args), + PSCommandHelpers.BuildCommandFromArguments(string.Concat('"', command, '"'), args), CancellationToken.None); } @@ -176,8 +176,16 @@ await debugService.SetCommandBreakpointsAsync( Assert.Equal("[ArrayList: 0]", var.ValueString); } - [Fact] - public async Task DebuggerAcceptsScriptArgs() + // See https://www.thomasbogholm.net/2021/06/01/convenient-member-data-sources-with-xunit/ + public static IEnumerable DebuggerAcceptsScriptArgsTestData => new List() + { + new object[] { new object[] { "Foo -Param2 @('Bar','Baz') -Force Extra1" } }, + new object[] { new object[] { "Foo", "-Param2", "@('Bar','Baz')", "-Force", "Extra1" } } + }; + + [Theory] + [MemberData(nameof(DebuggerAcceptsScriptArgsTestData))] + public async Task DebuggerAcceptsScriptArgs(string[] args) { // The path is intentionally odd (some escaped chars but not all) because we are testing // the internal path escaping mechanism - it should escape certains chars ([, ] and space) but @@ -197,9 +205,6 @@ public async Task DebuggerAcceptsScriptArgs() Assert.True(breakpoint.Verified); }); - // TODO: This test used to also pass the args as a single string, but that doesn't seem - // to work any more. Perhaps that's a bug? - var args = new[] { "Foo", "-Param2", "@('Bar','Baz')", "-Force", "Extra1" }; Task _ = ExecutePowerShellCommand(debugWithParamsFile.FilePath, args); AssertDebuggerStopped(debugWithParamsFile.FilePath, 3); diff --git a/test/PowerShellEditorServices.Test/Utility/ArgumentEscapingTests.cs b/test/PowerShellEditorServices.Test/Utility/ArgumentEscapingTests.cs deleted file mode 100644 index d6c155211..000000000 --- a/test/PowerShellEditorServices.Test/Utility/ArgumentEscapingTests.cs +++ /dev/null @@ -1,69 +0,0 @@ -// Copyright (c) Microsoft Corporation. -// Licensed under the MIT License. - -using Xunit; -using Microsoft.PowerShell.EditorServices.Utility; -using System.IO; -using System.Management.Automation; -using System.Linq; - -namespace Microsoft.PowerShell.EditorServices.Test.Session -{ - public class ArgumentEscapingTests - { - [Trait("Category", "ArgumentEscaping")] - [Theory] - [InlineData(" has spaces", "\" has spaces\"")] - [InlineData("-Parameter", "-Parameter")] - [InlineData("' single quote left alone'", "' single quote left alone'")] - [InlineData("\"double quote left alone\"", "\"double quote left alone\"")] - [InlineData("/path/to/fi le", "\"/path/to/fi le\"")] - [InlineData("'/path/to/fi le'", "'/path/to/fi le'")] - [InlineData("|pipeline", "|pipeline")] - [InlineData("am&pe rsand", "\"am&pe rsand\"")] - [InlineData("semicolon ;", "\"semicolon ;\"")] - [InlineData(": colon", "\": colon\"")] - [InlineData("$(expressions should be quoted)", "\"$(expressions should be quoted)\"")] - [InlineData("{scriptBlocks should not have escaped-spaces}", "{scriptBlocks should not have escaped-spaces}")] - [InlineData("-Parameter test", "\"-Parameter test\"")] //This is invalid, but should be obvious enough looking at the PSIC invocation - public void CorrectlyEscapesPowerShellArguments(string Arg, string expectedArg) - { - string quotedArg = ArgumentEscaping.Escape(Arg); - Assert.Equal(expectedArg, quotedArg); - } - - [Trait("Category", "ArgumentEscaping")] - [Theory] - [InlineData("/path/t o/file", "/path/t o/file")] - [InlineData("/path/with/$(echo 'expression')inline", "/path/with/expressioninline")] - [InlineData("/path/with/$(echo 'expression') inline", "/path/with/expression inline")] - [InlineData("am&per sand", "am&per sand")] - [InlineData("'inner\"\"quotes'", "inner\"\"quotes")] - public void CanEvaluateArguments(string Arg, string expectedOutput) - { - var escapedArg = ArgumentEscaping.Escape(Arg); - var psCommand = new PSCommand().AddScript($"& Write-Output {escapedArg}"); - using var pwsh = System.Management.Automation.PowerShell.Create(); - pwsh.Commands = psCommand; - var scriptOutput = pwsh.Invoke().First(); - Assert.Equal(expectedOutput, scriptOutput); - } - - [Trait("Category", "ArgumentEscaping")] - [Theory] - [InlineData("NormalScript.ps1")] - [InlineData("Bad&name4script.ps1")] - [InlineData("[Truly] b&d `Name_4_script.ps1")] - public void CanDotSourcePath(string rawFileName) - { - var ScriptAssetPath = @"..\..\..\..\PowerShellEditorServices.Test.Shared\scriptassets"; - var fullPath = Path.Combine(ScriptAssetPath, rawFileName); - var escapedPath = PathUtils.WildcardEscapePath(fullPath).ToString(); - var psCommand = new PSCommand().AddScript($"& \"{escapedPath}\""); - - using var pwsh = System.Management.Automation.PowerShell.Create(); - pwsh.Commands = psCommand; - pwsh.Invoke(); - } - } -}