fix(dart/transform): Consider of line numbers in inliner_for_test

Tim Blasi · kegluneq · commit a31e2f5f4799 · 2015-11-17T01:51:58.000Z
Ensure that line numbers aren't changed by inliner_for_test. Fixes #5281 Closes #5285
diff --git a/modules_dart/transform/lib/src/transform/inliner_for_test/transformer.dart b/modules_dart/transform/lib/src/transform/inliner_for_test/transformer.dart
@@ -1,6 +1,7 @@
 library angular2.src.transform.inliner_for_test.transformer;
 
 import 'dart:async';
+import 'dart:convert' show LineSplitter;
 
 import 'package:analyzer/analyzer.dart';
 import 'package:analyzer/src/generated/ast.dart';
@@ -49,6 +50,7 @@ class InlinerForTest extends Transformer {
 }
 
 /// 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.
@@ -73,6 +75,10 @@ Future<String> inline(AssetReader reader, AssetId assetId,
 final _urlResolver = const TransformerUrlResolver();
 
 class _ViewPropInliner extends RecursiveAstVisitor<Object> {
+  /// The prefixes given to inlined names.
+  static const _inlinedTemplateBase = '_template';
+  static const _inlinedStyleBase = '_style';
+
   final AssetId _assetId;
 
   /// The code we are operating on.
@@ -84,6 +90,9 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
   final XHR _xhr;
   final AnnotationMatcher _annotationMatcher;
 
+  /// Variable name, string to be inlined pairs.
+  final _inlinedValues = <_InlinedValue>[];
+
   /// The final index of the last substring we wrote.
   int _lastIndex = 0;
 
@@ -105,6 +114,7 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
     final retVal = super.visitCompilationUnit(node);
     if (modifiedSource) {
       _writer.print(_code.substring(_lastIndex));
+      _inlinedValues.forEach((v) => _writer.asyncPrint(v.asyncToString()));
     }
     return retVal;
   }
@@ -142,6 +152,18 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
     return super.visitNamedExpression(node);
   }
 
+  /// Counts the newline characters in the code represented by `node`.
+  int _countNewlines(AstNode node) {
+    if (node.offset == null ||
+        node.offset < 0 ||
+        node.end == null ||
+        node.end < 0) {
+      return 0;
+    }
+    return LineSplitter.split(_code.substring(node.offset, node.end)).length -
+        1;
+  }
+
   void _populateStyleUrls(NamedExpression node) {
     var urls = naiveEval(node.expression);
     if (urls is! List) {
@@ -154,14 +176,16 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
     _writer.print('styles: const [');
     for (var url in urls) {
       if (url is String) {
-        _writer.print("r'''");
-        _writer.asyncPrint(_readOrEmptyString(url));
-        _writer.print("''', ");
+        final inlinedVal = _addInlineValue(url, varBase: _inlinedStyleBase);
+        _writer.print('${inlinedVal.name},');
       } else {
         zone.log.warning('style url is not a String (${url})');
       }
     }
-    _writer.println(']');
+    _writer.print(']');
+    for (var i = 0, n = _countNewlines(node); i < n; ++i) {
+      _writer.println('');
+    }
   }
 
   void _populateTemplateUrl(NamedExpression node) {
@@ -172,9 +196,11 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
     }
     _writer.print(_code.substring(_lastIndex, node.offset));
     _lastIndex = node.end;
-    _writer.print("template: r'''");
-    _writer.asyncPrint(_readOrEmptyString(url));
-    _writer.println("'''");
+    final inlinedVal = _addInlineValue(url, varBase: _inlinedTemplateBase);
+    _writer.print('template: ${inlinedVal.name}');
+    for (var i = 0, n = _countNewlines(node); i < n; ++i) {
+      _writer.println('');
+    }
   }
 
   /// Attempts to read the content from [url]. If [url] is relative, uses
@@ -187,4 +213,28 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
       return '';
     });
   }
