Fix regression with F5 to use . instead of & operator (#1652)

This also corrects the scope of the script's parameter variables to be `Local` instead of `Command` in the `DebuggerAcceptsScriptArgs` test, and `Local` instead of `Auto` for most of the rest of the tests (as they were previous to the pipeline work). Excitingly, this even fixes the `DebuggerSetsVariablesNoConversion` test, although `DebuggerSetsVariablesWithConversion` is still broken (for a different reason). Co-authored-by: Patrick Meinecke <[email protected]>
PowerShell · Jan 3, 2022 · 1cf808f · 1cf808f
1 parent 3c4e48e
commit 1cf808f
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 63 deletions.
diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs b/src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
@@ -45,7 +45,7 @@ internal class DebugService
         private int nextVariableId;
         private string temporaryScriptListingPath;
         private List<VariableDetailsBase> variables;
-        internal VariableContainerDetails globalScopeVariables; // Internal for unit testing.
+        private VariableContainerDetails globalScopeVariables;
         private VariableContainerDetails scriptScopeVariables;
         private VariableContainerDetails localScopeVariables;
         private StackFrameDetails[] stackFrameDetails;
@@ -372,9 +372,12 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
             // Evaluate the expression to get back a PowerShell object from the expression string.
             // This may throw, in which case the exception is propagated to the caller
             PSCommand evaluateExpressionCommand = new PSCommand().AddScript(value);
-            object expressionResult =
-                (await _executionService.ExecutePSCommandAsync<object>(evaluateExpressionCommand, CancellationToken.None)
-                .ConfigureAwait(false)).FirstOrDefault();
+            IReadOnlyList<object> expressionResults = await _executionService.ExecutePSCommandAsync<object>(evaluateExpressionCommand, CancellationToken.None).ConfigureAwait(false);
+            if (expressionResults.Count == 0)
+            {
+                throw new InvalidPowerShellExpressionException("Expected an expression result.");
+            }
+            object expressionResult = expressionResults[0];
 
             // If PowerShellContext.ExecuteCommand returns an ErrorRecord as output, the expression failed evaluation.
             // Ideally we would have a separate means from communicating error records apart from normal output.
@@ -423,7 +426,13 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
                 .AddParameter("Name", name.TrimStart('$'))
                 .AddParameter("Scope", scope);
 
-            PSVariable psVariable = (await _executionService.ExecutePSCommandAsync<PSVariable>(getVariableCommand, CancellationToken.None).ConfigureAwait(false)).FirstOrDefault();
+            IReadOnlyList<PSVariable> psVariables = await _executionService.ExecutePSCommandAsync<PSVariable>(getVariableCommand, CancellationToken.None).ConfigureAwait(false);
+            if (psVariables.Count == 0)
+            {
+                throw new Exception("Failed to retrieve PSVariables");
+            }
+
+            PSVariable psVariable = psVariables[0];
             if (psVariable is null)
             {
                 throw new Exception($"Failed to retrieve PSVariable object for '{name}' from scope '{scope}'.");
@@ -449,6 +458,8 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
             {
                 _logger.LogTrace($"Setting variable '{name}' using conversion to value: {expressionResult ?? "<null>"}");
 
+                // TODO: This is throwing a 'PSInvalidOperationException' thus causing
+                // 'DebuggerSetsVariablesWithConversion' to fail.
                 psVariable.Value = await _executionService.ExecuteDelegateAsync(
                     "PS debugger argument converter",
                     ExecutionOptions.Default,

diff --git a/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs b/src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs
@@ -135,6 +135,7 @@ await _executionService
             }
             else
             {
+                // TODO: Fix this so the added script doesn't show up.
                 await _executionService
                     .ExecutePSCommandAsync(
                         PSCommandHelpers.BuildCommandFromArguments(scriptToLaunch, _debugStateService.Arguments),

diff --git a/src/PowerShellEditorServices/Utility/PSCommandExtensions.cs b/src/PowerShellEditorServices/Utility/PSCommandExtensions.cs
@@ -129,21 +129,16 @@ private static StringBuilder AddCommandText(this StringBuilder sb, Command comma
 
         public static PSCommand BuildCommandFromArguments(string command, IReadOnlyList<string> arguments)
         {
-            if (arguments is null or { Count: 0 })
-            {
-                return new PSCommand().AddCommand(command);
-            }
-
             // 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('"')
                 .Append(command)
                 .Append('"');
 
-            foreach (string arg in arguments)
+            foreach (string arg in arguments ?? System.Linq.Enumerable.Empty<string>())
             {
                 sb
                 .Append(' ')

diff --git a/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs b/test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs
@@ -90,6 +90,14 @@ private ScriptFile GetDebugScript(string fileName)
                 )));
         }
 
+        private VariableDetailsBase[] GetVariables(string scopeName)
+        {
+            VariableScope scope = Array.Find(
+                debugService.GetVariableScopes(0),
+                s => s.Name == scopeName);
+            return debugService.GetVariables(scope.Id);
+        }
+
         private Task ExecutePowerShellCommand(string command, params string[] args)
         {
             return psesHost.ExecutePSCommandAsync(
@@ -158,10 +166,10 @@ await debugService.SetCommandBreakpointsAsync(
 
             StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
             Assert.Equal(StackFrameDetails.NoFileScriptPath, stackFrames[0].ScriptPath);
-            VariableDetailsBase[] variables = debugService.GetVariables(debugService.globalScopeVariables.Id);
 
             // NOTE: This assertion will fail if any error occurs. Notably this happens in testing
             // when the assembly path changes and the commands definition file can't be found.
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.GlobalScopeName);
             var var = Array.Find(variables, v => v.Name == "$Error");
             Assert.NotNull(var);
             Assert.True(var.IsExpandable);
@@ -197,8 +205,7 @@ public async Task DebuggerAcceptsScriptArgs()
 
             AssertDebuggerStopped(debugWithParamsFile.FilePath, 3);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             var var = Array.Find(variables, v => v.Name == "$Param1");
             Assert.NotNull(var);
@@ -220,6 +227,7 @@ public async Task DebuggerAcceptsScriptArgs()
             Assert.True(var.IsExpandable);
 
             // NOTE: $args are no longer found in AutoVariables but CommandVariables instead.
+            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
             variables = debugService.GetVariables(stackFrames[0].CommandVariables.Id);
             var = Array.Find(variables, v => v.Name == "$args");
             Assert.NotNull(var);
@@ -266,8 +274,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints()
             Task _ = ExecuteDebugFile();
             AssertDebuggerStopped(debugScriptFile.FilePath, 6);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Verify the function breakpoint broke at Write-Host and $i is 1
             var i = Array.Find(variables, v => v.Name == "$i");
@@ -279,8 +286,7 @@ public async Task DebuggerStopsOnFunctionBreakpoints()
             debugService.Continue();
             AssertDebuggerStopped(debugScriptFile.FilePath, 6);
 
-            stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Verify the function breakpoint broke at Write-Host and $i is 1
             i = Array.Find(variables, v => v.Name == "$i");
@@ -356,8 +362,7 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteDebugFile();
             AssertDebuggerStopped(debugScriptFile.FilePath, 7);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1
             var i = Array.Find(variables, v => v.Name == "$i");
@@ -370,8 +375,7 @@ await debugService.SetLineBreakpointsAsync(
             debugService.Continue();
             AssertDebuggerStopped(debugScriptFile.FilePath, 7);
 
-            stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1
             i = Array.Find(variables, v => v.Name == "$i");
@@ -395,8 +399,7 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteDebugFile();
             AssertDebuggerStopped(debugScriptFile.FilePath, 6);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1
             var i = Array.Find(variables, v => v.Name == "$i");
@@ -418,8 +421,7 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteDebugFile();
             AssertDebuggerStopped(debugScriptFile.FilePath, 6);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Verify the breakpoint only broke at the condition ie. $i -eq breakpointValue1
             var i = Array.Find(variables, v => v.Name == "$i");
@@ -519,8 +521,7 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteVariableScriptFile();
             AssertDebuggerStopped(variableScriptFile.FilePath);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             var var = Array.Find(variables, v => v.Name == "$strVar");
             Assert.NotNull(var);
@@ -539,8 +540,7 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteVariableScriptFile();
             AssertDebuggerStopped(variableScriptFile.FilePath);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // TODO: Add checks for correct value strings as well
             var strVar = Array.Find(variables, v => v.Name == "$strVar");
@@ -580,7 +580,7 @@ await debugService.SetLineBreakpointsAsync(
         }
 
         [Trait("Category", "DebugService")]
-        [Fact(Skip = "Variable setting is broken")]
+        [Fact]
         public async Task DebuggerSetsVariablesNoConversion()
         {
             await debugService.SetLineBreakpointsAsync(
@@ -590,16 +590,16 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteVariableScriptFile();
             AssertDebuggerStopped(variableScriptFile.FilePath);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableScope[] scopes = debugService.GetVariableScopes(0);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Test set of a local string variable (not strongly typed)
-            const string newStrValue = "\"Goodbye\"";
-            string setStrValue = await debugService.SetVariableAsync(stackFrames[0].AutoVariables.Id, "$strVar", newStrValue).ConfigureAwait(true);
+            const string newStrValue = "Goodbye";
+            VariableScope localScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.LocalScopeName);
+            // TODO: Fix this so it has the second quotes again?
+            string setStrValue = await debugService.SetVariableAsync(localScope.Id, "$strVar", '"' + newStrValue + '"').ConfigureAwait(true);
             Assert.Equal(newStrValue, setStrValue);
 
-            VariableScope[] scopes = debugService.GetVariableScopes(0);
-
             // Test set of script scope int variable (not strongly typed)
             VariableScope scriptScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.ScriptScopeName);
             const string newIntValue = "49";
@@ -615,33 +615,27 @@ await debugService.SetLineBreakpointsAsync(
 
             // The above just tests that the debug service returns the correct new value string.
             // Let's step the debugger and make sure the values got set to the new values.
-            debugService.StepOver();
+            await Task.Run(() => debugService.StepOver()).ConfigureAwait(true);
             AssertDebuggerStopped(variableScriptFile.FilePath);
 
-            stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-
             // Test set of a local string variable (not strongly typed)
-            variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            variables = GetVariables(VariableContainerDetails.LocalScopeName);
             var strVar = Array.Find(variables, v => v.Name == "$strVar");
             Assert.Equal(newStrValue, strVar.ValueString);
 
-            scopes = debugService.GetVariableScopes(0);
-
             // Test set of script scope int variable (not strongly typed)
-            scriptScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.ScriptScopeName);
-            variables = debugService.GetVariables(scriptScope.Id);
+            variables = GetVariables(VariableContainerDetails.ScriptScopeName);
             var intVar = Array.Find(variables, v => v.Name == "$scriptInt");
             Assert.Equal(newIntValue, intVar.ValueString);
 
             // Test set of global scope int variable (not strongly typed)
-            globalScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.GlobalScopeName);
-            variables = debugService.GetVariables(globalScope.Id);
+            variables = GetVariables(VariableContainerDetails.GlobalScopeName);
             var intGlobalVar = Array.Find(variables, v => v.Name == "$MaximumHistoryCount");
             Assert.Equal(newGlobalIntValue, intGlobalVar.ValueString);
         }
 
         [Trait("Category", "DebugService")]
-        [Fact(Skip = "Variable setting is broken")]
+        [Fact(Skip = "Variable conversion is broken")]
         public async Task DebuggerSetsVariablesWithConversion()
         {
             await debugService.SetLineBreakpointsAsync(
@@ -652,17 +646,16 @@ await debugService.SetLineBreakpointsAsync(
             Task _ = ExecuteVariableScriptFile();
             AssertDebuggerStopped(variableScriptFile.FilePath);
 
-            StackFrameDetails[] stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-            VariableDetailsBase[] variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            VariableScope[] scopes = debugService.GetVariableScopes(0);
+            VariableDetailsBase[] variables = GetVariables(VariableContainerDetails.LocalScopeName);
 
             // Test set of a local string variable (not strongly typed but force conversion)
-            const string newStrValue = "\"False\"";
+            const string newStrValue = "False";
             const string newStrExpr = "$false";
-            string setStrValue = await debugService.SetVariableAsync(stackFrames[0].AutoVariables.Id, "$strVar2", newStrExpr).ConfigureAwait(true);
+            VariableScope localScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.LocalScopeName);
+            string setStrValue = await debugService.SetVariableAsync(localScope.Id, "$strVar2", newStrExpr).ConfigureAwait(true);
             Assert.Equal(newStrValue, setStrValue);
 
-            VariableScope[] scopes = debugService.GetVariableScopes(0);
-
             // Test set of script scope bool variable (strongly typed)
             VariableScope scriptScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.ScriptScopeName);
             const string newBoolValue = "$true";
@@ -679,27 +672,23 @@ await debugService.SetLineBreakpointsAsync(
 
             // The above just tests that the debug service returns the correct new value string.
             // Let's step the debugger and make sure the values got set to the new values.
-            debugService.StepOver();
+            await Task.Run(() => debugService.StepOver()).ConfigureAwait(true);
             AssertDebuggerStopped(variableScriptFile.FilePath);
 
-            stackFrames = await debugService.GetStackFramesAsync().ConfigureAwait(true);
-
             // Test set of a local string variable (not strongly typed but force conversion)
-            variables = debugService.GetVariables(stackFrames[0].AutoVariables.Id);
+            variables = GetVariables(VariableContainerDetails.LocalScopeName);
             var strVar = Array.Find(variables, v => v.Name == "$strVar2");
             Assert.Equal(newStrValue, strVar.ValueString);
 
             scopes = debugService.GetVariableScopes(0);
 
             // Test set of script scope bool variable (strongly typed)
-            scriptScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.ScriptScopeName);
-            variables = debugService.GetVariables(scriptScope.Id);
+            variables = GetVariables(VariableContainerDetails.ScriptScopeName);
             var boolVar = Array.Find(variables, v => v.Name == "$scriptBool");
             Assert.Equal(newBoolValue, boolVar.ValueString);
 
             // Test set of global scope ActionPreference variable (strongly typed)
-            globalScope = Array.Find(scopes, s => s.Name == VariableContainerDetails.GlobalScopeName);
-            variables = debugService.GetVariables(globalScope.Id);
+            variables = GetVariables(VariableContainerDetails.GlobalScopeName);
             var globalVar = Array.Find(variables, v => v.Name == "$VerbosePreference");
             Assert.Equal(newGlobalValue, globalVar.ValueString);
         }