Skip to content

Commit

Permalink
fix(dart/transform): Handle edge cases in ReflectionRemover
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Tim Blasi authored and alexeagle committed Feb 1, 2016
1 parent c5aa6d1 commit 3e9b532
Show file tree
Hide file tree
Showing 6 changed files with 121 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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', () {
Expand Down
Original file line number Diff line number Diff line change
@@ -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(); });
}
""";
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
import 'package:angular2/bootstrap.dart' show bootstrap;

void main() {
bootstrap(MyComponent);
}
Original file line number Diff line number Diff line change
@@ -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(); });
});
}
""";
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
import 'package:angular2/bootstrap.dart' deferred as ng;

void main() {
ng.loadLibrary().then((_) {
ng.bootstrap(MyComponent);
});
}

0 comments on commit 3e9b532

Please sign in to comment.