From a31e2f5f4799e9ce20ae508f9f91dfeba05805f2 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Fri, 13 Nov 2015 17:13:47 -0800 Subject: [PATCH] fix(dart/transform): Consider of line numbers in inliner_for_test Ensure that line numbers aren't changed by inliner_for_test. Fixes #5281 Closes #5285 --- .../inliner_for_test/transformer.dart | 64 +++++++++++++++++-- .../expected/hello.dart | 6 +- .../transform/inliner_for_test/all_tests.dart | 59 +++++++++++++++-- .../multiline_template/hello.dart | 14 ++++ .../expected/hello.dart | 11 ++-- .../test/transform/transform.server.spec.dart | 1 + 6 files changed, 135 insertions(+), 20 deletions(-) create mode 100644 modules_dart/transform/test/transform/inliner_for_test/multiline_template/hello.dart 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 index 034a3057f4..87bf181223 100644 --- 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 inline(AssetReader reader, AssetId assetId, final _urlResolver = const TransformerUrlResolver(); class _ViewPropInliner extends RecursiveAstVisitor { + /// 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 { 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 { 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 { 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 { _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 { } _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 { 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 futureValue; + + _InlinedValue(this.name, this.futureValue); + + /// Returns a const declaration of the inlined value. + Future 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 index 8a4c74a483..ddb3bee244 100644 --- 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 index b20e271e23..a1a737d914 100644 --- 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 new file mode 100644 index 0000000000..d33fd5bb5c --- /dev/null +++ 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 index 05db7e38b8..db46c02ace 100644 --- 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 index 4851717964..d3fe7a54e1 100644 --- 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); }