refactor(dart/transform): Allow inlining only using `inline_views`

Remove the `inliner_for_test` standalone transformer and make inlining
available via the `inline_views` parameter to the main angular2
transformer.

modules_dart/transform/lib/src/transform/common/options.dart

@@ -12,7 +12,6 @@ const FORMAT_CODE_PARAM = 'format_code';
 const GENERATE_CHANGE_DETECTORS_PARAM = 'generate_change_detectors';
 const REFLECT_PROPERTIES_AS_ATTRIBUTES = 'reflectPropertiesAsAttributes';
 const INIT_REFLECTOR_PARAM = 'init_reflector';
-// TODO(kegluenq): Remove this after 30 Oct (i/4433).
 const INLINE_VIEWS_PARAM = 'inline_views';
 const MIRROR_MODE_PARAM = 'mirror_mode';
 const OPTIMIZATION_PHASES_PARAM = 'optimization_phases';
@@ -43,6 +42,13 @@ class TransformerOptions {
   /// invalidate the source maps generated by `dart2js` and/or other tools.
   final bool formatCode;
 
+  /// Whether to inline views.
+  /// If this is `true`, the transformer will *only* make a single pass over the
+  /// input files and inline `templateUrl` and `styleUrls` values.
+  /// This is undocumented, for testing purposes only, and may change or break
+  /// at any time.
+  final bool inlineViews;
+
   TransformerOptions._internal(
       this.entryPoints,
       this.entryPointGlobs,
@@ -51,6 +57,7 @@ class TransformerOptions {
       this.initReflector,
       this.annotationMatcher,
       {this.reflectPropertiesAsAttributes,
+      this.inlineViews,
       this.formatCode});
 
   factory TransformerOptions(List<String> entryPoints,
@@ -58,7 +65,7 @@ class TransformerOptions {
       MirrorMode mirrorMode: MirrorMode.none,
       bool initReflector: true,
       List<ClassDescriptor> customAnnotationDescriptors: const [],
-      bool inlineViews: true,
+      bool inlineViews: false,
       bool reflectPropertiesAsAttributes: true,
       bool formatCode: false}) {
     var annotationMatcher = new AnnotationMatcher()
@@ -69,6 +76,7 @@ class TransformerOptions {
     return new TransformerOptions._internal(entryPoints, entryPointGlobs,
         modeName, mirrorMode, initReflector, annotationMatcher,
         reflectPropertiesAsAttributes: reflectPropertiesAsAttributes,
+        inlineViews: inlineViews,
         formatCode: formatCode);
   }
 }

modules_dart/transform/lib/src/transform/common/options_reader.dart

@@ -34,6 +34,7 @@ TransformerOptions parseBarbackSettings(BarbackSettings settings) {
       initReflector: initReflector,
       customAnnotationDescriptors: _readCustomAnnotations(config),
       reflectPropertiesAsAttributes: reflectPropertiesAsAttributes,
+      inlineViews: _readBool(config, INLINE_VIEWS_PARAM, defaultValue: false),
       formatCode: formatCode);
 }
 
@@ -116,10 +117,4 @@ void _warnDeprecated(Map config) {
     print('${GENERATE_CHANGE_DETECTORS_PARAM} is no longer necessary for '
         'Angular 2 apps. Please remove it from your pubspec.');
   }
-  if (config.containsKey(INLINE_VIEWS_PARAM)) {
-    print('Parameter "${INLINE_VIEWS_PARAM}" no longer has any effect on the '
-        'Angular2 transformer. Inlining of views is only needed for tests that '
-        'manipulate component metadata. For this purpose, use transformer '
-        'angular2/src/transform/inliner_for_test.');
-  }
 }

modules_dart/transform/lib/src/transform/inliner_for_test.dart

