Revert to expanding public instance and static properties

After fixing the bug where we'd stop retrieving values when one value
failed, that seemed to fix the overall bug where lots of expected
properties failed to show up. While investigating, I also found many
properties duplicated, seemingly due to being retrieved both off the
PSObject and off the .NET object, so now when we do the latter we check
(by name) that it hasn't already been added.

Author: andyleejordan

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

@@ -123,7 +123,7 @@ private static object SafeGetValue(PSPropertyInfo psProperty)
             {
                 // Sometimes we can't get the value, like ExitCode, for reasons beyond our control,
                 // so just return the message from the exception that arises.
-                return ex.Message;
+                return new UnableToRetrievePropertyMessage { Name = psProperty.Name, Message = ex.Message };
             }
         }
 
@@ -345,11 +345,8 @@ protected static void AddDotNetProperties(object obj, List<VariableDetails> chil
                 return;
             }
 
-            // We include NonPublic properties because the C# extension seems to as well, and this
-            // isn't an inline local because we're often debugging it.
-            PropertyInfo[] properties = objectType.GetProperties(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance);
-
-            foreach (PropertyInfo property in properties)
+            // Search all the public instance or static properties and add those missing.
+            foreach (PropertyInfo property in objectType.GetProperties())
             {
                 // Don't display indexer properties, it causes an exception anyway.
                 if (property.GetIndexParameters().Length > 0)
@@ -359,10 +356,11 @@ protected static void AddDotNetProperties(object obj, List<VariableDetails> chil
 
                 try
                 {
-                    childVariables.Add(
-                        new VariableDetails(
-                            property.Name,
-                            property.GetValue(obj)));
+                    // Only add unique properties because we may have already added some.
+                    if (!childVariables.Exists(p => p.Name == property.Name))
+                    {
+                        childVariables.Add(new VariableDetails(property.Name, property.GetValue(obj)));
+                    }
                 }
                 catch (Exception ex)
                 {
@@ -376,21 +374,19 @@ protected static void AddDotNetProperties(object obj, List<VariableDetails> chil
                     childVariables.Add(
                         new VariableDetails(
                             property.Name,
-                            new UnableToRetrievePropertyMessage(
-                                "Error retrieving property - " + ex.GetType().Name)));
+                            new UnableToRetrievePropertyMessage { Name = property.Name, Message = ex.Message }));
                 }
             }
         }
 
         #endregion
 
-        private readonly struct UnableToRetrievePropertyMessage
+        private record UnableToRetrievePropertyMessage
         {
-            public UnableToRetrievePropertyMessage(string message) => Message = message;
-
-            public string Message { get; }
+            public string Name { get; init; }
+            public string Message { get; init; }
 
-            public override string ToString() => "<" + Message + ">";
+            public override string ToString() => $"Error retrieving property '${Name}': ${Message}";
         }
     }
 

test/PowerShellEditorServices.Test/Debugging/DebugServiceTests.cs

@@ -919,18 +919,14 @@ await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true
             Assert.NotNull(rawDetailsView);
             Assert.Empty(rawDetailsView.Type);
             Assert.Empty(rawDetailsView.ValueString);
-            Assert.Collection(rawDetailsView.GetChildren(NullLogger.Instance),
+            VariableDetailsBase[] rawViewChildren = rawDetailsView.GetChildren(NullLogger.Instance);
+            Assert.Collection(rawViewChildren,
                 (i) =>
                 {
                     Assert.Equal("Length", i.Name);
                     Assert.Equal("4", i.ValueString);
                 },
                 (i) =>
-                {
-                    Assert.Equal("NativeLength", i.Name);
-                    Assert.Equal("4", i.ValueString);
-                },
-                (i) =>
                 {
                     Assert.Equal("LongLength", i.Name);
                     Assert.Equal("4", i.ValueString);
@@ -980,10 +976,8 @@ await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true
             Assert.NotNull(rawDetailsView);
             Assert.Empty(rawDetailsView.Type);
             Assert.Empty(rawDetailsView.ValueString);
-            Assert.Collection(rawDetailsView.GetChildren(NullLogger.Instance),
-                (i) => Assert.Equal("hcp", i.Name),
-                (i) => Assert.Equal("comparer", i.Name),
-                (i) => Assert.Equal("EqualityComparer", i.Name),
+            VariableDetailsBase[] rawDetailsChildren = rawDetailsView.GetChildren(NullLogger.Instance);
+            Assert.Collection(rawDetailsChildren,
                 (i) =>
                 {
                     Assert.Equal("IsReadOnly", i.Name);
@@ -1045,8 +1039,28 @@ await GetVariables(VariableContainerDetails.ScriptScopeName).ConfigureAwait(true
             Assert.Empty(rawDetailsView.Type);
             Assert.Empty(rawDetailsView.ValueString);
             VariableDetailsBase[] rawViewChildren = rawDetailsView.GetChildren(NullLogger.Instance);
-            Assert.Equal(15, rawViewChildren.Length);
-            Assert.NotNull(Array.Find(rawViewChildren, v => v.Name == "Comparer"));
+            Assert.Collection(rawViewChildren,
+                (i) =>
+                {
+                    Assert.Equal("Count", i.Name);
+                    Assert.Equal("4", i.ValueString);
+                },
+                (i) =>
+                {
+                    Assert.Equal("Comparer", i.Name);
+                    Assert.Equal("[GenericComparer`1]", i.ValueString);
+                },
+                (i) =>
+                {
+                    Assert.Equal("Keys", i.Name);
+                    Assert.Equal("[KeyCollection: 4]", i.ValueString);
+                },
+                (i) =>
+                {
+                    Assert.Equal("Values", i.Name);
+                    Assert.Equal("[ValueCollection: 4]", i.ValueString);
+                }
+            );
         }
 
         [Fact]
@@ -1098,7 +1112,7 @@ await debugService.SetLineBreakpointsAsync(
             Assert.True(var.IsExpandable);
 
             VariableDetailsBase[] childVars = await debugService.GetVariables(var.Id, CancellationToken.None).ConfigureAwait(true);
-            Assert.Contains(childVars, i => i.Name is "Name" && i.ValueString is "\"dotnet\"");
+            Assert.Contains(childVars, i => i.Name is "Name");
             Assert.Contains(childVars, i => i.Name is "Handles");
             Assert.Contains(childVars, i => i.Name is "CommandLine");
             Assert.Contains(childVars, i => i.Name is "ExitCode");