+
+  /// Adds a url to be inlined and requests its content.
+  ///
+  /// `varBase` is the base of the variable name the inlined value will be
+  /// assigned to.
+  /// Returns the created [_InlinedValue] object.
+  _InlinedValue _addInlineValue(String url, {String varBase}) {
+    final val = new _InlinedValue(
+        '${varBase}${_inlinedValues.length}', _readOrEmptyString(url));
+    _inlinedValues.add(val);
+    return val;
+  }
+}
+
+class _InlinedValue {
+  final String name;
+  final Future<String> futureValue;
+
+  _InlinedValue(this.name, this.futureValue);
+
+  /// Returns a const declaration of the inlined value.
+  Future<String> asyncToString() async {
+    return "const $name = r'''${await futureValue}''';";
+  }
 }
diff --git a/modules_dart/transform/test/transform/inliner_for_test/absolute_url_expression_files/expected/hello.dart b/modules_dart/transform/test/transform/inliner_for_test/absolute_url_expression_files/expected/hello.dart
@@ -4,9 +4,9 @@ import 'package:angular2/angular2.dart'
     show Component, Directive, View, NgElement;
 
 @Component(selector: 'hello-app')
-@View(
-    template: r'''{{greeting}}''',
-    styles: const [r'''.greeting { .color: blue; }''',])
+@View(template: _template0, styles: const [_style1,])
 class HelloCmp {}
 
 @Injectable() hello() {}
+const _template0 = r'''{{greeting}}''';
+const _style1 = r'''.greeting { .color: blue; }''';
diff --git a/modules_dart/transform/test/transform/inliner_for_test/all_tests.dart b/modules_dart/transform/test/transform/inliner_for_test/all_tests.dart
@@ -1,6 +1,7 @@
 library angular2.test.transform.inliner_for_test.all_tests;
 
 import 'dart:async';
+import 'dart:convert' show LineSplitter;
 
 import 'package:barback/barback.dart';
 import 'package:code_transformers/tests.dart';
@@ -18,6 +19,7 @@ import '../common/recording_logger.dart';
 
 main() {
   allTests();
+  endToEndTests();
 }
 
 DartFormatter formatter = new DartFormatter();
@@ -36,7 +38,7 @@ void allTests() {
         absoluteReader, _assetId('url_expression_files/hello.dart'));
     expect(output).toBeNotNull();
     expect(() => formatter.format(output)).not.toThrow();
-    expect(output).toContain("template: r'''{{greeting}}'''");
+    expect(output).toContain("r'''{{greeting}}'''");
   });
 
   it(
@@ -57,9 +59,8 @@ void allTests() {
     expect(output).toBeNotNull();
     expect(() => formatter.format(output)).not.toThrow();
 
-    expect(output).toContain("template: r'''{{greeting}}'''");
-    expect(output).toContain("styles: const ["
-        "r'''.greeting { .color: blue; }''', ]");
+    expect(output).toContain("r'''{{greeting}}'''");
+    expect(output).toContain("r'''.greeting { .color: blue; }'''");
   });
 
   it('should inline multiple `styleUrls` values expressed as absolute urls.',
@@ -114,6 +115,56 @@ void allTests() {
     expect(output).toContain('@Attribute(\'thing\')');
   });
 
