Switch FE to use the libraries.json format.

pkg/front_end/lib/src/base/libraries_spec.dart

+// Copyright (c) 2017, the Dart project authors.  Please see the AUTHORS file

[ahe, 2017/08/03 11:58:15]

Consider renaming this file to libraries_specification.dart.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done.

+// 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 specification in-memory representation.
+///
+/// Many dart tools are configurable to support different target platforms.  For
+/// a given target, they need to know what libraries are available and where are
+/// the sources and target-specific patches.
+///
+/// Here we define APIs to represent this specification and implement
+/// serialization to (and deserialization from) JSON.  The JSON representation
+/// follows the following structure (which we write in YAML here for
+/// readability purposes):

[ahe, 2017/08/03 11:58:15]

I don't think using YAML is helpful if the format is JSON. It increases the
mental overhead and assumes familiarity with YAML and JSON, and how they map to
each other.

[Paul Berry, 2017/08/03 17:50:41]

I think I agree with this...it makes my brain hurt a bit to read this
pseudo-YAML :)

[Siggi Cherem (dart-lang), 2017/08/05 00:41:01]

Done :)

+///
+///     vm:              # top-level keys are the target names
+///       libraries:     # there must be a 'libraries' entry under a target
+///         async:       # key matches the path in `dart:*` imports
+///
+///           # `path` declares the main library uri, note that the actual path
+///           # is relative to this specification file

[ahe, 2017/08/03 11:58:15]

You're saying that `path` is a URI (that's relative). But below, you're
interpreting it as a file path.

[Paul Berry, 2017/08/03 17:50:41]

Yeah, since paths use different separators on Windows, let's be explicit here. 
I assume that this "path" should always use "/" (even on Windows) so that we
don't have to distribute different JSON files for use on Windows.

Also, it's not clear to me whether this path is *required* to be relative, or
whether it's allowed to be absolute.  If it's required to be relative, is it
allowed to contain ".."?  If it's allowed to be absolute, is it allowed to
declare a scheme?  The answers to these questions might have security
implications (because if we consider the contents of the JSON to be "untrusted
input" it could potentially be used to redirect us to open any file) so I think
we should start off conservative: require it to be a posix-style relative path
with no ".."s.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:01]

Ok - I decided to match what we do here with what we do for .packages files. The
package-config specifies that the values should be uri references, so I updated
the text and logic below to do the same here.

Note: I also believe we need to support "../" because of how the sdk libraries
and patch files are laid out in the repo today (some of which are under sdk/lib/
some under runtime/...). That's also supported for .packages files, so accepting
this would also keep it consistent.

