fix #626, add AMD module format and make it default

lib/src/compiler/compiler.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 'dart:collection' show HashSet, Queue;
+import 'dart:convert' show JSON;
+import 'dart:io' show File;
 import 'package:analyzer/dart/element/element.dart' show LibraryElement;
 import 'package:analyzer/analyzer.dart'
     show AnalysisError, CompilationUnit, ErrorSeverity;
 import 'package:analyzer/file_system/file_system.dart' show ResourceProvider;
 import 'package:analyzer/src/generated/engine.dart' show AnalysisContext;
 import 'package:analyzer/src/generated/source.dart' show DartUriResolver;
 import 'package:analyzer/src/generated/source_io.dart'
     show Source, SourceKind, UriResolver;
 import 'package:analyzer/src/summary/package_bundle_reader.dart'
     show InSummarySource;
 import 'package:args/args.dart' show ArgParser, ArgResults;
 import 'package:args/src/usage_exception.dart' show UsageException;
 import 'package:func/func.dart' show Func1;
 import 'package:path/path.dart' as path;
+import 'package:source_maps/source_maps.dart';
 
 import '../analyzer/context.dart'
     show AnalyzerOptions, createAnalysisContextWithSources;
-import 'extension_types.dart' show ExtensionTypeSet;
+import '../js_ast/js_ast.dart' as JS;
 import 'code_generator.dart' show CodeGenerator;
 import 'error_helpers.dart' show errorSeverity, formatError, sortErrors;
+import 'extension_types.dart' show ExtensionTypeSet;
+import 'js_names.dart' as JS;
+import 'module_builder.dart' show lowerModuleFormat, ModuleFormat;
+import 'source_map_printer.dart' show SourceMapPrintingContext;
 
 /// Compiles a set of Dart files into a single JavaScript module.
 ///
 /// For a single [BuildUnit] definition, this will produce a [JSModuleFile].
 /// Those objects are record types that record the data consumed and produced
 /// for a single compile.
 ///
 /// This class exists to cache global state associated with a single in-memory
 /// AnalysisContext, such as information about extension types in the Dart SDK.
 /// It can be used once to produce a single module, or reused to save warm-up
@@ skipping 92 matching lines @@
 
     sortErrors(context, errors);
     var messages = <String>[];
     for (var e in errors) {
       var m = formatError(context, e);
       if (m != null) messages.add(m);
     }
 
     if (!options.unsafeForceCompile &&
         errors.any((e) => errorSeverity(context, e) == ErrorSeverity.ERROR)) {
-      return new JSModuleFile.invalid(unit.name, messages);
+      return new JSModuleFile.invalid(unit.name, messages, options);
     }
 
     var codeGenerator = new CodeGenerator(context, options, _extensionTypes);
     return codeGenerator.compile(unit, trees, messages);
   }
 }
 