@@ -1,159 +0,0 @@
-library angular2.src.transform.transform_for_test;
-
-import 'dart:async';
-
-import 'package:analyzer/analyzer.dart';
-import 'package:analyzer/src/generated/ast.dart';
-import 'package:angular2/src/core/compiler/xhr.dart' show XHR;
-import 'package:angular2/src/transform/common/asset_reader.dart';
-import 'package:barback/barback.dart';
-import 'package:dart_style/dart_style.dart';
-
-import 'common/asset_reader.dart';
-import 'common/async_string_writer.dart';
-import 'common/logging.dart';
-import 'common/options_reader.dart';
-import 'common/url_resolver.dart';
-import 'common/xhr_impl.dart';
-import 'directive_processor/inliner.dart';
-
-/// Processes .dart files and inlines `templateUrl` and styleUrls` values.
-class InlinerForTest extends Transformer implements DeclaringTransformer {
-  final DartFormatter _formatter;
-
-  InlinerForTest({bool formatCode: false})
-      : _formatter = formatCode ? new DartFormatter() : null;
-
-  @override
-  bool isPrimary(AssetId id) => id.extension.endsWith('dart');
-
-  @override
-  declareOutputs(DeclaringTransform transform) {
-    transform.consumePrimary();
-    transform.declareOutput(transform.primaryId);
-  }
-
-  @override
-  Future apply(Transform transform) async {
-    return initZoned(transform, () async {
-      var primaryId = transform.primaryInput.id;
-      transform.consumePrimary();
-      var inlinedCode =
-          await inline(new AssetReader.fromTransform(transform), primaryId);
-      if (inlinedCode == null || inlinedCode.isEmpty) {
-        transform.addOutput(transform.primaryInput);
-      } else {
-        if (_formatter != null) {
-          inlinedCode = _formatter.format(inlinedCode);
-        }
-        transform.addOutput(new Asset.fromString(primaryId, inlinedCode));
-      }
-    });
-  }
-
-  factory InlinerForTest.asPlugin(BarbackSettings settings) {
-    return new InlinerForTest(
-        formatCode: parseBarbackSettings(settings).formatCode);
-  }
-}
-
-Future<String> inline(AssetReader reader, AssetId assetId) async {
-  var codeWithParts = await inlineParts(reader, assetId);
-  if (codeWithParts == null) return null;
-  var parsedCode =
-      parseCompilationUnit(codeWithParts, name: '${assetId.path} and parts');
-  var writer = new AsyncStringWriter();
-  var visitor = new _ViewPropInliner(assetId, writer, new XhrImpl(reader));
-  parsedCode.accept(visitor);
-  return writer.asyncToString();
-}
-
-final _urlResolver = const TransformerUrlResolver();
-
-class _ViewPropInliner extends ToSourceVisitor {
-  final Uri _baseUri;
-  final AsyncStringWriter _writer;
-  final XHR _xhr;
-
-  _ViewPropInliner._(this._baseUri, AsyncStringWriter writer, this._xhr)
-      : _writer = writer,
-        super(writer);
-
-  factory _ViewPropInliner(AssetId assetId, AsyncStringWriter writer, XHR xhr) {
-    var baseUri =
-        new Uri(scheme: 'asset', path: '${assetId.package}/${assetId.path}');
-    return new _ViewPropInliner._(baseUri, writer, xhr);
-  }
-
-  @override
-  Object visitNamedExpression(NamedExpression node) {
-    if (node.name is! Label || node.name.label is! SimpleIdentifier) {
-      throw new FormatException(
-          'Angular 2 currently only supports simple identifiers in directives.',
-          '$node' /* source */);
-    }
-    var keyString = '${node.name.label}';
-    switch (keyString) {
-      case 'templateUrl':
-        _populateTemplateUrl(node.expression);
-        return null;
-      case 'styleUrls':
-        _populateStyleUrls(node.expression);
-        return null;
-    }
-    return super.visitNamedExpression(node);
-  }
-
-  void _populateStyleUrls(Expression value) {
-    var urls = _dumbEval(value);
-    if (urls is! List) {
-      logger.warning('styleUrls is not a List of Strings ($value)');
-      return;
-    }
-    _writer.print('styles: const [');
-    for (var url in urls) {
-      if (url is String) {
-        _writer.print("r'''");
-        _writer.asyncPrint(_readOrEmptyString(url));
-        _writer.print("''', ");
-      } else {
-        logger.warning('style url is not a String (${url})');
-      }
-    }
-    _writer.println(']');
-  }
-
-  void _populateTemplateUrl(Expression value) {
-    var url = _dumbEval(value);
-    if (url is! String) {
-      logger.warning('template url is not a String ($value)');
-      return;
-    }
-    _writer.print("template: r'''");
-    _writer.asyncPrint(_readOrEmptyString(url));
-    _writer.println("'''");
-  }
-
-  /// Attempts to read the content from [url]. If [url] is relative, uses
-  /// [_baseUri] as resolution base.
-  Future<String> _readOrEmptyString(String url) async {
-    final resolvedUri = _urlResolver.resolve(_baseUri.toString(), url);
-
-    return _xhr.get(resolvedUri).catchError((_) {
-      logger.error('$_baseUri: could not read $url');
-      return '';
-    });
-  }
-}
-
-final _constantEvaluator = new ConstantEvaluator();
-
-dynamic _dumbEval(Expression expr) {
-  var val;
-  if (expr is SimpleStringLiteral) {
-    val = stringLiteralToString(expr);
-  } else {
-    val = expr.accept(_constantEvaluator);
-  }
-  return val != ConstantEvaluator.NOT_A_CONSTANT ? val : null;
-}

