Skip to content

Commit 3e9b532

Browse files
Tim Blasialexeagle
Tim Blasi
authored and
alexeagle
committed
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
1 parent c5aa6d1 commit 3e9b532

6 files changed

Lines changed: 121 additions & 9 deletions

File tree

‎modules_dart/transform/lib/src/transform/reflection_remover/rewriter.dart‎

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,47 @@ class _RewriterVisitor extends Object with RecursiveAstVisitor<Object> {
173173
if (_rewriter._writeStaticInit) {
174174
// rewrite bootstrap import to its static version.
175175
buf.write(_rewriter._code.substring(_currentIndex, node.offset));
176-
// TODO(yjbanov): handle import "..." show/hide ...
177-
buf.write("import '$BOOTSTRAP_STATIC_URI';");
176+
buf.write("import '$BOOTSTRAP_STATIC_URI'");
177+
178+
// The index of the last character we've processed.
179+
var lastIdx = node.uri.end;
180+
181+
// Maintain the import prefix, if present.
182+
if (node.prefix != null) {
183+
buf.write(_rewriter._code.substring(lastIdx, node.prefix.end));
184+
lastIdx = node.prefix.end;
185+
}
186+
187+
// Handle combinators ("show" and "hide" on an "import" directive).
188+
// 1. A combinator like "show $BOOTSTRAP_NAME" no longer makes sense, so
189+
// we need to rewrite it.
190+
// 2. It's possible we'll need to call the setup method
191+
// (SETUP_METHOD_NAME), so make sure it is visible.
192+
if (node.combinators != null) {
193+
for (var combinator in node.combinators) {
194+
buf.write(_rewriter._code
195+
.substring(lastIdx, combinator.end)
196+
.replaceAll(BOOTSTRAP_NAME, BOOTSTRAP_STATIC_NAME));
197+
lastIdx = combinator.end;
198+
if (combinator is ShowCombinator) {
199+
buf.write(', $SETUP_METHOD_NAME');
200+
} else if (combinator is HideCombinator) {
201+
// Ensure the user is not explicitly hiding SETUP_METHOD_NAME.
202+
// I don't know why anyone would do this, but it would result in
203+
// some confusing behavior, so throw an explicit error.
204+
combinator.hiddenNames.forEach((id) {
205+
if (id.toString() == SETUP_METHOD_NAME) {
206+
throw new FormatException(
207+
'Import statement explicitly hides initialization function '
208+
'$SETUP_METHOD_NAME. Please do not do this: "$node"');
209+
}
210+
});
211+
}
212+
}
213+
}
214+
215+
// Write anything after the combinators.
216+
buf.write(_rewriter._code.substring(lastIdx, node.end));
178217
_hasStaticBootstrapImport = true;
179218
} else {
180219
// leave it as is
@@ -198,6 +237,10 @@ class _RewriterVisitor extends Object with RecursiveAstVisitor<Object> {
198237
_setupAdded ? '' : ', () { ${_getStaticReflectorInitBlock()} }';
199238

200239
// rewrite `bootstrap(...)` to `bootstrapStatic(...)`
240+
if (node.target != null && node.target is SimpleIdentifier) {
241+
// `bootstrap` imported with a prefix, maintain this.
242+
buf.write('${node.target}.');
243+
}
201244
buf.write('$BOOTSTRAP_STATIC_NAME(${args[0]}');
202245
if (numArgs == 1) {
203246
// bootstrap args are positional, so before we pass reflectorInit code

‎modules_dart/transform/test/transform/reflection_remover/all_tests.dart‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,10 @@ import 'package:angular2/src/transform/reflection_remover/rewriter.dart';
1212

1313
import '../common/read_file.dart';
1414
import 'bootstrap_files/expected/index.dart' as bootstrap_expected;
15+
import 'combinator_files/expected/index.dart' as combinator_expected;
1516
import 'debug_mirrors_files/expected/index.dart' as debug_mirrors;
17+
import 'deferred_bootstrap_files/expected/index.dart'
18+
as deferred_bootstrap_expected;
1619
import 'function_annotation_files/expected/index.dart'
1720
as func_annotation_expected;
1821
import 'log_mirrors_files/expected/index.dart' as log_mirrors;
@@ -64,13 +67,31 @@ void allTests() {
6467
expect(output).toEqual(log_mirrors.code);
6568
});
6669

67-
it('should rewrite bootstrap.', () {
68-
final bootstrapCode =
69-
readFile('reflection_remover/bootstrap_files/index.dart')
70-
.replaceAll('\r\n', '\n');
71-
var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
72-
writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
73-
expect(output).toEqual(bootstrap_expected.code);
70+
describe('`bootstrap` import and call', () {
71+
it('should be rewritten to `bootstrapStatic`.', () {
72+
final bootstrapCode =
73+
readFile('reflection_remover/bootstrap_files/index.dart')
74+
.replaceAll('\r\n', '\n');
75+
var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
76+
writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
77+
expect(output).toEqual(bootstrap_expected.code);
78+
});
79+
80+
it('should be rewritten correctly when deferred.', () {
81+
final bootstrapCode =
82+
readFile('reflection_remover/deferred_bootstrap_files/index.dart');
83+
var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
84+
writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
85+
expect(output).toEqual(deferred_bootstrap_expected.code);
86+
});
87+
88+
it('should maintain any combinators.', () {
89+
final bootstrapCode =
90+
readFile('reflection_remover/combinator_files/index.dart');
91+
var output = new Rewriter(bootstrapCode, codegen, entrypointMatcher,
92+
writeStaticInit: true).rewrite(parseCompilationUnit(bootstrapCode));
93+
expect(output).toEqual(combinator_expected.code);
94+
});
7495
});
7596

7697
describe('AngularEntrypoint annotation', () {

‎modules_dart/transform/test/transform/reflection_remover/combinator_files/expected/index.dart‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
library angular2.test.transform.reflection_remover.combinator_files;
2+
3+
// This file is intentionally formatted as a string to avoid having the
4+
// automatic transformer prettify it.
5+
//
6+
// This file represents transformed user code. Because this code will be
7+
// linked to output by a source map, we cannot change line numbers from the
8+
// original code and we therefore add our generated code on the same line as
9+
// those we are removing.
10+
11+
var code = """
12+
import 'package:angular2/bootstrap_static.dart' show bootstrapStatic, initReflector;import 'index.ng_deps.dart' as ngStaticInit;
13+
14+
void main() {
15+
bootstrapStatic(MyComponent, null, () { ngStaticInit.initReflector(); });
16+
}
17+
""";

‎modules_dart/transform/test/transform/reflection_remover/combinator_files/index.dart‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
import 'package:angular2/bootstrap.dart' show bootstrap;
2+
3+
void main() {
4+
bootstrap(MyComponent);
5+
}

‎modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/expected/index.dart‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
library angular2.test.transform.reflection_remover.deferred_bootstrap_files;
2+
3+
// This file is intentionally formatted as a string to avoid having the
4+
// automatic transformer prettify it.
5+
//
6+
// This file represents transformed user code. Because this code will be
7+
// linked to output by a source map, we cannot change line numbers from the
8+
// original code and we therefore add our generated code on the same line as
9+
// those we are removing.
10+
11+
var code = """
12+
import 'package:angular2/bootstrap_static.dart' deferred as ng;import 'index.ng_deps.dart' as ngStaticInit;
13+
14+
void main() {
15+
ng.loadLibrary().then((_) {
16+
ng.bootstrapStatic(MyComponent, null, () { ngStaticInit.initReflector(); });
17+
});
18+
}
19+
""";

‎modules_dart/transform/test/transform/reflection_remover/deferred_bootstrap_files/index.dart‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
import 'package:angular2/bootstrap.dart' deferred as ng;
2+
3+
void main() {
4+
ng.loadLibrary().then((_) {
5+
ng.bootstrap(MyComponent);
6+
});
7+
}

0 commit comments

Comments
 (0)