[ Service ] Add line/column information to SourceLocation objects

Removes the need for requesting a full Script object, which can be
extremely large when including source code. This change will have a
relatively small impact on response sizes.

Related issues: https://github.com/dart-lang/sdk/issues/47215, https://github.com/flutter/devtools/issues/3382

TEST=pkg/vm_service tests updated

Change-Id: I27999c4b1da65d4f0c643fa8db1a019c0fd1d689
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/227640
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Siva Annamalai <asiva@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>

pkg/vm_service/CHANGELOG.md

@@ -1,5 +1,9 @@
 # Changelog
 
+## 8.2.0-dev
+- Update to version `3.56` of the spec.
+- Added optional `line` and `column` properties to `SourceLocation`.
+
 ## 8.1.0
 - Update to version `3.55` of the spec.
 - Added `streamCpuSamplesWithUserTag` RPC.

pkg/vm_service/java/version.properties

@@ -1 +1 @@
-version=3.55
+version=3.56

pkg/vm_service/lib/src/vm_service.dart

@@ -26,7 +26,7 @@ export 'snapshot_graph.dart'
         HeapSnapshotObjectNoData,
         HeapSnapshotObjectNullData;
 
-const String vmServiceVersion = '3.55.0';
+const String vmServiceVersion = '3.56.0';
 
 /// @optional
 const String optional = 'optional';
@@ -7417,10 +7417,22 @@ class SourceLocation extends Response {
   @optional
   int? endTokenPos;
 
+  /// The line associated with this location. Only provided for non-synthetic
+  /// token positions.
+  @optional
+  int? line;
+
+  /// The column associated with this location. Only provided for non-synthetic
+  /// token positions.
+  @optional
+  int? column;
+
   SourceLocation({
     required this.script,
     required this.tokenPos,
     this.endTokenPos,
+    this.line,
+    this.column,
   });
 
   SourceLocation._fromJson(Map<String, dynamic> json) : super._fromJson(json) {
@@ -7428,6 +7440,8 @@ class SourceLocation extends Response {
         createServiceObject(json['script'], const ['ScriptRef']) as ScriptRef?;
     tokenPos = json['tokenPos'] ?? -1;
     endTokenPos = json['endTokenPos'];
+    line = json['line'];
+    column = json['column'];
   }
 
   @override
@@ -7442,6 +7456,8 @@ class SourceLocation extends Response {
       'tokenPos': tokenPos,
     });
     _setIfNotNull(json, 'endTokenPos', endTokenPos);
+    _setIfNotNull(json, 'line', line);
+    _setIfNotNull(json, 'column', column);
     return json;
   }
 

pkg/vm_service/pubspec.yaml

@@ -3,7 +3,7 @@ description: >-
   A library to communicate with a service implementing the Dart VM
   service protocol.
 
-version: 8.1.0
+version: 8.2.0-dev
 
 homepage: https://github.com/dart-lang/sdk/tree/master/pkg/vm_service
 

pkg/vm_service/test/add_breakpoint_rpc_kernel_test.dart