modules_dart/transform/lib/src/transform/inliner_for_test/transformer.dart

@@ -0,0 +1,206 @@
+library angular2.src.transform.inliner_for_test.transformer;
+
+import 'dart:async';
+
+import 'package:analyzer/analyzer.dart';
+import 'package:analyzer/src/generated/ast.dart';
+import 'package:angular2/src/core/compiler/xhr.dart' show XHR;
+import 'package:angular2/src/transform/common/annotation_matcher.dart';
+import 'package:angular2/src/transform/common/asset_reader.dart';
+import 'package:angular2/src/transform/common/async_string_writer.dart';
+import 'package:angular2/src/transform/common/logging.dart';
+import 'package:angular2/src/transform/common/options.dart';
+import 'package:angular2/src/transform/common/url_resolver.dart';
+import 'package:angular2/src/transform/common/xhr_impl.dart';
+import 'package:angular2/src/transform/directive_processor/inliner.dart';
+import 'package:barback/barback.dart';
+import 'package:dart_style/dart_style.dart';
+
+/// Processes .dart files and inlines `templateUrl` and styleUrls` values.
+class InlinerForTest extends Transformer implements DeclaringTransformer {
+  final DartFormatter _formatter;
+  final AnnotationMatcher _annotationMatcher;
+
+  InlinerForTest(TransformerOptions options)
+      : _formatter = options.formatCode ? new DartFormatter() : null,
+        _annotationMatcher = options.annotationMatcher;
+
+  @override
+  bool isPrimary(AssetId id) => id.extension.endsWith('dart');
+
+  @override
+  declareOutputs(DeclaringTransform transform) {
+    transform.consumePrimary();
+    transform.declareOutput(transform.primaryId);
+  }
+
+  @override
+  Future apply(Transform transform) async {
+    return initZoned(transform, () async {
+      var primaryId = transform.primaryInput.id;
+      transform.consumePrimary();
+      var inlinedCode = await inline(new AssetReader.fromTransform(transform),
+          primaryId, _annotationMatcher);
+      if (inlinedCode == null || inlinedCode.isEmpty) {
+        transform.addOutput(transform.primaryInput);
+      } else {
+        if (_formatter != null) {
+          inlinedCode = _formatter.format(inlinedCode);
+        }
+        transform.addOutput(new Asset.fromString(primaryId, inlinedCode));
+      }
+    });
+  }
+}
+
+/// Reads the code at `assetId`, inlining values where possible.
+/// Returns the code at `assetId` with the following modifications:
+/// - `part` Directives are inlined
+/// - `templateUrl` values are inlined as `template` values.
+/// - `styleUrls` values are inlined as `styles` values.
+///
+/// If this does not inline any `templateUrl` or `styleUrls` values, it will
+/// return `null` to signal that no modifications to the input code were
+/// necessary.
+Future<String> inline(AssetReader reader, AssetId assetId,
+    AnnotationMatcher annotationMatcher) async {
+  var codeWithParts = await inlineParts(reader, assetId);
+  if (codeWithParts == null) return null;
+  var parsedCode =
+      parseCompilationUnit(codeWithParts, name: '${assetId.path} and parts');
+  var writer = new AsyncStringWriter();
+  var visitor = new _ViewPropInliner(
+      assetId, codeWithParts, writer, new XhrImpl(reader), annotationMatcher);
+  parsedCode.accept(visitor);
+  return visitor.modifiedSource ? writer.asyncToString() : null;
+}
+
+final _urlResolver = const TransformerUrlResolver();
+
+class _ViewPropInliner extends RecursiveAstVisitor<Object> {
+  final AssetId _assetId;
+
+  /// The code we are operating on.
+  final String _code;
+
+  /// The asset uri for the code we are operating on.
+  final Uri _baseUri;
+  final AsyncStringWriter _writer;
+  final XHR _xhr;
+  final AnnotationMatcher _annotationMatcher;
+
+  /// The final index of the last substring we wrote.
+  int _lastIndex = 0;
+
+  /// Whether we are currently inlining.
+  bool _isInlining = false;
+
+  /// Whether this visitor actually inlined any properties.
+  bool get modifiedSource => _lastIndex > 0;
+
+  _ViewPropInliner(AssetId assetId, this._code, AsyncStringWriter writer,
+      this._xhr, this._annotationMatcher)
+      : _assetId = assetId,
+        _baseUri = Uri.parse(toAssetUri(assetId)),
+        _writer = writer,
+        super();
+
+  @override
+  Object visitCompilationUnit(CompilationUnit node) {
+    final retVal = super.visitCompilationUnit(node);
+    if (modifiedSource) {
+      _writer.print(_code.substring(_lastIndex));
+    }
+    return retVal;
+  }
+
+  @override
+  Object visitAnnotation(Annotation node) {
+    var wasInlining = _isInlining;
+    _isInlining = _annotationMatcher.isView(node, _assetId) ||
+        _annotationMatcher.isComponent(node, _assetId);
+    final retVal = super.visitAnnotation(node);
+    _isInlining = wasInlining;
+    return retVal;
+  }
+
+  @override
+  Object visitNamedExpression(NamedExpression node) {
+    if (_isInlining && node is NamedExpression) {
+      if (node.name is! Label || node.name.label is! SimpleIdentifier) {
+        throw new FormatException(
+            'Angular 2 currently only supports simple identifiers in directives.',
+            '$node' /* source */);
+      }
+      var keyString = '${node.name.label}';
+      switch (keyString) {
+        case 'templateUrl':
+          _populateTemplateUrl(node);
+          // Remove `templateUrl`
+          return null;
+        case 'styleUrls':
+          _populateStyleUrls(node);
+          // Remove `styleUrls`
+          return null;
+      }
+    }
+    return super.visitNamedExpression(node);
+  }
+
+  void _populateStyleUrls(NamedExpression node) {
+    var urls = _dumbEval(node.expression);
+    if (urls is! List) {
+      logger.warning('styleUrls is not a List of Strings (${node.expression})');
+      return;
+    }
+    _writer.print(_code.substring(_lastIndex, node.offset));
+    _lastIndex = node.end;
+    _writer.print('styles: const [');
+    for (var url in urls) {
+      if (url is String) {
+        _writer.print("r'''");
+        _writer.asyncPrint(_readOrEmptyString(url));
+        _writer.print("''', ");
+      } else {
+        logger.warning('style url is not a String (${url})');
+      }
+    }
+    _writer.println(']');
+  }
+
+  void _populateTemplateUrl(NamedExpression node) {
+    var url = _dumbEval(node.expression);
+    if (url is! String) {
+      logger.warning('template url is not a String (${node.expression})');
+      return;
+    }
+    _writer.print(_code.substring(_lastIndex, node.offset));
+    _lastIndex = node.end;
+    _writer.print("template: r'''");
+    _writer.asyncPrint(_readOrEmptyString(url));
+    _writer.println("'''");
+  }
+
+  /// Attempts to read the content from [url]. If [url] is relative, uses
+  /// [_baseUri] as resolution base.
+  Future<String> _readOrEmptyString(String url) async {
+    final resolvedUri = _urlResolver.resolve(_baseUri.toString(), url);
+
+    return _xhr.get(resolvedUri).catchError((_) {
+      logger.error('$_baseUri: could not read $url');
+      return '';
+    });
+  }
+}
+
+final _constantEvaluator = new ConstantEvaluator();
+
+dynamic _dumbEval(Expression expr) {
+  var val;
+  if (expr is SimpleStringLiteral) {
+    val = stringLiteralToString(expr);
+  } else {
+    val = expr.accept(_constantEvaluator);
+  }
+  return val != ConstantEvaluator.NOT_A_CONSTANT ? val : null;
+}

