Both halves of the HTMLDoc to JSON doc converter!

tools/html-json-doc/lib/HtmlToJson.dart

+// Copyright (c) 2012, 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.
+
+/**
+ * Library for extracting the documentation comments from files generated by
+ * the HTML library. The comments are stored in a JSON file.
+ *
+ * Comments must be in either the block style with leading *s:
+ *
+ *     /**
+ *      * Comment here.
+ *      */
+ *
+ * Or the triple-slash style:
+ *
+ *     /// Docs go here.
+ *     /// And here.
+ *
+ * Each member that is to be documented should be preceeded by a meta-comment
+ * containing the string `@docsEditable` such as:
+ *
+ *     /// @docsEditable
+ */
+library htmlToJson;
+
+import 'dart:json';
+import 'dart:io';
+
+
+/// True if any errors were triggered through the conversion.
+bool _anyErrors = false;
+
+
+/**
+ * Convert files on [htmlPath] and write JSON to [jsonPath].

[Bob Nystrom, 2012/11/26 22:00:19]

You do a mixture of /// and /** */ style doc comments. Any reason to not just
pick one?

[Andrei Mouravski, 2012/11/27 03:11:45]

Because nothing forces me to.

+ */
+Future<bool> convert(Path htmlPath, Path jsonPath) {
+  var completer = new Completer();
+
+  var futureConvert = _convertFiles(htmlPath);
+
+  // TODO(amouravski): make this transform once I know what I want this file to
+  // return.
+  futureConvert.then((convertedJson) {

[Bob Nystrom, 2012/11/26 22:00:19]

You don't actually use this variable for anything else, so you may as well
inline the expression:

_convertFiles(htmlPath).then((convertedJson) { ... });

Reads nicely that way.

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

+    final jsonFile = new File.fromPath(jsonPath);
+    var writeJson = convertedJson;
+
+    if (jsonFile.existsSync()) {
+      var writeJson = _mergeJsonAndFile(convertedJson, jsonFile);

[Bob Nystrom, 2012/11/26 22:00:19]

Do you intend to shadow writeJson here?

[Andrei Mouravski, 2012/11/27 03:11:45]

Wow. Nope.

+    } else {
+      jsonFile.createSync();

[Bob Nystrom, 2012/11/26 22:00:19]

I don't think this is necessary. openOutputStream should create the file for
you.

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

+    }
+
+    var outputStream = jsonFile.openOutputStream();
+    outputStream.writeString(prettyPrintJson(writeJson));
+
+    outputStream.onNoPendingWrites = () {
+      completer.complete(_anyErrors);
+    };
+    
+    outputStream.onClosed = () {
+      completer.complete(_anyErrors);
+    };

[Bob Nystrom, 2012/11/26 22:00:19]

Also add:

outputStream.onError = completer.completeException;

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

+  });
+
+  return completer.future;
+}
+
+
+/**
+ * Convert all files on [htmlPath].
+ *
+ * Returns a future that completes to the converted JSON object.
+ */
+Future<Object> _convertFiles(Path htmlPath) {
+  var completer = new Completer();
+
+  List<Future> fileFutures = [];
+
+  // Get a list of all HTML dart files.
+  // TODO(amouravski): discriminate .dart files.
+  final htmlDir = new Directory.fromPath(htmlPath);
+  final lister = htmlDir.list(recursive: false);
+
+  lister.onFile = (String path) {
+    final name = new Path.fromNative(path).filename;
+
+    // Ignore private classes.
+    if (name.startsWith('_')) return;
+
+    // Ignore non-dart files.
+    if (!name.endsWith('.dart')) return;
+
+    File file = new File(path);
+
+    // TODO(amouravski): Handle missing file.
+    if (!file.existsSync()) {
+      print('ERROR: cannot find file $path');
+      _anyErrors = true;
+      return;
+    }
+
+    fileFutures.add(_convertFile(file));
+  };
+
+
+  // Combine all JSON objects
+  lister.onDone = (_) {
+    Futures.wait(fileFutures).then((jsonList) {
+      var convertedJson = {};
+      jsonList.forEach((json) {
+        final k = json.keys[0];
+        convertedJson.putIfAbsent(k, () => json[k]);
+      });
+      completer.complete(convertedJson);
+    });
+  };
+
+  // TODO(amouravski): add more error handling.
+
+  return completer.future;
+}
+
+
+/**
+ * Convert a single file to JSON docs.
+ *
+ * Output is a single object which with one element that maps the file name

[Bob Nystrom, 2012/11/26 22:00:19]

How about "Returns a map with one entry whose key is the file name and whose
value is the list of comment lines."?

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

+ * to the list of comment lines.
+ */
+Future<Object> _convertFile(File file) {

[Bob Nystrom, 2012/11/26 22:00:19]

Future<Map>

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

+  var completer = new Completer();
+
+  var comments = {};
+
+  // Find all /// @docsEditable annotations.
+  InputStream file_stream = file.openInputStream();
+  StringInputStream inputLines = new StringInputStream(file_stream);
+
+  inputLines.onLine = () {
+    var comment = <String>[];
+
+    var docCommentFound = false;
+    String line;
+    while ((line = inputLines.readLine()) != null) {
+      var trimmedLine = line.trim();
+
+      // Sentinel found. Process the comment block.
+      if (trimmedLine.startsWith('///') &&
+          trimmedLine.contains('@docsEditable')) {
+        if (docCommentFound == true) {
+          var nextLine = inputLines.readLine();

[Bob Nystrom, 2012/11/26 22:00:19]

I don't think you can do this. readLine() can fail here non-deterministically if
it hasn't happened to buffer more than one line.

The simplest solution is to just sidestep the whole StringInputStream stuff
completely and do:

file.readAsLines().then((lines) {
  walk the list of lines...
}

[Andrei Mouravski, 2012/11/27 03:11:45]

Will get back to this if possible.

+
+          if (nextLine == null) return false;
+
+          var lineObject = {};
+          //lineObject[nextLine] = comment;
+          comments.putIfAbsent(nextLine, () => comment);
+        }
+
+        // Reset.
+        docCommentFound = false;
+        comment = <String>[];
+      } else if ( // Start a comment block.
+          trimmedLine.startsWith('/**') ||
+          trimmedLine.startsWith('///')) {
+        docCommentFound = true;
+        comment.add(line);
+      } else if (docCommentFound &&
+          (trimmedLine.startsWith('*') || trimmedLine.startsWith('///'))) {

[Bob Nystrom, 2012/11/26 22:00:19]

This would barf if you did:

/// A doc comment
* not part of a doc comment.

You'll probably need separate states for in a block comment versus in a series
of line comments.

[Andrei Mouravski, 2012/11/27 03:11:45]

I will barf, yes. In the interest of time, I'm going to add a TODO.

+        comment.add(line);
+      } else {
+        // Reset if we're not in a comment.
+        docCommentFound = false;
+        comment = <String>[];
+      }
+    }
+  };
+
+  inputLines.onClosed = () {
+    var jsonObject = {};
+    jsonObject[new Path(file.fullPathSync()).filename] = comments;
+    completer.complete(jsonObject);
+  };
+
+  // TODO(amouravski): better error handling.
+
+  return completer.future;
+}
+
+
+/**
+ * Merge the new JSON object and the existing file.
+ */
+Object _mergeJsonAndFile(Object json, File file) {
+  var completer = new Completer();
+
+  var fileJson = {};
+  var jsonRead = Strings.join(file.readAsLinesSync(), '\n');

[Bob Nystrom, 2012/11/26 22:00:19]

file.readAsStringSync();

[Andrei Mouravski, 2012/11/27 03:11:45]

I wrote this before I knew that method existed. :]

+
+  if (jsonRead == '') {
+    print('WARNING: no data read from '
+        '${new Path(file.fullPathSync()).filename}');
+    _anyErrors = true;
+  } else {
+    fileJson = JSON.parse(jsonRead);
+  }
+  return _mergeJson(json, fileJson);
+}
+
+
+/**
+ * Merge two JSON objects, such that the next JSON object is the
+ * union of both.
+ *
+ * Each JSON must be a map, with each value being a map.
+ */
+Object _mergeJson(Object json1, Object json2) {
+  if (json1 is Map && json2 is Map) {
+    // Then check if [json2] contains any key form [json1], in which case
+    // add all of the values from [json] to the values of [json1].
+    json2.forEach((k, v) {
+      if (json1.containsKey(k)) {
+        v.forEach((vk, vv) {
+          if (json1[k].containsKey(vk) &&
+              !_listsEqual(json1[k][vk],vv)) {
+            // TODO(amouravski): add better warning message and only if there's
+            // a conflict.
+            print('WARNING: duplicate keys.');
+            _anyErrors = true;
+          } else {
+            json1[k].putIfAbsent(vk, () => vv);
+          }
+        });
+      } else {
+        json1.putIfAbsent(k, () => v);
+      }

[Bob Nystrom, 2012/11/26 22:00:19]

This is a bit hard for me to read. What do you think of:

/// Merges [a] and [b]. Returns a map that contains all keys that are present
/// in either [a] or [b]. If a key is only present in one map, its value will be
/// used. If a key is present in both, it will be merged if both values are
/// maps. If they are lists, they must contain the same items. All other value
/// types are errors.
Map merge(Map a, Map b) {
  var keys = new Set.from(a.keys);
  keys.addAll(b.keys);

  var result = {};
  for (var key in keys) {
    result[key] = mergeValues(a[key], b[key]);
  }

  return result;
}

Object mergeValues(Object a, Object b) {
  // If only one value is provided, use it.
  if (a == null) return b;
  if (b == null) return a;

  // If both values are maps, so recursively merge.
  if (a is Map && b is Map) return merge(a, b);

  // If both values are lists, they have to have the same contents.
  if (a is List && b is List) {
    // Key is in both and both values are lists, so must agree.
    if (!a.every((e) => b.contains(e)) ||
        !b.every((e) => a.contains(e)) {
      // TODO(amouravski): add better warning message and only if there's
      // a conflict.
      print('WARNING: duplicate keys.');
    }
    return a;
  }

  throw "Don't know how to merge $a and $b.";
}

[Andrei Mouravski, 2012/11/27 03:11:45]

Will return to this, too.

+    });
+  } else {
+    throw new ArgumentError('JSON objects must both be Maps');
+  }
+
+  // TODO(amouravski): more error handling.
+
+  return json1;
+}
+
+
+/**
+ * Tests for equality between two lists.
+ *
+ * This checks the first level of depth, so does not work for nested lists.
+ */
+bool _listsEqual(List list1, List list2) {
+  return list1.every((e) => list2.contains(e)) &&
+      list2.every((e) => list1.contains(e));
+}
+
+
+/**
+ * Print JSON in a much nicer format.
+ *
+ * For example:
+ *     {"foo":["bar","baz"],"boo":{"far:"faz"}}
+ *
+ * becomes:
+ *
+ *     {
+ *       "foo":
+ *         [
+ *           "bar",
+ *           "baz"
+ *         ],
+ *       "boo":
+ *         {
+ *           "far":
+ *             "faz"
+ *         }
+ *     }
+ */
+String prettyPrintJson(Object json, [depth = 0]) {
+  var output;
+
+  var printSpaces = Strings.join(
+      new List<String>(depth + 1).map((e) => ''), '  ');

[Bob Nystrom, 2012/11/26 22:00:19]

This is a bit gratuitiously inefficient. How about:

"depth = 0" -> "indent = ''"
"$printSpaces" -> "$indent"
"prettyPrintJson(e, depth + 1)" -> "prettyPrintJson(e, '$indent  ')"

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

Gratuitious amounts of inefficiency. Powerthirst.

+
+  if (json is List) {
+    var recursiveOutput =
+        Strings.join(json.map((e) => prettyPrintJson(e, depth + 1)), ',\n');
+    output = '${printSpaces}[\n'

[Bob Nystrom, 2012/11/26 22:00:19]

If it's a single identifier, you can omit the {}, here and elsewhere. Just
$printSpaces.

[Andrei Mouravski, 2012/11/27 03:11:45]

Done.

+      '${recursiveOutput}'
+      '\n${printSpaces}]';
+  } else if (json is Map) {
+    var mapList = json.keys.map((key) =>
+        '${printSpaces}"${key}":\n${prettyPrintJson(json[key], depth + 1)}');

[Bob Nystrom, 2012/11/26 22:00:19]

Nit, but how about getting rid of the newline after ":"? That would get closer
to our style guide:

{
  "foo": [
    "bar",
    "baz"
  ]
}

[Andrei Mouravski, 2012/11/27 03:11:45]

I'd like to, but that'd mean editing a lot of files, and I can't manage that
right now, sorry. :[

+    var recursiveOutput =
+        Strings.join(mapList, ',\n');

[Bob Nystrom, 2012/11/26 22:00:19]

Nit: this can fit on one line.

[Andrei Mouravski, 2012/11/27 03:11:45]

At one point it didn't.

+    output = '${printSpaces}{\n'
+      '${recursiveOutput}'
+      '\n${printSpaces}}';
+  } else {
+    output = '${printSpaces}"${json}"';
+  }
+  return output;
+}