Skip to content

Fix uncaught exception when SafeToString returns null #1140

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.

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 1 commit into from
Jan 3, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -194,9 +194,9 @@ private static string GetValueStringAndType(object value, bool isExpandable, out
else
{
string valueToString = value.SafeToString();
if (valueToString.Equals(objType.ToString()))
if (valueToString == null || valueToString.Equals(objType.ToString()))
{
// If the ToString() matches the type name, then display the type
// If the ToString() matches the type name or is null, then display the type
// name in PowerShell format.
string shortTypeName = objType.Name;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ function Test-Variables {
$classVar.Name = "Test"
$classVar.Number = 42;
$enumVar = $ErrorActionPreference
$nullString = [NullString]::Value
$psObjVar = New-Object -TypeName PSObject -Property @{Name = 'John'; Age = 75}
$psCustomObjVar = [PSCustomObject] @{Name = 'Paul'; Age = 73}
$procVar = Get-Process system
Expand Down
28 changes: 28 additions & 0 deletions test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -815,6 +815,34 @@ await this.debugService.SetLineBreakpointsAsync(
this.powerShellContext.AbortExecution();
}

[Fact]
public async Task DebufferVariableNullStringDisplaysCorrectly()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests don't actually get run in CI as of right now because of the large migration but you don't need to worry about that. I'm going to enable them again soon.

Thanks for adding this!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to run it locally but honestly wasn't sure if it actually ran, had no failures (that were related to this though).

{
await this.debugService.SetLineBreakpointsAsync(
this.variableScriptFile,
new[] { BreakpointDetails.Create("", 18) });

// Execute the script and wait for the breakpoint to be hit
Task executeTask =
this.powerShellContext.ExecuteScriptStringAsync(
this.variableScriptFile.FilePath);

await this.AssertDebuggerStopped(this.variableScriptFile.FilePath);

StackFrameDetails[] stackFrames = debugService.GetStackFrames();

VariableDetailsBase[] variables =
debugService.GetVariables(stackFrames[0].LocalVariables.Id);

var nullStringVar = variables.FirstOrDefault(v => v.Name == "$nullString");
Assert.NotNull(nullStringVar);
Assert.True("[NullString]".Equals(nullStringVar.ValueString));
Assert.True(nullStringVar.IsExpandable);

// Abort execution of the script
this.powerShellContext.AbortExecution();
}

[Fact]
public async Task DebuggerVariablePSObjectDisplaysCorrectly()
{
Expand Down