modules_dart/transform/lib/src/transform/transformer.dart

@@ -3,16 +3,17 @@ library angular2.src.transform.transformer;
 import 'package:barback/barback.dart';
 import 'package:dart_style/dart_style.dart';
 
-import 'deferred_rewriter/transformer.dart';
-import 'directive_metadata_linker/transformer.dart';
-import 'directive_processor/transformer.dart';
 import 'bind_generator/transformer.dart';
-import 'reflection_remover/transformer.dart';
-import 'stylesheet_compiler/transformer.dart';
-import 'template_compiler/transformer.dart';
 import 'common/formatter.dart' as formatter;
 import 'common/options.dart';
 import 'common/options_reader.dart';
+import 'deferred_rewriter/transformer.dart';
+import 'directive_metadata_linker/transformer.dart';
+import 'directive_processor/transformer.dart';
+import 'inliner_for_test/transformer.dart';
+import 'reflection_remover/transformer.dart';
+import 'stylesheet_compiler/transformer.dart';
+import 'template_compiler/transformer.dart';
 
 export 'common/options.dart';
 
@@ -25,17 +26,24 @@ class AngularTransformerGroup extends TransformerGroup {
   }
 
   factory AngularTransformerGroup(TransformerOptions options) {
-    var phases = [
-      [new ReflectionRemover(options)],
-      [new DirectiveProcessor(options)]
-    ];
-    phases.addAll([
-      [new DirectiveMetadataLinker()],
-      [new BindGenerator(options)],
-      [new TemplateCompiler(options)],
-      [new StylesheetCompiler()],
-      [new DeferredRewriter(options)]
-    ]);
+    var phases;
+    if (options.inlineViews) {
+      phases = [
+        [new InlinerForTest(options)]
+      ];
+    } else {
+      phases = [
+        [new ReflectionRemover(options)],
+        [new DirectiveProcessor(options)],
+        [new DirectiveMetadataLinker()],
+        [new BindGenerator(options)],
+        [
+          new TemplateCompiler(options),
+          new StylesheetCompiler(),
+          new DeferredRewriter(options)
+        ]
+      ];
+    }
     return new AngularTransformerGroup._(phases,
         formatCode: options.formatCode);
   }