-enum ModuleFormat { es6, legacy, node }
-
-ModuleFormat parseModuleFormat(String s) => {
-      'es6': ModuleFormat.es6,
-      'node': ModuleFormat.node,
-      'legacy': ModuleFormat.legacy
-    }[s];
-
 class CompilerOptions {
   /// Whether to emit the source mapping file.
   ///
   /// This supports debugging the original source code instead of the generated
   /// code.
   final bool sourceMap;
 
   /// If [sourceMap] is emitted, this will emit a `sourceMappingUrl` comment
   /// into the output JavaScript module.
   final bool sourceMapComment;
@@ skipping 38 matching lines @@
   /// Supporting the syntax:
   /// * Chrome Canary (51)
   /// * Firefox
   ///
   /// Not yet supporting:
   /// * Atom (1.5.4)
   /// * Electron (0.36.3)
   // TODO(ochafik): Simplify this code when our target platforms catch up.
   final bool destructureNamedParams;
 
-  /// Which module format to support.
-  /// Currently 'es6' and 'legacy' are supported.
-  final ModuleFormat moduleFormat;
-
   const CompilerOptions(
       {this.sourceMap: true,
       this.sourceMapComment: true,
       this.summarizeApi: true,
       this.summaryExtension: 'sum',
       this.unsafeForceCompile: false,
       this.emitMetadata: false,
       this.closure: false,
       this.destructureNamedParams: false,
-      this.moduleFormat: ModuleFormat.legacy,
       this.hoistInstanceCreation: true,
       this.hoistSignatureTypes: false,
       this.nameTypeTests: true,
       this.hoistTypeTests: true,
       this.useAngular2Whitelist: false});
 
   CompilerOptions.fromArguments(ArgResults args)
       : sourceMap = args['source-map'],
         sourceMapComment = args['source-map-comment'],
         summarizeApi = args['summarize'],
         summaryExtension = args['summary-extension'],
         unsafeForceCompile = args['unsafe-force-compile'],
         emitMetadata = args['emit-metadata'],
         closure = args['closure-experimental'],
         destructureNamedParams = args['destructure-named-params'],
-        moduleFormat = parseModuleFormat(args['modules']),
         hoistInstanceCreation = args['hoist-instance-creation'],
         hoistSignatureTypes = args['hoist-signature-types'],
         nameTypeTests = args['name-type-tests'],
         hoistTypeTests = args['hoist-type-tests'],
         useAngular2Whitelist = args['unsafe-angular2-whitelist'];
 
   static void addArguments(ArgParser parser) {
     parser
       ..addFlag('summarize', help: 'emit an API summary file', defaultsTo: true)
       ..addOption('summary-extension',
           help: 'file extension for Dart summary files',
           defaultsTo: 'sum',
           hide: true)
       ..addFlag('source-map', help: 'emit source mapping', defaultsTo: true)
       ..addFlag('source-map-comment',
           help: 'adds a sourceMappingURL comment to the end of the JS,\n'
               'disable if using X-SourceMap header',
           defaultsTo: true,
           hide: true)
-      ..addOption('modules',
-          help: 'module pattern to emit',
-          allowed: ['es6', 'legacy', 'node'],
-          allowedHelp: {
-            'es6': 'es6 modules',
-            'legacy': 'a custom format used by dartdevc, similar to AMD',
-            'node': 'node.js modules (https://nodejs.org/api/modules.html)'
-          },
-          defaultsTo: 'legacy')
       ..addFlag('emit-metadata',
           help: 'emit metadata annotations queriable via mirrors',
           defaultsTo: false)
       ..addFlag('closure-experimental',
           help: 'emit Closure Compiler-friendly code (experimental)',
           defaultsTo: false)
       ..addFlag('destructure-named-params',
           help: 'Destructure named parameters', defaultsTo: false, hide: true)
       ..addFlag('unsafe-force-compile',
           help: 'Compile code even if it has errors. ಠ_ಠ\n'
@@ skipping 14 matching lines @@
           help: 'Hoist types used in type tests', defaultsTo: true, hide: true)
       ..addFlag('unsafe-angular2-whitelist', defaultsTo: false, hide: true);
   }
 }
 
 /// A unit of Dart code that can be built into a single JavaScript module.
 class BuildUnit {
   /// The name of this module.
   final String name;
 
-  /// Library root.  All library names are relative to this path/prefix.
+  /// All library names are relative to this path/prefix.
   final String libraryRoot;
 
   /// The list of sources in this module.
   ///
   /// The set of Dart files can be arbitrarily large, but it must contain
   /// complete libraries including all of their parts, as well as all libraries
   /// that are part of a library cycle.
   final List<String> sources;
 
   /// Given an imported library URI, this will determine to what Dart/JS module
   /// it belongs to.
   // TODO(jmesserly): we should replace this with another way of tracking
   // build units.
   final Func1<Source, String> libraryToModule;
 
   BuildUnit(this.name, this.libraryRoot, this.sources, this.libraryToModule);
 }
 
 /// The output of Dart->JS compilation.
 ///
 /// This contains the file contents of the JS module, as well as a list of
 /// Dart libraries that are contained in this module.
 class JSModuleFile {
   /// The name of this module.
   final String name;
 
   /// The list of messages (errors and warnings)
   final List<String> errors;
 
+  /// The AST that will be used to generate the [code] and [sourceMap] for this
+  /// module.
+  final JS.Program moduleTree;
+
+  /// The compiler options used to generate this module.
+  final CompilerOptions options;
+
+  /// The binary contents of the API summary file, including APIs from each of
+  /// the libraries in this module.
+  final List<int> summaryBytes;
+
+  JSModuleFile(
+      this.name, this.errors, this.options, this.moduleTree, this.summaryBytes);
+
+  JSModuleFile.invalid(this.name, this.errors, this.options)
+      : moduleTree = null,
+        summaryBytes = null;
+
+  /// True if this library was successfully compiled.
+  bool get isValid => moduleTree != null;
+
+  /// Gets the source code and source map for this JS module, given the
+  /// locations where the JS file and map file will be served from.
+  ///
+  /// Relative URLs will be used to point from the .js file to the .map file
+  //
+  // TODO(jmesserly): this should match our old logic, but I'm not sure we are
+  // correctly handling the pointer from the .js file to the .map file.
+  JSModuleCode getCode(ModuleFormat format, String jsUrl, String mapUrl) {
+    var opts = new JS.JavaScriptPrintingOptions(
+        emitTypes: options.closure,
+        allowKeywordsInProperties: true,
+        allowSingleLineIfStatements: true);
+    JS.SimpleJavaScriptPrintingContext printer;
+    SourceMapBuilder sourceMap;
+    if (options.sourceMap) {
+      var sourceMapContext = new SourceMapPrintingContext();
+      sourceMap = sourceMapContext.sourceMap;
+      printer = sourceMapContext;
+    } else {
+      printer = new JS.SimpleJavaScriptPrintingContext();
+    }
+
+    var tree = lowerModuleFormat(format, moduleTree);
+    tree.accept(
+        new JS.Printer(opts, printer, localNamer: new JS.TemporaryNamer(tree)));
+
+    if (options.sourceMap && options.sourceMapComment) {
+      printer.emit('\n//# sourceMappingURL=$mapUrl\n');
+    }
+
+    Map builtMap;
+    if (sourceMap != null) {
+      builtMap = placeSourceMap(sourceMap.build(jsUrl), mapUrl);
+    }
+    return new JSModuleCode(printer.getText(), builtMap);
+  }
+
+  /// Similar to [getCode] but immediately writes the resulting files.
+  ///
+  /// If [mapPath] is not supplied but [options.sourceMap] is set, mapPath
+  /// will default to [jsPath].map.
+  void writeCodeSync(ModuleFormat format, String jsPath, [String mapPath]) {
+    if (mapPath == null) mapPath = jsPath + '.map';
+    var code = getCode(format, jsPath, mapPath);
+    new File(jsPath).writeAsStringSync(code.code);
+    if (code.sourceMap != null) {
+      new File(mapPath).writeAsStringSync(JSON.encode(code.sourceMap));
+    }
+  }
+}
+
+/// The output of compiling a JavaScript module in a particular format.
+class JSModuleCode {
   /// The JavaScript code for this module.
   ///
   /// If a [sourceMap] is available, this will include the `sourceMappingURL`
   /// comment at end of the file.
   final String code;
 
   /// The JSON of the source map, if generated, otherwise `null`.
   ///
   /// The source paths will initially be absolute paths. They can be adjusted
   /// using [placeSourceMap].
   final Map sourceMap;
 
-  /// The binary contents of the API summary file, including APIs from each of
-  /// the [libraries] in this module.
-  final List<int> summaryBytes;
+  JSModuleCode(this.code, this.sourceMap);
+}
 