+  it('should maintain line numbers for long `templateUrl` values', () async {
+    // Regression test for https://github.com/angular/angular/issues/5281
+    final templateUrlVal =
+        'supersupersupersupersupersupersupersupersupersupersupersuper'
+        'superlongtemplate.html';
+    absoluteReader.addAsset(
+        _assetId('multiline_template/$templateUrlVal'), '{{greeting}}');
+    var output = await _testInline(
+        absoluteReader, _assetId('multiline_template/hello.dart'));
+    expect(output).toBeNotNull();
+    expect(() => formatter.format(output)).not.toThrow();
+    expect(output)
+      ..toContain("r'''{{greeting}}'''")
+      ..toContain('template: _template0\n');
+  });
+
+  it('should maintain line numbers when replacing values', () async {
+    // Regression test for https://github.com/angular/angular/issues/5281
+    final templateUrlVal =
+        'supersupersupersupersupersupersupersupersupersupersupersuper'
+        'superlongtemplate.html';
+    final t1Styles = '.body { color: green; }';
+    final t2Styles = '.div { color: red; }';
+    absoluteReader.addAsset(
+        _assetId('multiline_template/$templateUrlVal'), '{{greeting}}');
+    absoluteReader.addAsset(
+        _assetId('multiline_template/pretty_longish_template.css'), t1Styles);
+    absoluteReader.addAsset(
+        _assetId('multiline_template/other_pretty_longish_template.css'),
+        t2Styles);
+    var output = await _testInline(
+        absoluteReader, _assetId('multiline_template/hello.dart'));
+    expect(output).toBeNotNull();
+    expect(() => formatter.format(output)).not.toThrow();
+    expect(output)
+      ..toContain("r'''{{greeting}}'''")
+      ..toContain("r'''$t1Styles'''")
+      ..toContain("r'''$t2Styles'''");
+
+    final splitter = const LineSplitter();
+    final inputLines =
+        splitter.convert(_readFile('multiline_template/hello.dart'));
+    final outputLines = splitter.convert(output);
+
+    expect(outputLines.indexOf('class HelloCmp {}'))
+        .toEqual(inputLines.indexOf('class HelloCmp {}'));
+  });
+}
+
+void endToEndTests() {
   _runAbsoluteUrlEndToEndTest();
   _runMultiStylesEndToEndTest();
 }
diff --git a/modules_dart/transform/test/transform/inliner_for_test/multiline_template/hello.dart b/modules_dart/transform/test/transform/inliner_for_test/multiline_template/hello.dart
@@ -0,0 +1,14 @@
+library playground.src.hello_world.url_expression_files;
+
+import 'package:angular2/angular2.dart'
+    show Component, Directive, View, NgElement;
+
+@Component(selector: 'hello-app')
+@View(
+    templateUrl: 'supersupersupersupersupersupersupersupersupersupersupersuper'
+        'superlongtemplate.html',
+    styleUrls: const [
+      'pretty_longish_template.css',
+      'other_pretty_longish_template.css'
+    ])
+class HelloCmp {}
diff --git a/modules_dart/transform/test/transform/inliner_for_test/multiple_style_urls_files/expected/hello.dart b/modules_dart/transform/test/transform/inliner_for_test/multiple_style_urls_files/expected/hello.dart
@@ -4,10 +4,9 @@ import 'package:angular2/angular2.dart'
     show Component, Directive, View, NgElement;
 
 @Component(selector: 'hello-app')
-@View(
-    template: r'''{{greeting}}''',
-    styles: const [
-      r'''.greeting { .color: blue; }''',
-      r'''.hello { .color: red; }''',
-    ])
+@View(template: _template0, styles: const [_style1, _style2,])
 class HelloCmp {}
+
+const _template0 = r'''{{greeting}}''';
+const _style1 = r'''.greeting { .color: blue; }''';
+const _style2 = r'''.hello { .color: red; }''';
diff --git a/modules_dart/transform/test/transform/transform.server.spec.dart b/modules_dart/transform/test/transform/transform.server.spec.dart
@@ -32,5 +32,6 @@ main() {
   describe('Url Resolver', urlResolver.allTests);
   // NOTE(kegluneq): These use `code_transformers#testPhases`, which is not
   // designed to work with `guinness`.
+  group('Inliner For Test e2e', inliner.endToEndTests);
   group('Transformer Pipeline', integration.allTests);
 }

Original file line number	Diff line number	Diff line change
`@@ -32,5 +32,6 @@ main() {`
`32`	`32`	`describe('Url Resolver', urlResolver.allTests);`
`33`	`33`	// NOTE(kegluneq): These use `code_transformers#testPhases`, which is not
`34`	`34`	// designed to work with `guinness`.
	`35`	`+ group('Inliner For Test e2e', inliner.endToEndTests);`
`35`	`36`	`group('Transformer Pipeline', integration.allTests);`
`36`	`37`	`}`