Remove argument escaping logic from script launching

Attempting to be "helpful" by escaping arguments proved to cause more
issues than it solved. We can instead massively simplify our script
launching logic (and fix yet another test that had a "maybe bug") by
just launching what the user gave us. This should also be easier for the
user to debug.

Author: andyleejordan

src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs

@@ -102,35 +102,13 @@ public Task<ConfigurationDoneResponse> Handle(ConfigurationDoneArguments request
 
         private async Task LaunchScriptAsync(string scriptToLaunch)
         {
-            PSCommand cmd;
-            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 and 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 _,
-                        out _);
-                    // TODO: This breaks the ability to debug untitled files with breakpoints.
-                    cmd = PSCommandHelpers.BuildCommandFromArguments(ast.GetScriptBlock().ToString(), _debugStateService.Arguments, isScriptBlock: true);
-                }
-                else
-                {
-                    cmd = PSCommandHelpers.BuildCommandFromArguments(untitledScript.Contents, _debugStateService.Arguments, isScriptBlock: true);
-                }
-            }
-            else
-            {
-                cmd = PSCommandHelpers.BuildCommandFromArguments(scriptToLaunch, _debugStateService.Arguments, isScriptBlock: 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.
+            PSCommand cmd = ScriptFile.IsUntitledPath(scriptToLaunch)
+                ? PSCommandHelpers.BuildCommandFromArguments(string.Concat("{ ", _workspaceService.GetFile(scriptToLaunch).Contents, " }"), _debugStateService.Arguments)
+                : PSCommandHelpers.BuildCommandFromArguments(string.Concat('"', scriptToLaunch, '"'), _debugStateService.Arguments);
             await _executionService.ExecutePSCommandAsync(cmd, CancellationToken.None, s_debuggerExecutionOptions).ConfigureAwait(false);
             _debugAdapterServer.SendNotification(EventNames.Terminated);
         }

src/PowerShellEditorServices/Utility/ArgumentUtils.cs

@@ -129,25 +129,10 @@ private static StringBuilder AddCommandText(this StringBuilder sb, Command comma
             return sb;
         }
 
-        public static PSCommand BuildCommandFromArguments(string command, IReadOnlyList<string> arguments, bool isScriptBlock = false)
+        public static PSCommand BuildCommandFromArguments(string command, IEnumerable<string> arguments = default)
         {
             // 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(isScriptBlock ? "{ " : '"')
-                .Append(command)
-                .Append(isScriptBlock ? " }" : '"');
-
-            foreach (string arg in arguments ?? System.Linq.Enumerable.Empty<string>())
-            {
-                sb
-                .Append(' ')
-                .Append(ArgumentEscaping.Escape(arg));
-            }
-
-            return new PSCommand().AddScript(sb.ToString());
+            return new PSCommand().AddScript(string.Concat(". ", command, " ", string.Join(" ", arguments)));
         }
     }
 }

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