Merge pull request #1702 from PowerShell/andschwa/untitled-with-args

Fix running untitled scripts with arguments (but break line breakpoints)

Author: andyleejordan

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);
         }
 

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<bool> _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<bool>();
             _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<LaunchAndAttachHandler>()
                     .WithHandler<DisconnectHandler>()

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<PsesInternalHost>();
 
@@ -74,10 +73,7 @@ public static IServiceCollection AddPsesDebugServices(
                 .AddSingleton<PsesDebugServer>(psesDebugServer)
                 .AddSingleton<DebugService>()
                 .AddSingleton<BreakpointService>()
-                .AddSingleton<DebugStateService>(new DebugStateService
-                {
-                     OwnsEditorSession = useTempSession
-                })
+                .AddSingleton<DebugStateService>()
                 .AddSingleton<DebugEventHandlerService>();
         }
     }

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<bool> 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();
     }
 }

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<ConfigurationDoneHandler>();
             _debugAdapterServer = debugAdapterServer;
@@ -61,23 +54,18 @@ public ConfigurationDoneHandler(
             _executionService = executionService;
             _workspaceService = workspaceService;
             _debugContext = debugContext;
-            _runspaceContext = runspaceContext;
         }
 
         public Task<ConfigurationDoneResponse> 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<ConfigurationDoneResponse> 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<object>(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);
         }
     }

src/PowerShellEditorServices/Utility/ArgumentUtils.cs

@@ -129,25 +129,11 @@ private static StringBuilder AddCommandText(this StringBuilder sb, Command comma
             return sb;
         }
 
-        public static PSCommand BuildCommandFromArguments(string command, IReadOnlyList<string> arguments)
+        public static PSCommand BuildCommandFromArguments(string command, IEnumerable<string> 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<string>())
-            {
-                sb
-                .Append(' ')
-                .Append(ArgumentEscaping.Escape(arg));
-            }
-
-            return new PSCommand().AddScript(sb.ToString());
+            string script = string.Concat(". ", command, " ", string.Join(" ", arguments ?? Array.Empty<string>()));
+            return new PSCommand().AddScript(script);
         }
     }
 }

src/PowerShellEditorServices/Utility/PSCommandExtensions.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<object[]> DebuggerAcceptsScriptArgsTestData => new List<object[]>()
+        {
+            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);