Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jan 3, 2022
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d827121
Fix regression with `F5` to use `.` instead of `&` operator
andyleejordan Dec 29, 2021
7d8f23c
Apply suggestions from code review
andyleejordan Jan 3, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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();
var 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.
Expand Down Expand Up @@ -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();
var 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}'.");
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/PowerShellEditorServices/Services/DebugAdapter/Handlers/ConfigurationDoneHandler.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
9 changes: 2 additions & 7 deletions src/PowerShellEditorServices/Utility/PSCommandExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(' ')
Expand Down
91 changes: 40 additions & 51 deletions test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand Down Expand Up @@ -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);
Expand All @@ -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");
Expand Down Expand Up @@ -580,7 +580,7 @@ await debugService.SetLineBreakpointsAsync(
}

[Trait("Category", "DebugService")]
[Fact(Skip = "Variable setting is broken")]
[Fact]
public async Task DebuggerSetsVariablesNoConversion()
{
await debugService.SetLineBreakpointsAsync(
Expand All @@ -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";
Expand All @@ -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(
Expand All @@ -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";
Expand All @@ -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);
}
Expand Down