Improve debugger's variable population mechanism

src/PowerShellEditorServices/Services/DebugAdapter/DebugService.cs

@@ -17,6 +17,7 @@
 using Microsoft.PowerShell.EditorServices.Services.PowerShell.Execution;
 using Microsoft.PowerShell.EditorServices.Services.PowerShell.Host;
 using Microsoft.PowerShell.EditorServices.Services.PowerShell.Debugging;
+using System.Collections;
 
 namespace Microsoft.PowerShell.EditorServices.Services
 {
@@ -45,6 +46,7 @@ internal class DebugService
         private List<VariableDetailsBase> variables;
         private VariableContainerDetails globalScopeVariables;
         private VariableContainerDetails scriptScopeVariables;
+        private VariableContainerDetails localScopeVariables;
         private StackFrameDetails[] stackFrameDetails;
         private readonly PropertyInfo invocationTypeScriptPositionProperty;
 
@@ -445,11 +447,6 @@ public async Task<string> SetVariableAsync(int variableContainerReferenceId, str
                 for (int i = 0; i < stackFrames.Length; i++)
                 {
                     var stackFrame = stackFrames[i];
-                    if (stackFrame.LocalVariables.ContainsVariable(variable.Id))
-                    {
-                        scope = i.ToString();
-                        break;
-                    }
                 }
             }
 
@@ -626,13 +623,12 @@ internal async Task<StackFrameDetails[]> GetStackFramesAsync(CancellationToken c
         public VariableScope[] GetVariableScopes(int stackFrameId)
         {
             var stackFrames = this.GetStackFrames();
-            int localStackFrameVariableId = stackFrames[stackFrameId].LocalVariables.Id;
             int autoVariablesId = stackFrames[stackFrameId].AutoVariables.Id;
 
             return new VariableScope[]
             {
                 new VariableScope(autoVariablesId, VariableContainerDetails.AutoVariablesName),
-                new VariableScope(localStackFrameVariableId, VariableContainerDetails.LocalScopeName),
+                new VariableScope(this.localScopeVariables.Id, VariableContainerDetails.LocalScopeName),
                 new VariableScope(this.scriptScopeVariables.Id, VariableContainerDetails.ScriptScopeName),
                 new VariableScope(this.globalScopeVariables.Id, VariableContainerDetails.GlobalScopeName),
             };
@@ -655,30 +651,27 @@ private async Task FetchStackFramesAndVariablesAsync(string scriptNameOverride)
                     new VariableDetails("Dummy", null)
                 };
 
-                // Must retrieve global/script variales before stack frame variables
-                // as we check stack frame variables against globals.
-                await FetchGlobalAndScriptVariablesAsync().ConfigureAwait(false);
+
+                // Must retrieve in order of broadest to narrowest scope for efficient deduplication: global, script, local
+                this.globalScopeVariables =
+                    await FetchVariableContainerAsync(VariableContainerDetails.GlobalScopeName).ConfigureAwait(false);
+
+                this.scriptScopeVariables =
+                    await FetchVariableContainerAsync(VariableContainerDetails.ScriptScopeName).ConfigureAwait(false);
+
+                this.localScopeVariables =
+                    await FetchVariableContainerAsync(VariableContainerDetails.LocalScopeName).ConfigureAwait(false);
+
                 await FetchStackFramesAsync(scriptNameOverride).ConfigureAwait(false);
+
             }
             finally
             {
                 this.debugInfoHandle.Release();
             }
         }
 