modules_dart/transform/test/transform/inliner_for_test/all_tests.dart

@@ -2,9 +2,11 @@ library angular2.test.transform.inliner_for_test.all_tests;
 
 import 'dart:async';
 
+import 'package:angular2/src/transform/common/annotation_matcher.dart';
 import 'package:angular2/src/transform/common/asset_reader.dart';
 import 'package:angular2/src/transform/common/logging.dart' as log;
-import 'package:angular2/src/transform/inliner_for_test.dart';
+import 'package:angular2/src/transform/common/options.dart';
+import 'package:angular2/src/transform/inliner_for_test/transformer.dart';
 import 'package:barback/barback.dart';
 import 'package:code_transformers/tests.dart';
 import 'package:guinness/guinness.dart';
@@ -18,12 +20,14 @@ main() {
 }
 
 DartFormatter formatter = new DartFormatter();
+AnnotationMatcher annotationMatcher;
 
 void allTests() {
   AssetReader absoluteReader;
 
   beforeEach(() {
     absoluteReader = new TestAssetReader();
+    annotationMatcher = new AnnotationMatcher();
   });
 
   it('should inline `templateUrl` values', () async {
@@ -83,18 +87,46 @@ void allTests() {
     expect(output).toContain("{{greeting}}");
   });
 
+  it('should not inline values outside of View/Component annotations',
+      () async {
+    var output = await _testInline(
+        absoluteReader, _assetId('false_match_files/hello.dart'));
+
+    expect(output).toBeNotNull();
+    expect(output).not.toContain('{{greeting}}');
+    expect(output).toContain('.greeting { .color: blue; }');
+  });
+
+  it('should not modify files with no `templateUrl` or `styleUrls` values.',
+      () async {
+    var output = await _testInline(
+        absoluteReader, _assetId('no_modify_files/hello.dart'));
+
+    expect(output).toBeNull();
+  });
+
+  it('should not strip property annotations.', () async {
+    // Regression test for https://github.com/dart-lang/sdk/issues/24578
+    var output = await _testInline(
+        absoluteReader, _assetId('prop_annotations_files/hello.dart'));
+
+    expect(output).toContain('@Attribute(\'thing\')');
+  });
+
   _runAbsoluteUrlEndToEndTest();
   _runMultiStylesEndToEndTest();
 }
 
 Future<String> _testInline(AssetReader reader, AssetId assetId) {
-  return log.setZoned(new RecordingLogger(), () => inline(reader, assetId));
+  return log.setZoned(
+      new RecordingLogger(), () => inline(reader, assetId, annotationMatcher));
 }
 
 AssetId _assetId(String path) => new AssetId('a', 'inliner_for_test/$path');
 
 void _runAbsoluteUrlEndToEndTest() {
-  InlinerForTest transformer = new InlinerForTest(formatCode: true);
+  var options = new TransformerOptions([], inlineViews: true, formatCode: true);
+  InlinerForTest transformer = new InlinerForTest(options);
   var inputMap = {
     'a|absolute_url_expression_files/hello.dart':
         _readFile('absolute_url_expression_files/hello.dart'),
@@ -119,7 +151,8 @@ void _runAbsoluteUrlEndToEndTest() {
 }
 
 void _runMultiStylesEndToEndTest() {
-  InlinerForTest transformer = new InlinerForTest(formatCode: true);
+  var options = new TransformerOptions([], inlineViews: true, formatCode: true);
+  InlinerForTest transformer = new InlinerForTest(options);
   var inputMap = {
     'pkg|web/hello.dart': _readFile('multiple_style_urls_files/hello.dart'),
     'pkg|web/template.css': _readFile('multiple_style_urls_files/template.css'),

modules_dart/transform/test/transform/inliner_for_test/false_match_files/hello.dart

@@ -0,0 +1,12 @@
+library angular2.test.transform.inliner_for_test.false_match_files;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+
+@Component(selector: 'hello-app')
+@View(styleUrls: const ['template.css'])
+class HelloCmp {}
+
+void main() {
+  final testThing = new Component(templateUrl: 'template.html');
+}

modules_dart/transform/test/transform/inliner_for_test/false_match_files/template.css

@@ -0,0 +1 @@
+.greeting { .color: blue; }

modules_dart/transform/test/transform/inliner_for_test/false_match_files/template.html

@@ -0,0 +1 @@
+{{greeting}}

modules_dart/transform/test/transform/inliner_for_test/no_modify_files/hello.dart

@@ -0,0 +1,8 @@
+library angular2.test.transform.inliner_for_test.false_match_files;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+
+part 'hello_part.dart';
+
+void main() {}

modules_dart/transform/test/transform/inliner_for_test/no_modify_files/hello_part.dart

@@ -0,0 +1,3 @@
+part of angular2.test.transform.inliner_for_test.false_match_files;
+
+void doStuff() {}

modules_dart/transform/test/transform/inliner_for_test/prop_annotations_files/hello.dart

@@ -0,0 +1,10 @@
+library angular2.test.transform.inliner_for_test.false_match_files;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+
+@Component(selector: 'hello-app')
+@View(styleUrls: const ['template.css'])
+class HelloCmp {
+  HelloCmp(@Attribute('thing') String thing);
+}

modules_dart/transform/test/transform/inliner_for_test/prop_annotations_files/template.css

@@ -0,0 +1 @@
+.greeting { .color: blue; }