[dds/dap] Prevent duplicate values showing in variables views when a class has both a field and a getter with the same name

When a class contains both a field and getter with the same name, we'd include both in the `variables` response. In most cases these are the same value (although it's not guaranteed).

I've chosen to just hide the field in this case and always show the result from evaluating the getter (since I think that's what the user would expect, even in the case where they happen to have different values). Another option could be to show both (but change the name so that fields/getters are shown differently), however in that would change the display (for example adding `get ` in front of all getters) we should probably only do that if it's clear there is demand for it.

Fixes https://github.com/Dart-Code/Dart-Code/issues/5128

Change-Id: I9e23d22a844ee22c38988456b1f275422c5c9e04
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/370640
Reviewed-by: Ben Konyi <bkonyi@google.com>
Reviewed-by: Helin Shiah <helinx@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/dds/CHANGELOG.md

@@ -1,5 +1,6 @@
 # 4.2.4
 - Added missing type to `Event` in `postEvent`.
+- [DAP] Instaces with both fields and getters of the same name will no longer show duplicates in `variables` responses.
 
 # 4.2.3
 - Added missing await of `WebSocketChannel.ready` in `startDartDevelopmentService`.

pkg/dds/lib/src/dap/protocol_converter.dart

@@ -296,10 +296,18 @@ class ProtocolConverter {
           }
         }
 
-        // Collect getter names for this instances class and its supers.
+        // Collect getter names for this instances class and its supers, but
+        // remove any names that have already showed up as fields because
+        // otherwise they will show up duplicated.
         final getterNames =
             await _getterNamesForClassHierarchy(thread, instance.classRef);
 
+        // Remove any existing field variables where there are getters, because
+        // otherwise we'll have dupes, and the user will generally expected to
+        // see the getters value (if not the same).
+        variables
+            .removeWhere((variable) => getterNames.contains(variable.name));
+
         final getterVariables = getterNames.mapIndexed(createVariable);
         variables.addAll(await Future.wait(getterVariables));
       }

pkg/dds/test/dap/integration/debug_variables_test.dart

@@ -238,6 +238,82 @@ class A {
       );
     });
 
+    test('does not duplicate getters overriding fields', () async {
+      final client = dap.client;
+      final testFile = dap.createTestFile('''
+void main(List<String> args) {
+  final myVariable = A();
+  print('Hello!'); $breakpointMarker
+}
+
+class Base {
+  String aaa = 'nnn';
+}
+
+class A extends Base {
+  @override
+  String get aaa => 'yyy';
+}
+    ''');
+      final breakpointLine = lineWith(testFile, breakpointMarker);
+
+      final stop = await client.hitBreakpoint(
+        testFile,
+        breakpointLine,
+        launch: () => client.launch(
+          testFile.path,
+          evaluateGettersInDebugViews: true,
+        ),
+      );
+      await client.expectLocalVariable(
+        stop.threadId!,
+        expectedName: 'myVariable',
+        expectedDisplayString: 'A',
+        expectedVariables: '''
+            aaa: "yyy", eval: myVariable.aaa
+        ''',
+        ignore: {'runtimeType'},
+      );
+    });
+
+    test('does not duplicate fields overriding getters', () async {
+      final client = dap.client;
+      final testFile = dap.createTestFile('''
+void main(List<String> args) {
+  final myVariable = A();
+  print('Hello!'); $breakpointMarker
+}
+
+abstract class Base {
+  String get aaa => 'nnn';
+}
+
+class A extends Base {
+  @override
+  final String aaa = 'yyy';
+}
+    ''');
+      final breakpointLine = lineWith(testFile, breakpointMarker);
+
+      final stop = await client.hitBreakpoint(
+        testFile,
+        breakpointLine,
+        launch: () => client.launch(
+          testFile.path,
+          evaluateGettersInDebugViews: true,
+        ),
+      );
+      await client.expectLocalVariable(
+        stop.threadId!,
+        expectedName: 'myVariable',
+        expectedDisplayString: 'A',
+        expectedVariables: '''
+            aaa: "yyy", eval: myVariable.aaa
+        ''',
+        ignore: {'runtimeType'},
+      );
+    });
+
     test('includes record fields', () async {
       final client = dap.client;
       final testFile = dap.createTestFile('''