Make debugger skip same locations in dart during stepping. (#2043)

Anna Gringauze · web-flow · commit bb22b3a053d2 · 2023-03-22T08:56:39.000-07:00
* Temp

* Add tests and cleanup

* Update changelog and disable new tests for all versions before current main

* Update changelog with pull request reference

* Fix test failures

* Update patterns tests to skip correct sdk versions
diff --git a/dwds/CHANGELOG.md b/dwds/CHANGELOG.md
@@ -1,6 +1,7 @@
 ## 18.0.2-dev
 
 - Support new DDC temp names for patterns. - [#2042](https://github.com/dart-lang/webdev/pull/2042)
+- Make debugger find next dart location when stepping. -[#2043](https://github.com/dart-lang/webdev/pull/2043)
 
 ## 18.0.1
 
diff --git a/dwds/lib/src/debugging/debugger.dart b/dwds/lib/src/debugging/debugger.dart
@@ -83,6 +83,7 @@ class Debugger extends Domain {
   FrameComputer? stackComputer;
 
   bool _isStepping = false;
+  DartLocation? _previousSteppingLocation;
 
   void updateInspector(AppInspectorInterface appInspector) {
     inspector = appInspector;
@@ -143,6 +144,7 @@ class Debugger extends Domain {
       }
     } else {
       _isStepping = false;
+      _previousSteppingLocation = null;
       result = await _remoteDebugger.resume();
     }
     handleErrorIfPresent(result);
@@ -354,7 +356,12 @@ class Debugger extends Domain {
     final url = urlForScriptId(scriptId);
     if (url == null) return null;
 
-    return _locations.locationForJs(url, line, column);
+    final loc = await _locations.locationForJs(url, line, column);
+    if (loc == null || loc.dartLocation == _previousSteppingLocation) {
+      return null;
+    }
+    _previousSteppingLocation = loc.dartLocation;
+    return loc;
   }
 
   /// Returns script ID for the paused event.
diff --git a/dwds/lib/src/debugging/location.dart b/dwds/lib/src/debugging/location.dart
@@ -74,6 +74,19 @@ class DartLocation {
     return result == 0 ? column.compareTo(otherColumn) : result;
   }
 
+  @override
+  int get hashCode => Object.hashAll([uri, line, column]);
+
+  @override
+  bool operator ==(Object? other) {
+    if (other is! DartLocation) {
+      return false;
+    }
+    return uri.serverPath == other.uri.serverPath &&
+        line == other.line &&
+        column == other.column;
+  }
+
   @override
   String toString() => '[${uri.serverPath}:$line:$column]';
 
diff --git a/dwds/test/instances/instance_inspection_common.dart b/dwds/test/instances/instance_inspection_common.dart
@@ -119,6 +119,20 @@ class TestInspector {
     expect(result, isA<Instance>());
     return result as Instance;
   }
+
+  Future<Map<String?, Instance?>> getFrameVariables(
+      String isolateId, Frame frame) async {
+    final refs = <String, InstanceRef>{
+      for (var variable in frame.vars!)
+        variable.name!: variable.value as InstanceRef
+    };
+    final instances = <String, Instance>{};
+    for (final p in refs.entries) {
+      instances[p.key] =
+          await service.getObject(isolateId, p.value.id!) as Instance;
+    }
+    return instances;
+  }
 }
 
 Map<String, InstanceRef> _associationsToMap(
@@ -179,7 +193,7 @@ Object? _getValue(InstanceRef instanceRef) {
       return instanceRef.valueAsString == 'true';
     case InstanceKind.kDouble:
     case InstanceKind.kInt:
-      return int.parse(instanceRef.valueAsString!);
+      return double.parse(instanceRef.valueAsString!);
     case InstanceKind.kString:
       return instanceRef.valueAsString;
     default:
diff --git a/dwds/test/instances/patterns_inspection_test.dart b/dwds/test/instances/patterns_inspection_test.dart
@@ -0,0 +1,155 @@
+// Copyright (c) 2023, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+@TestOn('vm')
+@Timeout(Duration(minutes: 2))
+
+import 'dart:io';
+
+import 'package:pub_semver/pub_semver.dart' as semver;
+import 'package:test/test.dart';
+import 'package:test_common/logging.dart';
+import 'package:test_common/test_sdk_configuration.dart';
+import 'package:vm_service/vm_service.dart';
+
+import '../fixtures/context.dart';
+import '../fixtures/project.dart';
+import 'instance_inspection_common.dart';
+
+void main() async {
+  // Enable verbose logging for debugging.
+  final debug = false;
+
+  final provider = TestSdkConfigurationProvider(verbose: debug);
+  tearDownAll(provider.dispose);
+
+  for (var compilationMode in CompilationMode.values) {
+    await _runTests(
+      provider: provider,
+      compilationMode: compilationMode,
+      debug: debug,
+    );
+  }
+}
+
+Future<void> _runTests({
+  required TestSdkConfigurationProvider provider,
+  required CompilationMode compilationMode,
+  required bool debug,
+}) async {
+  final context =
+      TestContext(TestProject.testExperimentWithSoundNullSafety, provider);
+  final testInspector = TestInspector(context);
+
+  late VmServiceInterface service;
+  late Stream<Event> stream;
+  late String isolateId;
+  late ScriptRef mainScript;
+
+  onBreakPoint(breakPointId, body) => testInspector.onBreakPoint(
+      stream, isolateId, mainScript, breakPointId, body);
+
+  getInstanceRef(frame, expression) =>
+      testInspector.getInstanceRef(isolateId, frame, expression);
+
+  getFields(instanceRef, {offset, count}) => testInspector
+      .getFields(isolateId, instanceRef, offset: offset, count: count);
+
+  getFrameVariables(Frame frame) =>
+      testInspector.getFrameVariables(isolateId, frame);
+
+  group('$compilationMode |', () {
+    setUpAll(() async {
+      setCurrentLogWriter(debug: debug);
+      await context.setUp(
+        compilationMode: compilationMode,
+        enableExpressionEvaluation: true,
+        verboseCompiler: debug,
+        experiments: ['records', 'patterns'],
+      );
+      service = context.debugConnection.vmService;
+
+      final vm = await service.getVM();
+      isolateId = vm.isolates!.first.id!;
+      final scripts = await service.getScripts(isolateId);
+
+      await service.streamListen('Debug');
+      stream = service.onEvent('Debug');
+
+      mainScript = scripts.scripts!
+          .firstWhere((each) => each.uri!.contains('main.dart'));
+    });
+
+    tearDownAll(() async {
+      await context.tearDown();
+    });
+
+    setUp(() => setCurrentLogWriter(debug: debug));
+    tearDown(() => service.resume(isolateId));
+
+    test('pattern match case 1', () async {
+      await onBreakPoint('testPatternCase1', (event) async {
+        final frame = event.topFrame!;
+
+        expect(await getFrameVariables(frame), {
+          'obj': matchListInstance(type: 'List<Object>'),
+          'a': matchPrimitiveInstance(kind: InstanceKind.kString, value: 'a'),
+          'n': matchPrimitiveInstance(kind: InstanceKind.kDouble, value: 1),
+        });
+      });
+    });
+
+    test('pattern match case 2', () async {
+      await onBreakPoint('testPatternCase2', (event) async {
+        final frame = event.topFrame!;
+
+        expect(await getFrameVariables(frame), {
+          'obj': matchListInstance(type: 'List<Object>'),
+          'a': matchPrimitiveInstance(kind: InstanceKind.kString, value: 'b'),
+          'n': matchPrimitiveInstance(kind: InstanceKind.kDouble, value: 3.14),
+        });
+      });
+    });
+
+    test('pattern match default case', () async {
+      await onBreakPoint('testPatternDefault', (event) async {
+        final frame = event.topFrame!;
+        final frameIndex = frame.index!;
+        final instanceRef = await getInstanceRef(frameIndex, 'obj');
+        expect(await getFields(instanceRef), [0, 1]);
+
+        expect(await getFrameVariables(frame), {
+          'obj': matchListInstance(type: 'List<int>'),
+        });
+      });
+    });
+
+    test('stepping through pattern match', () async {
+      await onBreakPoint('callTestPattern1', (Event event) async {
+        var previousLocation = event.topFrame!.location;
+        for (var step in [
+          // Make sure we step into the callee.
+          for (var i = 0; i < 4; i++) 'Into',
+          // Make a few steps inside the callee.
+          for (var i = 0; i < 4; i++) 'Over',
+        ]) {
+          await service.resume(isolateId, step: step);
+
+          event = await stream
+              .firstWhere((e) => e.kind == EventKind.kPauseInterrupted);
+
+          if (step == 'Over') {
+            expect(event.topFrame!.code!.name, 'testPattern');
+          }
+
+          final location = event.topFrame!.location;
+          expect(location, isNot(equals(previousLocation)));
+          previousLocation = location;
+        }
+      });
+    });
+  }, // TODO(annagrin): Remove when dart 3.0 is stable.
+      skip: semver.Version.parse(Platform.version.split(' ')[0]) <
+          semver.Version.parse('3.0.0-351.0.dev'));
+}
diff --git a/dwds/test/instances/record_inspection_test.dart b/dwds/test/instances/record_inspection_test.dart
@@ -65,7 +65,7 @@ Future<void> _runTests({
         compilationMode: compilationMode,
         enableExpressionEvaluation: true,
         verboseCompiler: debug,
-        experiments: ['records'],
+        experiments: ['records', 'patterns'],
       );
       service = context.debugConnection.vmService;
 
@@ -88,7 +88,7 @@ Future<void> _runTests({
     tearDown(() => service.resume(isolateId));
 
     test('simple records', () async {
-      await onBreakPoint('printSimpleLocal', (event) async {
+      await onBreakPoint('printSimpleLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, 'record');
 
@@ -111,7 +111,7 @@ Future<void> _runTests({
     });
 
     test('simple records, field access', () async {
-      await onBreakPoint('printSimpleLocal', (event) async {
+      await onBreakPoint('printSimpleLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         expect(await getInstance(frame, r'record.$1'),
             matchPrimitiveInstance(kind: InstanceKind.kBool, value: true));
@@ -122,7 +122,7 @@ Future<void> _runTests({
     });
 
     test('simple records with named fields', () async {
-      await onBreakPoint('printSimpleNamedLocal', (event) async {
+      await onBreakPoint('printSimpleNamedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, 'record');
 
@@ -149,7 +149,7 @@ Future<void> _runTests({
     });
 
     test('simple records with named fields, field access', () async {
-      await onBreakPoint('printSimpleNamedLocal', (event) async {
+      await onBreakPoint('printSimpleNamedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         expect(await getInstance(frame, r'record.$1'),
             matchPrimitiveInstance(kind: InstanceKind.kBool, value: true));
@@ -160,7 +160,7 @@ Future<void> _runTests({
     });
 
     test('complex records fields', () async {
-      await onBreakPoint('printComplexLocal', (event) async {
+      await onBreakPoint('printComplexLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, 'record');
 
@@ -208,7 +208,7 @@ Future<void> _runTests({
     });
 
     test('complex records, field access', () async {
-      await onBreakPoint('printComplexLocal', (event) async {
+      await onBreakPoint('printComplexLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         expect(await getInstance(frame, r'record.$1'),
             matchPrimitiveInstance(kind: InstanceKind.kBool, value: true));
@@ -223,7 +223,7 @@ Future<void> _runTests({
     });
 
     test('complex records with named fields', () async {
-      await onBreakPoint('printComplexNamedLocal', (event) async {
+      await onBreakPoint('printComplexNamedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, 'record');
 
@@ -272,7 +272,7 @@ Future<void> _runTests({
     });
 
     test('complex records with named fields, field access', () async {
-      await onBreakPoint('printComplexNamedLocal', (event) async {
+      await onBreakPoint('printComplexNamedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         expect(await getInstance(frame, r'record.$1'),
             matchPrimitiveInstance(kind: InstanceKind.kBool, value: true));
@@ -287,7 +287,7 @@ Future<void> _runTests({
     });
 
     test('nested records', () async {
-      await onBreakPoint('printNestedLocal', (event) async {
+      await onBreakPoint('printNestedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, 'record');
 
@@ -324,7 +324,7 @@ Future<void> _runTests({
     });
 
     test('nested records, field access', () async {
-      await onBreakPoint('printNestedLocal', (event) async {
+      await onBreakPoint('printNestedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, r'record.$2');
 
@@ -338,7 +338,7 @@ Future<void> _runTests({
     });
 
     test('nested records with named fields,', () async {
-      await onBreakPoint('printNestedNamedLocal', (event) async {
+      await onBreakPoint('printNestedNamedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, 'record');
 
@@ -382,7 +382,7 @@ Future<void> _runTests({
     });
 
     test('nested records with named fields, field access', () async {
-      await onBreakPoint('printNestedNamedLocal', (event) async {
+      await onBreakPoint('printNestedNamedLocalRecord', (event) async {
         final frame = event.topFrame!.index!;
         final instanceRef = await getInstanceRef(frame, r'record.inner');
 
diff --git a/fixtures/_experimentSound/web/main.dart b/fixtures/_experimentSound/web/main.dart
