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

lib/src/compiler/module_builder.dart

Index: lib/src/compiler/module_builder.dart
diff --git a/lib/src/compiler/module_builder.dart b/lib/src/compiler/module_builder.dart
index 6e72217f6d65df982eb17ce7384ef565e94f6e54..0f2dbdd9de8c8558ce18604cf75b3f4959d0f88f 100644
--- a/lib/src/compiler/module_builder.dart
+++ b/lib/src/compiler/module_builder.dart
@@ -2,11 +2,91 @@
 // 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 'package:args/args.dart' show ArgParser, ArgResults;
 import 'package:path/path.dart' as path;
 
 import '../js_ast/js_ast.dart';
 import 'js_names.dart';
 
+/// The module format to emit.
+enum ModuleFormat {
+  /// EcmaScript 6 module using import and export.

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

Nit: "ECMAScript" (also below)

My editor blood runs thick.

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

done :)  fixed all occurrences under dev_compiler/lib/

(there are some we pulled in that other people wrote in the Dart language tests,
I didn't fix those)

+  es6,
+
+  /// CommonJS module (used in Node.js)
+  common,
+
+  /// Asynchronous Module Definition (AMD, used in browsers)
+  amd,
+
+  /// Dart Dev Compiler's legacy format (deprecated).
+  legacy
+}
+
+/// Parses a string into a [ModuleFormat].
+ModuleFormat parseModuleFormat(String s) => {
+      'es6': ModuleFormat.es6,
+      'common': ModuleFormat.common,
+      'amd': ModuleFormat.amd,
+      // Deprecated:
+      'node': ModuleFormat.common,
+      'legacy': ModuleFormat.legacy
+    }[s];
+
+/// Parse the module format option added by [addModuleFormatOptions].
+List<ModuleFormat> parseModuleFormatOption(ArgResults argResults) {
+  var format = argResults['modules'];
+  if (format is String) {
+    return [parseModuleFormat(format)];
+  }
+  return (format as List<String>).map(parseModuleFormat).toList();
+}
+
+/// Adds an option to the [argParser] for choosing the module format, optionally
+/// [allowMultiple] formats to be specified, with each emitted into a separate
+/// file.
+void addModuleFormatOptions(ArgParser argParser, {bool allowMultiple: false}) {
+  argParser.addOption('modules',
+      help: 'module pattern to emit',
+      allowed: [
+        'es6',
+        'common',
+        'amd',
+        'legacy', // deprecated
+        'node', // renamed to commonjs
+        'all' // to emit all flavors for the SDK
+      ],
+      allowedHelp: {
+        'es6': 'EcmaScript 6 modules',
+        'common': 'CommonJS/Node.js modules',
+        'amd': 'AMD/RequireJS modules'
+      },
+      allowMultiple: allowMultiple,
+      defaultsTo: 'amd');
+}
+
+/// Transform an ES6 [module] into a given module [format].

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

Nit: "Transforms"

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

Done.

+///
+/// If the format is [ModuleFormat.es6] this will return [module] unchanged.
+///
+/// Because JS ASTs are immutable the resulting module will share as much
+/// structure as possible with the original. The transformation is a shallow one
+/// that affects the top-level module items, especially [ImportDeclaration]s and
+/// [ExportDeclaration]s.
+Program lowerModuleFormat(ModuleFormat format, Program module) {

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

I think "lower" is confusing here. Maybe "change" would be clearer?

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

hmmm, yeah that's tricky. "lower" is a bit of compiler jargon[1][2][3]. On the
one hand, I think that sort of thing is appropriate in a compiler for internal
APIs. But on the other, I'm definitely sympathetic to choosing plain language
wherever possible.

My problem with "change" is we can't change in all directions, if you lower to
AMD you can't go back to ES6. That's why "lower" suggests a direction: that
we're transforming from a "higher level" form to a "lower level" form, and
intuitively it's easy to go from "high level" to "low level" but not the
reverse.

Perhaps "transform" as the documentation comment uses ... while it's still not
suggesting a direction, it suggests we made a "dramatic change in the form"
which may remove the intuition that one can easily change back :)

[1]: https://ericlippert.com/2014/04/28/lowering-in-language-design-part-one/
[2]: http://www.cs.cornell.edu/courses/cs412/2003sp/lectures/lec18.pdf
[3]: https://gcc.gnu.org/onlinedocs/gccint/GENERIC.html

+  switch (format) {
+    case ModuleFormat.legacy:
+      return new LegacyModuleBuilder().build(module);
+    case ModuleFormat.common:
+      return new CommonJSModuleBuilder().build(module);
+    case ModuleFormat.amd:
+      return new AmdModuleBuilder().build(module);
+    case ModuleFormat.es6:
+      return module;
+  }
+  return null; // unreachable. suppresses a bogus analyzer message
+}
+
 /// Base class for compiling ES6 modules into various ES5 module patterns.
 ///
 /// This is a helper class for utilities and state that is shared by several