@@ -71,13 +71,17 @@ var tests = <IsolateTest>[
     expect(futureBpt1.resolved, isTrue);
     expect(script.getLineNumberFromTokenPos(futureBpt1.location!.tokenPos!),
         LINE_A);
+    expect(futureBpt1.location!.line, LINE_A);
     expect(
         script.getColumnNumberFromTokenPos(futureBpt1.location!.tokenPos!), 12);
+    expect(futureBpt1.location!.column, 12);
     expect(futureBpt2.resolved, isTrue);
     expect(script.getLineNumberFromTokenPos(futureBpt2.location!.tokenPos!),
         LINE_A);
+    expect(futureBpt2.location!.line, LINE_A);
     expect(
         script.getColumnNumberFromTokenPos(futureBpt2.location!.tokenPos!), 3);
+    expect(futureBpt2.location!.column, 3);
 
     // The first breakpoint hits before value is modified.
     InstanceRef result =
@@ -121,13 +125,19 @@ var tests = <IsolateTest>[
       print('$LINE_A:${col} -> ${resolvedLine}:${resolvedCol}');
       if (col <= 12) {
         expect(resolvedLine, LINE_A);
+        expect(bpt.location!.line, LINE_A);
         expect(resolvedCol, 3);
+        expect(bpt.location!.column, 3);
       } else if (col <= 36) {
         expect(resolvedLine, LINE_A);
+        expect(bpt.location!.line, LINE_A);
         expect(resolvedCol, 12);
+        expect(bpt.location!.column, 12);
       } else {
         expect(resolvedLine, LINE_B);
+        expect(bpt.location!.line, LINE_B);
         expect(resolvedCol, 12);
+        expect(bpt.location!.column, 12);
       }
       expect(
           (await service.removeBreakpoint(isolateId, bpt.id!)).type, 'Success');

pkg/vm_service/test/breakpoint_async_break_test.dart

@@ -9,7 +9,8 @@ import 'package:vm_service/vm_service.dart';
 import 'common/service_test_common.dart';
 import 'common/test_helper.dart';
 
-const int LINE = 18;
+const int LINE = 19;
+const int COL = 7;
 
 // Issue: https://github.com/dart-lang/sdk/issues/36622
 Future<void> testMain() async {
@@ -62,7 +63,10 @@ var tests = <IsolateTest>[
     expect(futureBpt.resolved, isTrue);
     expect(
         script.getLineNumberFromTokenPos(futureBpt.location!.tokenPos), LINE);
-    expect(script.getColumnNumberFromTokenPos(futureBpt.location!.tokenPos), 7);
+    expect(futureBpt.location!.line, LINE);
+    expect(
+        script.getColumnNumberFromTokenPos(futureBpt.location!.tokenPos), COL);
+    expect(futureBpt.location!.column, COL);
 
     // Remove the breakpoints.
     expect((await service.removeBreakpoint(isolateId, futureBpt.id!)).type,

pkg/vm_service/test/debugging_test.dart

@@ -91,6 +91,7 @@ var tests = <IsolateTest>[
     final SourceLocation location = bpt.location;
     expect(location.script!.id, script.id);
     expect(script.getLineNumberFromTokenPos(location.tokenPos!), 16);
+    expect(location.line, 16);
 
     isolate = await service.getIsolate(isolateId);
     expect(isolate.breakpoints!.length, 1);
@@ -103,13 +104,12 @@ var tests = <IsolateTest>[
     final isolateId = isolateRef.id!;
     final stack = await service.getStack(isolateId);
     expect(stack.frames!.length, greaterThanOrEqualTo(1));
-
-    Script script = (await service.getObject(
-        isolateId, stack.frames![0].location!.script!.id!)) as Script;
+    final location = stack.frames![0].location!;
+    Script script =
+        (await service.getObject(isolateId, location.script!.id!)) as Script;
     expect(script.uri, endsWith('debugging_test.dart'));
-    expect(
-        script.getLineNumberFromTokenPos(stack.frames![0].location!.tokenPos!),
-        16);
+    expect(script.getLineNumberFromTokenPos(location.tokenPos!), 16);
+    expect(location.line, 16);
   },
 
 // Stepping
@@ -136,13 +136,12 @@ var tests = <IsolateTest>[
     final isolateId = isolateRef.id!;
     final stack = await service.getStack(isolateId);
     expect(stack.frames!.length, greaterThanOrEqualTo(1));
-
-    final Script script = (await service.getObject(
-        isolateId, stack.frames![0].location!.script!.id!)) as Script;
+    final location = stack.frames![0].location!;
+    final Script script =
+        (await service.getObject(isolateId, location.script!.id!)) as Script;
     expect(script.uri, endsWith('debugging_test.dart'));
-    expect(
-        script.getLineNumberFromTokenPos(stack.frames![0].location!.tokenPos!),
-        17);
+    expect(script.getLineNumberFromTokenPos(location.tokenPos!), 17);
+    expect(location.line, 17);
   },
 // Remove breakpoint
   (VmService service, IsolateRef isolateRef) async {
@@ -214,6 +213,7 @@ var tests = <IsolateTest>[
         (await service.getObject(isolateId, bpt.location.script.id)) as Script;
     expect(script.uri, endsWith('debugging_test.dart'));
     expect(script.getLineNumberFromTokenPos(bpt.location.tokenPos), 14);
+    expect(bpt.location.line, 14);
 
     // Refresh isolate state.
     isolate = await service.getIsolate(isolateId);
@@ -226,13 +226,12 @@ var tests = <IsolateTest>[
     final isolateId = isolateRef.id!;
     final stack = await service.getStack(isolateId);
     expect(stack.frames!.length, greaterThanOrEqualTo(1));
-
-    final Script script = (await service.getObject(
-        isolateId, stack.frames![0].location!.script!.id!)) as Script;
+    final location = stack.frames![0].location!;
+    final Script script =
+        (await service.getObject(isolateId, location.script!.id!)) as Script;
     expect(script.uri, endsWith('debugging_test.dart'));
-    expect(
-        script.getLineNumberFromTokenPos(stack.frames![0].location!.tokenPos!),
-        14);
+    expect(script.getLineNumberFromTokenPos(location.tokenPos!), 14);
+    expect(location.line, 14);
   },
 ];
 

runtime/observatory/tests/service/get_version_rpc_test.dart

@@ -12,7 +12,7 @@ var tests = <VMTest>[
     final result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], 'Version');
     expect(result['major'], 3);
-    expect(result['minor'], 55);
+    expect(result['minor'], 56);
     expect(result['_privateMajor'], 0);
     expect(result['_privateMinor'], 0);
   },

runtime/observatory_2/tests/service_2/get_version_rpc_test.dart

@@ -12,7 +12,7 @@ var tests = <VMTest>[
     final result = await vm.invokeRpcNoUpgrade('getVersion', {});
     expect(result['type'], equals('Version'));
     expect(result['major'], equals(3));
-    expect(result['minor'], equals(55));
+    expect(result['minor'], equals(56));
     expect(result['_privateMajor'], equals(0));
     expect(result['_privateMinor'], equals(0));
   },

runtime/vm/json_stream.cc

@@ -523,6 +523,13 @@ void JSONObject::AddLocation(const Script& script,
   if (end_token_pos.IsReal()) {
     location.AddProperty("endTokenPos", end_token_pos);
   }
+  intptr_t line = -1;
+  intptr_t column = -1;
+  // Add line and column information if token_pos is real.
+  if (script.GetTokenLocation(token_pos, &line, &column)) {
+    location.AddProperty("line", line);
+    location.AddProperty("column", column);
+  }
 }
 
 void JSONObject::AddLocation(const BreakpointLocation* bpt_loc) const {

runtime/vm/json_test.cc

@@ -182,7 +182,8 @@ ISOLATE_UNIT_TEST_CASE(JSON_JSONStream_DartObject) {
       "Class\",\"fixedId\":true,\"id\":\"\",\"name\":\"Null\",\"location\":{"
       "\"type\":\"SourceLocation\",\"script\":{\"type\":\"@Script\","
       "\"fixedId\":true,\"id\":\"\",\"uri\":\"dart:core\\/null.dart\",\"_"
-      "kind\":\"kernel\"},\"tokenPos\":925,\"endTokenPos\":1165},\"library\":{"
+      "kind\":\"kernel\"},\"tokenPos\":925,\"endTokenPos\":1165,\"line\":23,"
+      "\"column\":1},\"library\":{"
       "\"type\":\"@Library\",\"fixedId\":true,\"id\":\"\",\"name\":\"dart."
       "core\",\"uri\":\"dart:core\"}},\"kind\":\"Null\",\"fixedId\":true,"
       "\"id\":\"\",\"valueAsString\":\"null\"},{\"object_key\":{\"type\":\"@"
@@ -190,7 +191,8 @@ ISOLATE_UNIT_TEST_CASE(JSON_JSONStream_DartObject) {
       "\"fixedId\":true,\"id\":\"\",\"name\":\"Null\",\"location\":{\"type\":"
       "\"SourceLocation\",\"script\":{\"type\":\"@Script\",\"fixedId\":true,"
       "\"id\":\"\",\"uri\":\"dart:core\\/null.dart\",\"_kind\":\"kernel\"},"
-      "\"tokenPos\":925,\"endTokenPos\":1165},\"library\":{\"type\":\"@"
+      "\"tokenPos\":925,\"endTokenPos\":1165,\"line\":23,\"column\":1},"
+      "\"library\":{\"type\":\"@"
       "Library\",\"fixedId\":true,\"id\":\"\",\"name\":\"dart.core\",\"uri\":"
       "\"dart:core\"}},\"kind\":\"Null\",\"fixedId\":true,\"id\":\"\","
       "\"valueAsString\":\"null\"}}]",

runtime/vm/service.h

@@ -15,7 +15,7 @@
 namespace dart {
 
 #define SERVICE_PROTOCOL_MAJOR_VERSION 3
-#define SERVICE_PROTOCOL_MINOR_VERSION 55
+#define SERVICE_PROTOCOL_MINOR_VERSION 56
 
 class Array;
 class EmbedderServiceHandler;

runtime/vm/service/service.md

@@ -1,8 +1,8 @@
-# Dart VM Service Protocol 3.55
+# Dart VM Service Protocol 3.56
 
 > Please post feedback to the [observatory-discuss group][discuss-list]
 
-This document describes of _version 3.55_ of the Dart VM Service Protocol. This
+This document describes of _version 3.56_ of the Dart VM Service Protocol. This
 protocol is used to communicate with a running Dart Virtual Machine.
 
 To use the Service Protocol, start the VM with the *--observe* flag.
@@ -3878,6 +3878,14 @@ class SourceLocation extends Response {
 
   // The last token of the location if this is a range.
   int endTokenPos [optional];
+
+  // The line associated with this location. Only provided for non-synthetic
+  // token positions.
+  int line [optional];
+
+  // The column associated with this location. Only provided for non-synthetic
+  // token positions.
+  int column [optional];
 }
 ```
 
@@ -4319,5 +4327,6 @@ version | comments
 3.53 | Added `setIsolatePauseMode` RPC.
 3.54 | Added `CpuSamplesEvent`, updated `cpuSamples` property on `Event` to have type `CpuSamplesEvent`.
 3.55 | Added `streamCpuSamplesWithUserTag` RPC.
+3.56 | Added optional `line` and `column` properties to `SourceLocation`.
 
 [discuss-list]: https://groups.google.com/a/dartlang.org/forum/#!forum/observatory-discuss

runtime/vm/source_report_test.cc

@@ -541,7 +541,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_SimpleCall) {
       "\"_intrinsic\":false,\"_native\":false,\"location\":{\"type\":"
       "\"SourceLocation\",\"script\":{\"type\":\"@Script\",\"fixedId\":true,"
       "\"id\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\",\"_kind\":\"kernel\"},"
-      "\"tokenPos\":0,\"endTokenPos\":11}},\"count\":1}]}]}],"
+      "\"tokenPos\":0,\"endTokenPos\":11,\"line\":1,\"column\":1}},\"count\":1}"
+      "]}]}],"
 
       // One script in the script table.
       "\"scripts\":[{\"type\":\"@Script\",\"fixedId\":true,\"id\":\"\","
@@ -602,7 +603,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_PolymorphicCall) {
       "\"script\":{\"type\":\"@Script\","
       "\"fixedId\":true,\"id\":\"\","
       "\"uri\":\"file:\\/\\/\\/test-lib\","
-      "\"_kind\":\"kernel\"},\"tokenPos\":0,\"endTokenPos\":27},"
+      "\"_kind\":\"kernel\"},\"tokenPos\":0,\"endTokenPos\":27,\"line\":1,"
+      "\"column\":1},"
       "\"library\":{\"type\":\"@Library\",\"fixedId\":true,"
       "\"id\":\"\",\"name\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\"}},"
 
@@ -614,7 +616,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_PolymorphicCall) {
       "\"script\":{\"type\":\"@Script\","
       "\"fixedId\":true,\"id\":\"\","
       "\"uri\":\"file:\\/\\/\\/test-lib\","
-      "\"_kind\":\"kernel\"},\"tokenPos\":0,\"endTokenPos\":27},"
+      "\"_kind\":\"kernel\"},\"tokenPos\":0,\"endTokenPos\":27,\"line\":1,"
+      "\"column\":1},"
       "\"library\":{\"type\":\"@Library\",\"fixedId\":true,"
       "\"id\":\"\",\"name\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\"}"
       "},\"_kind\":\"RegularFunction\","
@@ -624,7 +627,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_PolymorphicCall) {
       "\"location\":{\"type\":\"SourceLocation\","
       "\"script\":{\"type\":\"@Script\",\"fixedId\":true,"
       "\"id\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\","
-      "\"_kind\":\"kernel\"},\"tokenPos\":17,\"endTokenPos\":25}},"
+      "\"_kind\":\"kernel\"},\"tokenPos\":17,\"endTokenPos\":25,\"line\":2,"
+      "\"column\":3}},"
 
       "\"count\":2},"
 
@@ -635,7 +639,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_PolymorphicCall) {
       "\"script\":{\"type\":\"@Script\","
       "\"fixedId\":true,\"id\":\"\","
       "\"uri\":\"file:\\/\\/\\/test-lib\","
-      "\"_kind\":\"kernel\"},\"tokenPos\":29,\"endTokenPos\":58},"
+      "\"_kind\":\"kernel\"},\"tokenPos\":29,\"endTokenPos\":58,\"line\":4,"
+      "\"column\":1},"
       "\"library\":{\"type\":\"@Library\",\"fixedId\":true,"
       "\"id\":\"\",\"name\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\"}},"
 
@@ -647,7 +652,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_PolymorphicCall) {
       "\"script\":{\"type\":\"@Script\","
       "\"fixedId\":true,\"id\":\"\","
       "\"uri\":\"file:\\/\\/\\/test-lib\","
-      "\"_kind\":\"kernel\"},\"tokenPos\":29,\"endTokenPos\":58},"
+      "\"_kind\":\"kernel\"},\"tokenPos\":29,\"endTokenPos\":58,\"line\":4,"
+      "\"column\":1},"
       "\"library\":{\"type\":\"@Library\",\"fixedId\":true,"
       "\"id\":\"\",\"name\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\"}"
       "},\"_kind\":\"RegularFunction\","
@@ -657,7 +663,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_CallSites_PolymorphicCall) {
       "\"location\":{\"type\":\"SourceLocation\","
       "\"script\":{\"type\":\"@Script\",\"fixedId\":true,"
       "\"id\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\","
-      "\"_kind\":\"kernel\"},\"tokenPos\":48,\"endTokenPos\":56}},"
+      "\"_kind\":\"kernel\"},\"tokenPos\":48,\"endTokenPos\":56,\"line\":5,"
+      "\"column\":3}},"
 
       "\"count\":1}]}]}],"
 
@@ -713,7 +720,8 @@ ISOLATE_UNIT_TEST_CASE(SourceReport_MultipleReports) {
       "intrinsic\":false,\"_native\":false,\"location\":{\"type\":"
       "\"SourceLocation\",\"script\":{\"type\":\"@Script\",\"fixedId\":true,"
       "\"id\":\"\",\"uri\":\"file:\\/\\/\\/test-lib\",\"_kind\":\"kernel\"},"
-      "\"tokenPos\":0,\"endTokenPos\":11}},\"count\":1}]}],\"coverage\":{"
+      "\"tokenPos\":0,\"endTokenPos\":11,\"line\":1,\"column\":1}},\"count\":1}"
+      "]}],\"coverage\":{"
       "\"hits\":[26,37],\"misses\":[]}}],"
 
       // One script in the script table.