-  JSModuleFile(
-      this.name, this.errors, this.code, this.sourceMap, this.summaryBytes);
 
-  JSModuleFile.invalid(this.name, this.errors)
-      : code = null,
-        sourceMap = null,
-        summaryBytes = null;
+/// Adjusts the source paths in [sourceMap] to be relative to [sourceMapPath],
+/// and returns the new map.
+// TODO(jmesserly): find a new home for this.
+Map placeSourceMap(Map sourceMap, String sourceMapPath) {
+  var dir = path.dirname(sourceMapPath);
 
-  /// True if this library was successfully compiled.
-  bool get isValid => code != null;
-
-  /// Adjusts the source paths in [sourceMap] to be relative to [sourceMapPath],
-  /// and returns the new map.
-  ///
-  /// See also [writeSourceMap].
-  Map placeSourceMap(String sourceMapPath) {
-    var dir = path.dirname(sourceMapPath);
-
-    var map = new Map.from(this.sourceMap);
-    List list = new List.from(map['sources']);
-    map['sources'] = list;
-    for (int i = 0; i < list.length; i++) {
-      list[i] = path.relative(list[i], from: dir);
-    }
-    return map;
+  var map = new Map.from(sourceMap);
+  List list = new List.from(map['sources']);

[nweiz, 2016/08/24 23:33:26]

Nit: "var list"

[Jennifer Messerly, 2016/08/25 16:21:39]

yeah this method just moved from somewhere. Never would've written it this way
in new code :)

but fixed.

+  map['sources'] = list;
+  for (int i = 0; i < list.length; i++) {

[nweiz, 2016/08/24 23:33:26]

Nit: "var i"

[Jennifer Messerly, 2016/08/25 16:21:39]

Done.

+    list[i] = path.relative(list[i], from: dir);

[nweiz, 2016/08/24 23:33:26]

Source map locations are URIs, not local filesystem paths. You probably want:

    p.toUri(p.relative(p.fromUri(list[i]), from: dir))

[Jennifer Messerly, 2016/08/25 16:21:39]

Done. The reason it was working: it only needs to handle source maps emitted by
our compiler. But that said I'm fine making it more correct in the general case.

   }
+  return sourceMap;

[nweiz, 2016/08/24 23:33:26]

I really don't like returning *and* modifying a parameter. It's too easy to look
at the signature and assume that it's safe to pass something that shouldn't be
modified.

[vsm, 2016/08/25 12:19:24]

John: did you mean to return the new map (as per your method comment)?

[Jennifer Messerly, 2016/08/25 16:21:39]

good catch. yes it should be returning the copy. And I don't think we modify the
original map, but it does share structure for anything not mutated.

 }