Merge pull request #1540 from PowerShell/andschwa/debugger-tests

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

Author: andyleejordan

.editorconfig

@@ -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

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

@@ -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
 {
@@ -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;
@@ -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 =
@@ -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
@@ -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>
@@ -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)
@@ -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();
 

src/PowerShellEditorServices/Services/DebugAdapter/Debugging/BreakpointDetails.cs

@@ -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
             {

src/PowerShellEditorServices/Services/PowerShellContext/PowerShellContextService.cs

@@ -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();
@@ -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));
@@ -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
@@ -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>(
@@ -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.

test/PowerShellEditorServices.Test.Shared/Debugging/VariableTest.ps1

@@ -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.