+///           path: async/async.dart
+///
+///           # `patches` declares all patch files of the target # platform.
+///           # note: some platforms have more than one patch file, so `patches`
+///           # is expected to be a list
+///           patches:
+///             - _internal/js_runtime/lib/async_patch.dart
+///             ...
+///
+///       environment: # not supported today, but possibly in the future other
+///                    # platform-specific configuration can go here.
+///         dart.libraries.io: true
+///
+/// Note: we currently have several different files that needs to be updated
+/// when changing libraries, sources, and patch files:
+///    * .platform files (for dart2js)
+///    * .gypi files (for vm)
+///    * sdk_library_metadata/lib/libraries.dart (for analyzer, ddc)
+///
+/// we are in the process of unifying them all under this format (see
+/// https://github.com/dart-lang/sdk/issues/28836), but for now we need to pay
+/// close attention to change them consistently.
+
+// TODO(sigmund): move this file to a shared package.
+import 'dart:convert' show JSON;
+
+import '../fasta/util/relativize.dart';
+
+/// Contents from a single library specification file.
+///
+/// Contains information about all libraries on all target platforms defined in
+/// that file.
+class LibrariesSpecification {
+  final Map<String, TargetLibrariesSpecification> _targets;
+
+  const LibrariesSpecification(
+      [this._targets = const <String, TargetLibrariesSpecification>{}]);
+
+  /// The [TargetLibrariesSpecification] for [targetName].

[ahe, 2017/08/03 11:58:15]

Isn't this kinda obvious?

[Siggi Cherem (dart-lang), 2017/08/05 00:41:01]

Rephrased a bit to indicate what happens if target is not specified in the file.

+  TargetLibrariesSpecification specificationFor(String targetName) =>
+      _targets[targetName];
+
+  /// Parse the given [json] as a library specification, resolving any relative
+  /// paths from [fileUri].
+  static LibrariesSpecification parse(Uri fileUri, String json) {

[ahe, 2017/08/03 11:58:15]

Consider renaming fileUri to base or baseUri.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:01]

Done.

+    if (json == null) return const LibrariesSpecification();
+    Map jsonData = JSON.decode(json);

[ahe, 2017/08/03 11:58:15]

May throw a format exception.

[ahe, 2017/08/03 11:58:15]

jsonData may not be a map.

[Paul Berry, 2017/08/03 17:50:41]

Agreed, and given that we're still straddling the line between strong and
non-strong semantics, this code might be run in non-strong mode with checked
assertions off, so the problem would not be caught until line 70, with a
potentially confusing exception message.

IMHO it wuld be sufficient to do:

var jsonData = JSON.decode(json) as Map;

[Paul Berry, 2017/08/03 17:50:41]

I think it would be ok to address this possibility by declaring in the comment
above line 66 that if [json] is improperly formatted, an exception will be
raised.  (IMHO it's not necessary to catch the exception from JSON.decode and
wrap it)

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done. I added more general error handling here and below, with some unit tests.

Paul - you might have not seen that discussion about .packages on an earlier CL,
here is more context: https://github.com/dart-lang/package_config/issues/40.
Given that, I decided to wrap the FormatException as well.

+    var targets = <String, TargetLibrariesSpecification>{};
+    jsonData.forEach((String targetName, targetData) {

[ahe, 2017/08/03 11:58:15]

targetData here.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done

+      Map<String, LibraryInfo> libraries = <String, LibraryInfo>{};
+      Map targetData = jsonData[targetName];

[ahe, 2017/08/03 11:58:15]

targetData may not be a map.

[ahe, 2017/08/03 11:58:15]

And here.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:01]

Done.

+      if (targetData != null) {

[ahe, 2017/08/03 11:58:15]

Consider:

if (targetData is Map) {
...
} else {
  report error.
}

[Siggi Cherem (dart-lang), 2017/08/05 00:41:01]

Done.

+        Map librariesData = targetData["libraries"];

[ahe, 2017/08/03 11:58:15]

librariesData may not be a map.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done.

+        Uri resolve(String path) => fileUri.resolveUri(new Uri.file(path));

[ahe, 2017/08/03 11:58:15]

Why is it a filename, and not a URI? I'm pretty sure that this will catch
everyone by surprise on Windows.

[Paul Berry, 2017/08/03 17:50:41]

Agreed.  And considering the security concern I brought up earlier, I think we
should be really conservative in what we accept.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done. this is now consistently handled as a Uri reference.

+        librariesData.forEach((String name, Map data) {
+          var uri = resolve(data['path']);
+          var patches = data['patches']?.map(resolve)?.toList() ?? const [];
+          libraries[name] = new LibraryInfo(name, uri, patches);
+        });
+      }
+      targets[targetName] =
+          new TargetLibrariesSpecification(targetName, libraries);
+    });
+    return new LibrariesSpecification(targets);
+  }
+
+  /// Serialize this specification to json.
+  ///
+  /// If possible serializes paths relative to [outputUri].
+  String toJsonString(Uri outputUri) => JSON.encode(toJsonMap(outputUri));
+
+  Map toJsonMap(Uri outputUri) {
+    var result = {};
+    var dir = outputUri.resolve('.');
+    String pathFor(Uri uri) => relativizeUri(uri, base: dir);
+    _targets.forEach((targetName, target) {
+      var libraries = {};
+      target._libraries.forEach((name, lib) {
+        libraries[name] = {
+          'path': pathFor(lib.uri),

[ahe, 2017/08/03 11:58:15]

This is written as a URI, not a file name.

[Siggi Cherem (dart-lang), 2017/08/05 00:41:02]

Done.

+          'patches': lib.patches.map(pathFor).toList(),
+        };
+      });
+      result[targetName] = {'libraries': libraries};
+    });
+    return result;
+  }
+}
+
+/// Specifies information about all libraries supported by a given target.
+class TargetLibrariesSpecification {
+  /// Name of the target platform.
+  final String targetName;
+
+  final Map<String, LibraryInfo> _libraries;
+
+  const TargetLibrariesSpecification(this.targetName,
+      [this._libraries = const <String, LibraryInfo>{}]);
+
+  /// Details about a library whose import is `dart:$name`.
+  LibraryInfo libraryInfoFor(String name) => _libraries[name];
+}
+
+/// Information about a `dart:` library in a specific target platform.
+class LibraryInfo {
+  /// The name of the library, which is the path developers use to import this
+  /// library (as `dart:$name`).
+  final String name;
+
+  /// The file defining the main implementation of the library.
+  final Uri uri;
+
+  /// Patch files used for this library in the target platform, if any.
+  final List<Uri> patches;
+
+  const LibraryInfo(this.name, this.uri, this.patches);
+}