@@ -61,7 +141,7 @@ class LegacyModuleBuilder extends _ModuleBuilder {
     var importStatements = <Statement>[];
     for (var import in imports) {
       importNames.add(import.from);
-      // TODO(jmesserly): we could use destructuring once Atom supports it.
+      // TODO(jmesserly): we could use destructuring here.
       var moduleVar =
           new TemporaryId(pathToJSIdentifier(import.from.valueWithoutQuotes));
       parameters.add(moduleVar);
@@ -105,8 +185,8 @@ class LegacyModuleBuilder extends _ModuleBuilder {
   }
 }
 
-/// Generates node modules.
-class NodeModuleBuilder extends _ModuleBuilder {
+/// Generates CommonJS modules (used by Node.js).
+class CommonJSModuleBuilder extends _ModuleBuilder {
   Program build(Program module) {
     var importStatements = <Statement>[];
 
@@ -114,7 +194,7 @@ class NodeModuleBuilder extends _ModuleBuilder {
     visitProgram(module);
 
     for (var import in imports) {
-      // TODO(jmesserly): we could use destructuring once Atom supports it.
+      // TODO(jmesserly): we could use destructuring here.
       var moduleVar =
           new TemporaryId(pathToJSIdentifier(import.from.valueWithoutQuotes));
       importStatements
@@ -122,7 +202,8 @@ class NodeModuleBuilder extends _ModuleBuilder {
 
       // TODO(jmesserly): optimize for the common case of a single import.
       for (var importName in import.namedImports) {
-        assert(!importName.isStar); // import * not supported yet.
+        // import * is not emitted by the compiler, so we don't support it here.
+        assert(!importName.isStar);
         var asName = importName.asName ?? importName.name;
         importStatements.add(js.statement(
             'const # = #.#', [asName, moduleVar, importName.name.name]));
@@ -135,7 +216,8 @@ class NodeModuleBuilder extends _ModuleBuilder {
       statements.add(js.comment('Exports:'));
       for (var export in exports) {
         var names = export.exportedNames;
-        assert(names != null); // export * not supported in legacy modules.
+        // export * is not emitted by the compiler, so we don't handle it here.
+        assert(names != null);
         for (var name in names) {
           statements
               .add(js.statement('#.# = #;', [exportsVar, name.name, name]));
@@ -152,6 +234,56 @@ class NodeModuleBuilder extends _ModuleBuilder {
   }
 }
 
+/// Generates AMD modules (used in browsers with RequireJS).
+class AmdModuleBuilder extends _ModuleBuilder {
+  Program build(Program module) {
+    var importStatements = <Statement>[];
+
+    // Collect imports/exports/statements.
+    visitProgram(module);
+
+    var dependencies = <LiteralString>[];
+    var fnParams = <Parameter>[];
+    for (var import in imports) {
+      // TODO(jmesserly): we could use destructuring once Atom supports it.
+      var moduleVar =
+          new TemporaryId(pathToJSIdentifier(import.from.valueWithoutQuotes));
+      fnParams.add(moduleVar);
+      dependencies.add(import.from);
+
+      // TODO(jmesserly): optimize for the common case of a single import.
+      for (var importName in import.namedImports) {
+        // import * is not emitted by the compiler, so we don't handle it here.
+        assert(!importName.isStar);
+        var asName = importName.asName ?? importName.name;
+        importStatements.add(js.statement(
+            'const # = #.#', [asName, moduleVar, importName.name.name]));
+      }
+    }
+    statements.insertAll(0, importStatements);
+
+    if (exports.isNotEmpty) {
+      var exportedProps = <Property>[];
+      for (var export in exports) {
+        var names = export.exportedNames;
+        // export * is not emitted by the compiler, so we don't handle it here.
+        assert(names != null);
+        for (var name in names) {
+          exportedProps.add(new Property(js.string(name.name), name));
+        }
+      }
+      statements.add(js.comment('Exports:'));
+      statements.add(
+          new Return(new ObjectInitializer(exportedProps, multiline: true)));
+    }
+
+    var block = js.statement("define(#, function(#) { 'use strict'; #; });",
+        [new ArrayInitializer(dependencies), fnParams, statements]);
+
+    return new Program([block]);
+  }
+}
+
 /// Escape [name] to make it into a valid identifier.
 String pathToJSIdentifier(String name) {
   return toJSIdentifier(path.basenameWithoutExtension(name));