fix(dart/transform): Handle edge cases in ReflectionRemover

Tim Blasi · alexeagle · commit 3e9b53240964 · 2016-02-01T15:17:44.000-08:00
Handle some cases which would previously result in broken code. - Importing bootstrap.dart deferred - Using combinators when importing bootstrap.dart - Importing bootstrap.dart with a prefix Closes #6749
diff --git a/modules_dart/transform/lib/src/transform/reflection_remover/rewriter.dart b/modules_dart/transform/lib/src/transform/reflection_remover/rewriter.dart
@@ -173,8 +173,47 @@ class _RewriterVisitor extends Object with RecursiveAstVisitor<Object> {
     if (_rewriter._writeStaticInit) {
       // rewrite bootstrap import to its static version.
       buf.write(_rewriter._code.substring(_currentIndex, node.offset));
-      // TODO(yjbanov): handle import "..." show/hide ...
-      buf.write("import '$BOOTSTRAP_STATIC_URI';");
+      buf.write("import '$BOOTSTRAP_STATIC_URI'");
+
+      // The index of the last character we've processed.
+      var lastIdx = node.uri.end;
+
+      // Maintain the import prefix, if present.
+      if (node.prefix != null) {
+        buf.write(_rewriter._code.substring(lastIdx, node.prefix.end));
+        lastIdx = node.prefix.end;
+      }
+
+      // Handle combinators ("show" and "hide" on an "import" directive).
+      // 1. A combinator like "show $BOOTSTRAP_NAME" no longer makes sense, so
+      //    we need to rewrite it.
+      // 2. It's possible we'll need to call the setup method
+      //    (SETUP_METHOD_NAME), so make sure it is visible.
+      if (node.combinators != null) {
+        for (var combinator in node.combinators) {
+          buf.write(_rewriter._code
+              .substring(lastIdx, combinator.end)
+              .replaceAll(BOOTSTRAP_NAME, BOOTSTRAP_STATIC_NAME));
+          lastIdx = combinator.end;
+          if (combinator is ShowCombinator) {
+            buf.write(', $SETUP_METHOD_NAME');
+          } else if (combinator is HideCombinator) {
+            // Ensure the user is not explicitly hiding SETUP_METHOD_NAME.
+            // I don't know why anyone would do this, but it would result in
+            // some confusing behavior, so throw an explicit error.
+            combinator.hiddenNames.forEach((id) {
+              if (id.toString() == SETUP_METHOD_NAME) {
+                throw new FormatException(
+                    'Import statement explicitly hides initialization function '
+                    '$SETUP_METHOD_NAME. Please do not do this: "$node"');
+              }
+            });
+          }
+        }
+      }
+
+      // Write anything after the combinators.
+      buf.write(_rewriter._code.substring(lastIdx, node.end));
       _hasStaticBootstrapImport = true;
     } else {
       // leave it as is
@@ -198,6 +237,10 @@ class _RewriterVisitor extends Object with RecursiveAstVisitor<Object> {
           _setupAdded ? '' : ', () { ${_getStaticReflectorInitBlock()} }';
 
       // rewrite `bootstrap(...)` to `bootstrapStatic(...)`
+      if (node.target != null && node.target is SimpleIdentifier) {
+        // `bootstrap` imported with a prefix, maintain this.
+        buf.write('${node.target}.');
+      }
       buf.write('$BOOTSTRAP_STATIC_NAME(${args[0]}');
       if (numArgs == 1) {
         // bootstrap args are positional, so before we pass reflectorInit code
diff --git a/modules_dart/transform/test/transform/reflection_remover/all_tests.dart b/modules_dart/transform/test/transform/reflection_remover/all_tests.dart
@@ -12,7 +12,10 @@ import 'package:angular2/src/transform/reflection_remover/rewriter.dart';
 
 import '../common/read_file.dart';
 import 'bootstrap_files/expected/index.dart' as bootstrap_expected;
+import 'combinator_files/expected/index.dart' as combinator_expected;
 import 'debug_mirrors_files/expected/index.dart' as debug_mirrors;
+import 'deferred_bootstrap_files/expected/index.dart'
+    as deferred_bootstrap_expected;
 import 'function_annotation_files/expected/index.dart'
     as func_annotation_expected;
 import 'log_mirrors_files/expected/index.dart' as log_mirrors;
@@ -64,13 +67,31 @@ void allTests() {
     expect(output).toEqual(log_mirrors.code);
   });
 
-  it('should rewrite bootstrap.', () {
-    final bootstrapCode =
-        readFile('reflection_remover/bootstrap_files/index.dart')
-            .replaceAll('\r\n', '\n');
-    var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
-        writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
-    expect(output).toEqual(bootstrap_expected.code);
+  describe('`bootstrap` import and call', () {
+    it('should be rewritten to `bootstrapStatic`.', () {
+      final bootstrapCode =
+          readFile('reflection_remover/bootstrap_files/index.dart')
+              .replaceAll('\r\n', '\n');
+      var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
+          writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
+      expect(output).toEqual(bootstrap_expected.code);
+    });
+
+    it('should be rewritten correctly when deferred.', () {
+      final bootstrapCode =
+          readFile('reflection_remover/deferred_bootstrap_files/index.dart');
+      var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
+          writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
+      expect(output).toEqual(deferred_bootstrap_expected.code);
+    });
+
+    it('should maintain any combinators.', () {
+      final bootstrapCode =
+          readFile('reflection_remover/combinator_files/index.dart');
+      var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
+          writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
+      expect(output).toEqual(combinator_expected.code);
+    });
   });
 
   describe('AngularEntrypoint annotation', () {
diff --git a/modules_dart/transform/test/transform/reflection_remover/combinator_files/expected/index.dart b/modules_dart/transform/test/transform/reflection_remover/combinator_files/expected/index.dart
@@ -0,0 +1,17 @@
+library angular2.test.transform.reflection_remover.combinator_files;
+
+// This file is intentionally formatted as a string to avoid having the
+// automatic transformer prettify it.
+//
+// This file represents transformed user code. Because this code will be
+// linked to output by a source map, we cannot change line numbers from the
+// original code and we therefore add our generated code on the same line as
+// those we are removing.
+
+var code = """
+import 'package:angular2/bootstrap_static.dart' show bootstrapStatic, initReflector;import 'index.ng_deps.dart' as ngStaticInit;
+
+void main() {
+  bootstrapStatic(MyComponent, null, () { ngStaticInit.initReflector(); });
+}
+""";
diff --git a/modules_dart/transform/test/transform/reflection_remover/combinator_files/index.dart b/modules_dart/transform/test/transform/reflection_remover/combinator_files/index.dart
@@ -0,0 +1,5 @@
+import 'package:angular2/bootstrap.dart' show bootstrap;
+
+void main() {
+  bootstrap(MyComponent);
+}
diff --git a/modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/expected/index.dart b/modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/expected/index.dart
@@ -0,0 +1,19 @@
+library angular2.test.transform.reflection_remover.deferred_bootstrap_files;
+
+// This file is intentionally formatted as a string to avoid having the
+// automatic transformer prettify it.
+//
+// This file represents transformed user code. Because this code will be
+// linked to output by a source map, we cannot change line numbers from the
+// original code and we therefore add our generated code on the same line as
+// those we are removing.
+
+var code = """
+import 'package:angular2/bootstrap_static.dart' deferred as ng;import 'index.ng_deps.dart' as ngStaticInit;
+
+void main() {
+  ng.loadLibrary().then((_) {
+    ng.bootstrapStatic(MyComponent, null, () { ngStaticInit.initReflector(); });
+  });
+}
+""";
diff --git a/modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/index.dart b/modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/index.dart
@@ -0,0 +1,7 @@
+import 'package:angular2/bootstrap.dart' deferred as ng;
+
+void main() {
+  ng.loadLibrary().then((_) {
+    ng.bootstrap(MyComponent);
+  });
+}
