From 3e9b532409644fd7548f1b7d143487d038f4879f Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Thu, 28 Jan 2016 13:58:12 -0800 Subject: [PATCH] fix(dart/transform): Handle edge cases in ReflectionRemover 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 --- .../reflection_remover/rewriter.dart | 47 ++++++++++++++++++- .../reflection_remover/all_tests.dart | 35 +++++++++++--- .../combinator_files/expected/index.dart | 17 +++++++ .../combinator_files/index.dart | 5 ++ .../expected/index.dart | 19 ++++++++ .../deferred_bootstrap_files/index.dart | 7 +++ 6 files changed, 121 insertions(+), 9 deletions(-) create mode 100644 modules_dart/transform/test/transform/reflection_remover/combinator_files/expected/index.dart create mode 100644 modules_dart/transform/test/transform/reflection_remover/combinator_files/index.dart create mode 100644 modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/expected/index.dart create mode 100644 modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/index.dart diff --git a/modules_dart/transform/lib/src/transform/reflection_remover/rewriter.dart b/modules_dart/transform/lib/src/transform/reflection_remover/rewriter.dart index 1133faa201..e22b14533b 100644 --- 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 { 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 { _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 index c8fca68bf9..018f28c82e 100644 --- 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 new file mode 100644 index 0000000000..447f783106 --- /dev/null +++ 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 new file mode 100644 index 0000000000..e51e8e8263 --- /dev/null +++ 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 new file mode 100644 index 0000000000..98723f8b14 --- /dev/null +++ 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 new file mode 100644 index 0000000000..94c8dff4cc --- /dev/null +++ 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); + }); +}