-        private async Task FetchGlobalAndScriptVariablesAsync()
-        {
-            // Retrieve globals first as script variable retrieval needs to search globals.
-            this.globalScopeVariables =
-                await FetchVariableContainerAsync(VariableContainerDetails.GlobalScopeName, null).ConfigureAwait(false);
-
-            this.scriptScopeVariables =
-                await FetchVariableContainerAsync(VariableContainerDetails.ScriptScopeName, null).ConfigureAwait(false);
-        }
-
-        private async Task<VariableContainerDetails> FetchVariableContainerAsync(
-            string scope,
-            VariableContainerDetails autoVariables)
+        private async Task<VariableContainerDetails> FetchVariableContainerAsync(string scope)
         {
             PSCommand psCommand = new PSCommand()
                 .AddCommand("Get-Variable")
@@ -687,8 +680,20 @@ private async Task<VariableContainerDetails> FetchVariableContainerAsync(
             var scopeVariableContainer = new VariableContainerDetails(this.nextVariableId++, "Scope: " + scope);
             this.variables.Add(scopeVariableContainer);
 
-            IReadOnlyList<PSObject> results = await _executionService.ExecutePSCommandAsync<PSObject>(psCommand, CancellationToken.None)
-                .ConfigureAwait(false);
+            IReadOnlyList<PSObject> results;
+            try
+            {
+                results = await _executionService.ExecutePSCommandAsync<PSObject>(psCommand, CancellationToken.None)
+                    .ConfigureAwait(false);
+            }
+            catch (CmdletInvocationException ex)
+            {
+                if (!ex.ErrorRecord.CategoryInfo.Reason.Equals("PSArgumentOutOfRangeException"))
+                {
+                    throw;
+                }
+                results = null;
+            }
 
             if (results != null)
             {
@@ -704,11 +709,6 @@ private async Task<VariableContainerDetails> FetchVariableContainerAsync(
                     var variableDetails = new VariableDetails(psVariableObject) { Id = this.nextVariableId++ };
                     this.variables.Add(variableDetails);
                     scopeVariableContainer.Children.Add(variableDetails.Name, variableDetails);
-
-                    if ((autoVariables != null) && AddToAutoVariables(psVariableObject, scope))
-                    {
-                        autoVariables.Children.Add(variableDetails.Name, variableDetails);
-                    }
                 }
             }
 
@@ -756,7 +756,7 @@ private bool AddToAutoVariables(PSObject psvariable, string scope)
             // Some local variables, if they exist, should be displayed by default
             if (psvariable.TypeNames[0].EndsWith("LocalVariable"))
             {
-                if (variableName.Equals("_"))
+                if (variableName.Equals("PSItem") || variableName.Equals("_"))
                 {
                     return true;
                 }
@@ -792,55 +792,90 @@ private bool AddToAutoVariables(PSObject psvariable, string scope)
         private async Task FetchStackFramesAsync(string scriptNameOverride)
         {
             PSCommand psCommand = new PSCommand();
+            // The serialization depth to retrieve variables from remote runspaces.
+            const int serializationDepth = 3;
 
             // This glorious hack ensures that Get-PSCallStack returns a list of CallStackFrame
             // objects (or "deserialized" CallStackFrames) when attached to a runspace in another
             // process.  Without the intermediate variable Get-PSCallStack inexplicably returns
             // an array of strings containing the formatted output of the CallStackFrame list.
-            var callStackVarName = $"$global:{PsesGlobalVariableNamePrefix}CallStack";
-            psCommand.AddScript($"{callStackVarName} = Get-PSCallStack; {callStackVarName}");
+            string callStackVarName = $"$global:{PsesGlobalVariableNamePrefix}CallStack";
+
+            string getPSCallStack = $"Get-PSCallStack | ForEach-Object {{ [void]{callStackVarName}.add(@($PSItem,$PSItem.GetFrameVariables())) }}";
+
+            // If we're attached to a remote runspace, we need to serialize the callstack prior to transport
+            // because the default depth is too shallow
+            bool isOnRemoteMachine = _psesHost.CurrentRunspace.IsOnRemoteMachine;
+            string returnSerializedIfOnRemoteMachine = isOnRemoteMachine
+                ? $"[Management.Automation.PSSerializer]::Serialize({callStackVarName}, {serializationDepth})"
+                : callStackVarName;
+
+            // We have to deal with a shallow serialization depth with ExecutePSCommandAsync as well, hence the serializer to get full var information
+            psCommand.AddScript($"[Collections.ArrayList]{callStackVarName} = @(); {getPSCallStack}; {returnSerializedIfOnRemoteMachine}");
 
-            var results = await _executionService.ExecutePSCommandAsync<PSObject>(psCommand, CancellationToken.None).ConfigureAwait(false);
 
-            var callStackFrames = results.ToArray();
+            // PSObject is used here instead of the specific type because we get deserialized objects from remote sessions and want a common interface
+            IReadOnlyList<PSObject> results = await _executionService.ExecutePSCommandAsync<PSObject>(psCommand, CancellationToken.None).ConfigureAwait(false);
 
-            this.stackFrameDetails = new StackFrameDetails[callStackFrames.Length];
+            IEnumerable callStack = isOnRemoteMachine
+                ? (PSSerializer.Deserialize(results[0].BaseObject as string) as PSObject).BaseObject as IList
+                : results;
 
-            for (int i = 0; i < callStackFrames.Length; i++)
+            List<StackFrameDetails> stackFrameDetailList = new List<StackFrameDetails>();
+            foreach (var callStackFrameItem in callStack)
             {
-                VariableContainerDetails autoVariables =
-                    new VariableContainerDetails(
-                        this.nextVariableId++,
-                        VariableContainerDetails.AutoVariablesName);
+                var callStackFrameComponents = (callStackFrameItem as PSObject).BaseObject as IList;
+                var callStackFrame = callStackFrameComponents[0] as PSObject;
+                IDictionary callStackVariables = isOnRemoteMachine
+                    ? (callStackFrameComponents[1] as PSObject).BaseObject as IDictionary
+                    : callStackFrameComponents[1] as IDictionary;
 
-                this.variables.Add(autoVariables);
+                var autoVariables = new VariableContainerDetails(
+                    nextVariableId++,
+                    VariableContainerDetails.AutoVariablesName);
 
-                VariableContainerDetails localVariables =
-                    await FetchVariableContainerAsync(i.ToString(), autoVariables).ConfigureAwait(false);
+                variables.Add(autoVariables);
 
-                // When debugging, this is the best way I can find to get what is likely the workspace root.
-                // This is controlled by the "cwd:" setting in the launch config.
-                string workspaceRootPath = _psesHost.InitialWorkingDirectory;
+                foreach (DictionaryEntry entry in callStackVariables)
+                {
+                    // TODO: This should be deduplicated into a new function for the other variable handling as well
+                    object psVarValue = isOnRemoteMachine
+                        ? (entry.Value as PSObject).Properties["Value"].Value
+                        : (entry.Value as PSVariable).Value;
+                    // The constructor we are using here does not automatically add the dollar prefix
+                    string psVarName = VariableDetails.DollarPrefix + entry.Key.ToString();
+                    var variableDetails = new VariableDetails(psVarName, psVarValue) { Id = nextVariableId++ };
+                    variables.Add(variableDetails);
+
+                    if (AddToAutoVariables(new PSObject(entry.Value), scope: null))
+                    {
+                        autoVariables.Children.Add(variableDetails.Name, variableDetails);
+                    }
+                }
 
-                this.stackFrameDetails[i] =
-                    StackFrameDetails.Create(callStackFrames[i], autoVariables, localVariables, workspaceRootPath);
+                var stackFrameDetailsEntry = StackFrameDetails.Create(callStackFrame, autoVariables);
 
-                string stackFrameScriptPath = this.stackFrameDetails[i].ScriptPath;
-                if (scriptNameOverride != null &&
+                string stackFrameScriptPath = stackFrameDetailsEntry.ScriptPath;
+                if (scriptNameOverride is not null &&
                     string.Equals(stackFrameScriptPath, StackFrameDetails.NoFileScriptPath))
                 {
-                    this.stackFrameDetails[i].ScriptPath = scriptNameOverride;
+                    stackFrameDetailsEntry.ScriptPath = scriptNameOverride;
                 }
-                else if (_psesHost.CurrentRunspace.IsOnRemoteMachine
-                    && this.remoteFileManager != null
+                else if (isOnRemoteMachine
+                    && remoteFileManager is not null
                     && !string.Equals(stackFrameScriptPath, StackFrameDetails.NoFileScriptPath))
                 {
-                    this.stackFrameDetails[i].ScriptPath =
-                        this.remoteFileManager.GetMappedPath(
+                    stackFrameDetailsEntry.ScriptPath =
+                        remoteFileManager.GetMappedPath(
                             stackFrameScriptPath,
                             _psesHost.CurrentRunspace);
                 }
+
+                stackFrameDetailList.Add(
+                    stackFrameDetailsEntry);
             }
+
+            stackFrameDetails = stackFrameDetailList.ToArray();
         }
 
         private static string TrimScriptListingLine(PSObject scriptLineObj, ref int prefixLength)

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

@@ -64,11 +64,6 @@ internal class StackFrameDetails
         /// </summary>
         public VariableContainerDetails AutoVariables { get; private set; }
 
-        /// <summary>
-        /// Gets or sets the VariableContainerDetails that contains the local variables.
-        /// </summary>
-        public VariableContainerDetails LocalVariables { get; private set; }
-
         #endregion
 
         #region Constructors
@@ -93,14 +88,9 @@ internal class StackFrameDetails
         /// <returns>A new instance of the StackFrameDetails class.</returns>
         static internal StackFrameDetails Create(
             PSObject callStackFrameObject,
-            VariableContainerDetails autoVariables,
-            VariableContainerDetails localVariables,
-            string workspaceRootPath = null)
+            VariableContainerDetails autoVariables)
         {
-            string moduleId = string.Empty;
             var isExternal = false;
-
-            var invocationInfo = callStackFrameObject.Properties["InvocationInfo"]?.Value as InvocationInfo;
             string scriptPath = (callStackFrameObject.Properties["ScriptName"].Value as string) ?? NoFileScriptPath;
             int startLineNumber = (int)(callStackFrameObject.Properties["ScriptLineNumber"].Value ?? 0);
 
@@ -122,7 +112,6 @@ static internal StackFrameDetails Create(
                 StartColumnNumber = 0,   // Column number isn't given in PowerShell stack frames
                 EndColumnNumber = 0,
                 AutoVariables = autoVariables,
-                LocalVariables = localVariables,
                 IsExternalCode = isExternal
             };
         }

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

@@ -290,7 +290,7 @@ private VariableDetails[] GetChildren(object obj, ILogger logger)
                         childVariables.AddRange(
                             psObject
                                 .Properties
-                                .Where(p => p.MemberType == PSMemberTypes.NoteProperty)
+                                .Where(p => (PSMemberTypes.Properties & p.MemberType) is not 0) // Filter out non-properties
                                 .Select(p => new VariableDetails(p)));
 
                         obj = psObject.BaseObject;