pkg/vm_service_client: add getSourceReport to VMServiceReference

lib/src/source_report.dart

+// Copyright (c) 2016, 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.
+
+import 'script.dart';
+import 'scope.dart';
+import 'source_location.dart';
+
+List<VMScriptToken> _getTokens(VMScriptRef script, Iterable<int> locations) {

[nweiz, 2016/05/04 23:05:52]

Make this a static method in VMSourceReport—that helps communicate where it's
used, and keeps the definition closer to the use.

Also, document it.

"locations" should probably be "positions", to match the terminology in the
service protocol documentation.

[kevmoo, 2016/05/05 20:46:27]

Done.

+  if (locations == null) return null;
+
+  return new List<VMScriptToken>.unmodifiable(
+      locations.map((i) => newVMScriptTokenFromScript(script, i)));

[nweiz, 2016/05/04 23:05:52]

"i" -> "location" (or "position")

[kevmoo, 2016/05/05 20:46:26]

Done.

+}
+
+VMSourceReport newSourceReport(Scope scope, Map json) {
+  if (json == null) return null;
+  assert(json["type"] == "SourceReport");
+
+  var scripts = (json['scripts'] as List)
+      .map((scriptItem) => newVMScriptRef(scope, scriptItem))

[nweiz, 2016/05/04 23:05:52]

Just call this variable "script", like in the other similar code in this
package.

[kevmoo, 2016/05/05 20:46:26]

Done.

+      .toList();
+
+  var ranges = new List<VMSourceReportRange>.unmodifiable(
+      (json['ranges'] as List)
+          .map((rangeItem) => _newSourceReportRange(rangeItem, scripts)));

[nweiz, 2016/05/04 23:05:52]

Don't do actual work in the "new*" functions. It makes this library confusingly
different from all the others in the package, which makes it more difficult to
figure out what's happening where.

[kevmoo, 2016/05/05 20:46:26]

Acknowledged.

[nweiz, 2016/05/11 23:03:31]

It doesn't look like this was addressed.

+
+  return new VMSourceReport._(ranges);
+}
+
+VMSourceReportRange _newSourceReportRange(Map json, List<VMScriptRef> scripts) {
+  if (json == null) return null;
+
+  var script = scripts[json['scriptIndex']];
+
+  var location =
+      newVMSourceLocationFromLocation(script, json['startPos'], json['endPos']);
+
+  return new VMSourceReportRange._(json, script, location);
+}
+
+/// A report about ranges of source code.
+///
+/// [ranges] exposes code coverage and information on possible breakpoints.
+class VMSourceReport {
+  /// Ranges in the program source corresponding
+  /// to ranges of executable code in the user's program (functions,
+  /// methods, constructors, etc.)

[nweiz, 2016/05/04 23:05:52]

Avoid abbreviations like "etc"
(https://www.dartlang.org/effective-dart/documentation/#avoid-abbreviations-an...)

Also add a period.

[kevmoo, 2016/05/05 20:46:26]

Done.

+  ///
+  /// Note:
+  ///  * ranges may nest in other ranges, in the case of nested
+  ///    functions.
+  ///  * ranges may be duplicated, in the case of mixins.

[nweiz, 2016/05/04 23:05:52]

I think bulleted lists are generally harder to read than prose. It's certainly
harder to ensure that the markdown won't break somehow. Are you confident that
dartdoc will render this correctly?

[kevmoo, 2016/05/05 20:46:26]

Replaced with sentences.

+  final List<VMSourceReportRange> ranges;
+
+  VMSourceReport._(this.ranges);
+}
+
+/// A range of executable code (function, method, constructor, etc)
+/// in the running program.
+///
+/// This is part of a [VMSourceReport].
+class VMSourceReportRange {
+  VMScriptRef get script => location.script;

[nweiz, 2016/05/04 23:05:52]

Document all public members.

[kevmoo, 2016/05/05 20:46:27]

Done.

[nweiz, 2016/05/11 23:03:31]

This member is still not documented.

+
+  final VMSourceLocation location;
+
+  /// `true` if this range been compiled by the Dart VM.

[nweiz, 2016/05/04 23:05:52]

I like to say "Whether" rather than "`true` if" to avoid starting a sentence
with a lower-case identifier. Up to you, though.

[kevmoo, 2016/05/05 20:46:27]

Acknowledged.

+  ///
+  /// If `false`, `possibleBreakpoints`, `hits`, and `misses` will be `null`.
+  ///
+  /// Use `forceCompile` in `getSourceReport` to ensure the target scripts are

[nweiz, 2016/05/04 23:05:52]

Make "getSourceReport" a link. Also below.

[kevmoo, 2016/05/05 20:46:26]

I didn't do this intentionally – since there are two of these.

[nweiz, 2016/05/11 23:03:31]

Linking to one of them is way better than linking to zero.

+  /// compiled.
+  final bool compiled;
+
+  /// Token locations at which breakpoints can be set.

[nweiz, 2016/05/04 23:05:52]

"Token locations" -> "Token positions"

[kevmoo, 2016/05/05 20:46:26]

Done.

+  ///
+  /// Provided only when the when the `includePossibleBreakpoints` in

[nweiz, 2016/05/04 23:05:52]

Remove "the".

[kevmoo, 2016/05/05 20:46:26]

Done.

+  /// `getSourceReport` is `true` and the range has been compiled.
+  final List<VMScriptToken> possibleBreakpoints;
+
+  /// A sorted list of token positions in a [VMSourceReportRang] which have been

[nweiz, 2016/05/04 23:05:52]

"Rang" -> "Range"

[kevmoo, 2016/05/05 20:46:26]

Done.

+  /// executed.
+  ///
+  /// Provided only when the when the `includeCoverageReport` in

[nweiz, 2016/05/04 23:05:52]

Remove "the"

[kevmoo, 2016/05/05 20:46:26]

Done.

+  /// `getSourceReport` is `true` and the range has been compiled.
+  final List<VMScriptToken> hits;
+
+  /// A sorted list of token positions in a [VMSourceReportRange] which have not
+  /// yet been executed.
+  ///
+  /// Provided only when the

[nweiz, 2016/05/04 23:05:52]

Remove "the"

[kevmoo, 2016/05/05 20:46:26]

Done.

+  /// `includeCoverageReport` is true and the range has been
+  /// compiled.
+  final List<VMScriptToken> misses;
+
+  VMSourceReportRange._(Map json, VMScriptRef script, this.location)
+      : compiled = json['compiled'],
+        hits = json['coverage'] == null
+            ? null
+            : _getTokens(script, json['coverage']['hits']),
+        misses = json['coverage'] == null
+            ? null
+            : _getTokens(script, json['coverage']['misses']),
+        possibleBreakpoints = json['possibleBreakpoints'] == null
+            ? null
+            : _getTokens(script, json['possibleBreakpoints']);
+}