Skip to content

Fix tests in Debugging/DebugServiceTests.cs and simplify faulty script path logic #1540

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 8 commits into from
Aug 12, 2021
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
2 changes: 2 additions & 0 deletions .editorconfig
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ dotnet_analyzer_diagnostic.category-Maintainability.severity = error
dotnet_diagnostic.VSTHRD002.severity = silent
# VSTHRD200: Use "Async" suffix for awaitable methods
dotnet_diagnostic.VSTHRD200.severity = silent
# IDE0003: this and Me preferences
dotnet_diagnostic.IDE0003.severity = silent

[*.{json}]
indent_size = 2
Expand Down
20 changes: 12 additions & 8 deletions src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
using System.Management.Automation.Language;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using Microsoft.PowerShell.EditorServices.Utility;
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Extensions.Logging;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Microsoft.PowerShell.EditorServices.Services.PowerShellContext;
using Microsoft.PowerShell.EditorServices.Services.DebugAdapter;
using Microsoft.PowerShell.EditorServices.Services.PowerShellContext;
using Microsoft.PowerShell.EditorServices.Services.TextDocument;
using Microsoft.PowerShell.EditorServices.Utility;

namespace Microsoft.PowerShell.EditorServices.Services
{
Expand All @@ -28,7 +28,6 @@ internal class DebugService

private const string PsesGlobalVariableNamePrefix = "__psEditorServices_";
private const string TemporaryScriptFileName = "Script Listing.ps1";
private readonly BreakpointDetails[] s_emptyBreakpointDetailsArray = new BreakpointDetails[0];

private readonly ILogger logger;
private readonly PowerShellContextService powerShellContext;
Expand Down Expand Up @@ -150,7 +149,7 @@ public async Task<BreakpointDetails[]> SetLineBreakpointsAsync(
this.logger.LogTrace(
$"Could not set breakpoints for local path '{scriptPath}' in a remote session.");

return s_emptyBreakpointDetailsArray;
return Array.Empty<BreakpointDetails>();
}

string mappedPath =
Expand All @@ -167,7 +166,7 @@ public async Task<BreakpointDetails[]> SetLineBreakpointsAsync(
this.logger.LogTrace(
$"Could not set breakpoint on temporary script listing path '{scriptPath}'.");

return s_emptyBreakpointDetailsArray;
return Array.Empty<BreakpointDetails>();
}

// Fix for issue #123 - file paths that contain wildcard chars [ and ] need to
Expand Down Expand Up @@ -216,7 +215,7 @@ await _breakpointService.RemoveBreakpointsAsync(
resultBreakpointDetails = (await _breakpointService.SetCommandBreakpoints(breakpoints).ConfigureAwait(false)).ToArray();
}

return resultBreakpointDetails ?? new CommandBreakpointDetails[0];
return resultBreakpointDetails ?? Array.Empty<CommandBreakpointDetails>();
}

/// <summary>
Expand Down Expand Up @@ -988,6 +987,7 @@ private void OnBreakpointUpdated(object sender, BreakpointUpdatedEventArgs e)
// this for CommandBreakpoint, as those span all script files.
if (e.Breakpoint is LineBreakpoint lineBreakpoint)
{
// TODO: This could be either a path or a script block!
string scriptPath = lineBreakpoint.Script;
if (this.powerShellContext.CurrentRunspace.Location == RunspaceLocation.Remote &&
this.remoteFileManager != null)
Expand All @@ -1008,6 +1008,10 @@ private void OnBreakpointUpdated(object sender, BreakpointUpdatedEventArgs e)
scriptPath = mappedPath;
}

// TODO: It is very strange that we use the path as the key, which it could also be
// a script block.
Validate.IsNotNullOrEmptyString(nameof(scriptPath), scriptPath);

// Normalize the script filename for proper indexing
string normalizedScriptName = scriptPath.ToLower();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ internal static BreakpointDetails Create(
string hitCondition = null,
string logMessage = null)
{
Validate.IsNotNull("source", source);
Validate.IsNotNullOrEmptyString(nameof(source), source);

return new BreakpointDetails
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1040,26 +1040,28 @@ public async Task ExecuteScriptWithArgsAsync(string script, string arguments = n

if (arguments != null)
{
// Need to determine If the script string is a path to a script file.
string scriptAbsPath = string.Empty;
try
{
// Assume we can only debug scripts from the FileSystem provider
string workingDir = (await ExecuteCommandAsync<PathInfo>(
new PSCommand()
.AddCommand("Microsoft.PowerShell.Management\\Get-Location")
.AddParameter("PSProvider", "FileSystem"),
sendOutputToHost: false,
sendErrorToHost: false).ConfigureAwait(false))
.FirstOrDefault()
.ProviderPath;

workingDir = workingDir.TrimEnd(Path.DirectorySeparatorChar);
scriptAbsPath = workingDir + Path.DirectorySeparatorChar + script;
}
catch (System.Management.Automation.DriveNotFoundException e)
// Add CWD from PowerShell if not an absolute path
if (!Path.IsPathRooted(script))
{
this.logger.LogHandledException("Could not determine current filesystem location", e);
try
{
// Assume we can only debug scripts from the FileSystem provider
string workingDir = (await ExecuteCommandAsync<PathInfo>(
new PSCommand()
.AddCommand("Microsoft.PowerShell.Management\\Get-Location")
.AddParameter("PSProvider", "FileSystem"),
sendOutputToHost: false,
sendErrorToHost: false).ConfigureAwait(false))
.FirstOrDefault()
.ProviderPath;

this.logger.LogTrace($"Prepending working directory {workingDir} to script path {script}");
script = Path.Combine(workingDir, script);
}
catch (System.Management.Automation.DriveNotFoundException e)
{
this.logger.LogHandledException("Could not determine current filesystem location", e);
}
}

var strBld = new StringBuilder();
Expand All @@ -1071,7 +1073,7 @@ public async Task ExecuteScriptWithArgsAsync(string script, string arguments = n
// If the provided path is already quoted, then File.Exists will not find it.
// This keeps us from quoting an already quoted path.
// Related to issue #123.
if (File.Exists(script) || File.Exists(scriptAbsPath))
if (File.Exists(script))
{
// Dot-source the launched script path and single quote the path in case it includes
strBld.Append(". ").Append(QuoteEscapeString(script));
Expand Down Expand Up @@ -1246,7 +1248,7 @@ public void AbortExecution(bool shouldAbortDebugSession)
this.versionSpecificOperations.StopCommandInDebugger(this);
if (shouldAbortDebugSession)
{
this.ResumeDebugger(DebuggerResumeAction.Stop);
this.ResumeDebugger(DebuggerResumeAction.Stop, shouldWaitForExit: false);
}
}
else
Expand Down Expand Up @@ -1938,7 +1940,7 @@ private void PowerShellContext_ExecutionStatusChangedAsync(object sender, Execut

private IEnumerable<TResult> ExecuteCommandInDebugger<TResult>(PSCommand psCommand, bool sendOutputToHost)
{
this.logger.LogTrace($"Attempting to execute command(s)a in the debugger: {GetStringForPSCommand(psCommand)}");
this.logger.LogTrace($"Attempting to execute command(s) in the debugger: {GetStringForPSCommand(psCommand)}");

IEnumerable<TResult> output =
this.versionSpecificOperations.ExecuteCommandInDebugger<TResult>(
Expand Down Expand Up @@ -2414,7 +2416,7 @@ private void OnDebuggerStop(object sender, DebuggerStopEventArgs e)

if (!IsDebugServerActive)
{
_languageServer.SendNotification("powerShell/startDebugger");
_languageServer?.SendNotification("powerShell/startDebugger");
}

// We've hit a breakpoint so go to a new line so that the prompt can be rendered.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ function Test-Variables {
$nullString = [NullString]::Value
$psObjVar = New-Object -TypeName PSObject -Property @{Name = 'John'; Age = 75}
$psCustomObjVar = [PSCustomObject] @{Name = 'Paul'; Age = 73}
$procVar = Get-Process system
$procVar = Get-Process -PID $PID
Write-Output "Done"
}

Test-Variables
# NOTE: If a line is added to the function above, the line numbers in the
# associated unit tests MUST be adjusted accordingly.
Loading