Skip to content

Commit a31e2f5

Browse files
Tim Blasikegluneq
Tim Blasi
authored and
kegluneq
committed
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
1 parent bcd926a commit a31e2f5

6 files changed

Lines changed: 135 additions & 20 deletions

File tree

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

Lines changed: 57 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
library angular2.src.transform.inliner_for_test.transformer;
22

33
import 'dart:async';
4+
import 'dart:convert' show LineSplitter;
45

56
import 'package:analyzer/analyzer.dart';
67
import 'package:analyzer/src/generated/ast.dart';
@@ -49,6 +50,7 @@ class InlinerForTest extends Transformer {
4950
}
5051

5152
/// Reads the code at `assetId`, inlining values where possible.
53+
///
5254
/// Returns the code at `assetId` with the following modifications:
5355
/// - `part` Directives are inlined
5456
/// - `templateUrl` values are inlined as `template` values.
@@ -73,6 +75,10 @@ Future<String> inline(AssetReader reader, AssetId assetId,
7375
final _urlResolver = const TransformerUrlResolver();
7476

7577
class _ViewPropInliner extends RecursiveAstVisitor<Object> {
78+
/// The prefixes given to inlined names.
79+
static const _inlinedTemplateBase = '_template';
80+
static const _inlinedStyleBase = '_style';
81+
7682
final AssetId _assetId;
7783

7884
/// The code we are operating on.
@@ -84,6 +90,9 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
8490
final XHR _xhr;
8591
final AnnotationMatcher _annotationMatcher;
8692

93+
/// Variable name, string to be inlined pairs.
94+
final _inlinedValues = <_InlinedValue>[];
95+
8796
/// The final index of the last substring we wrote.
8897
int _lastIndex = 0;
8998

@@ -105,6 +114,7 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
105114
final retVal = super.visitCompilationUnit(node);
106115
if (modifiedSource) {
107116
_writer.print(_code.substring(_lastIndex));
117+
_inlinedValues.forEach((v) => _writer.asyncPrint(v.asyncToString()));
108118
}
109119
return retVal;
110120
}
@@ -142,6 +152,18 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
142152
return super.visitNamedExpression(node);
143153
}
144154

155+
/// Counts the newline characters in the code represented by `node`.
156+
int _countNewlines(AstNode node) {
157+
if (node.offset == null ||
158+
node.offset < 0 ||
159+
node.end == null ||
160+
node.end < 0) {
161+
return 0;
162+
}
163+
return LineSplitter.split(_code.substring(node.offset, node.end)).length -
164+
1;
165+
}
166+
145167
void _populateStyleUrls(NamedExpression node) {
146168
var urls = naiveEval(node.expression);
147169
if (urls is! List) {
@@ -154,14 +176,16 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
154176
_writer.print('styles: const [');
155177
for (var url in urls) {
156178
if (url is String) {
157-
_writer.print("r'''");
158-
_writer.asyncPrint(_readOrEmptyString(url));
159-
_writer.print("''', ");
179+
final inlinedVal = _addInlineValue(url, varBase: _inlinedStyleBase);
180+
_writer.print('${inlinedVal.name},');
160181
} else {
161182
zone.log.warning('style url is not a String (${url})');
162183
}
163184
}
164-
_writer.println(']');
185+
_writer.print(']');
186+
for (var i = 0, n = _countNewlines(node); i < n; ++i) {
187+
_writer.println('');
188+
}
165189
}
166190

167191
void _populateTemplateUrl(NamedExpression node) {
@@ -172,9 +196,11 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
172196
}
173197
_writer.print(_code.substring(_lastIndex, node.offset));
174198
_lastIndex = node.end;
175-
_writer.print("template: r'''");
176-
_writer.asyncPrint(_readOrEmptyString(url));
177-
_writer.println("'''");
199+
final inlinedVal = _addInlineValue(url, varBase: _inlinedTemplateBase);
200+
_writer.print('template: ${inlinedVal.name}');
201+
for (var i = 0, n = _countNewlines(node); i < n; ++i) {
202+
_writer.println('');
203+
}
178204
}
179205

180206
/// Attempts to read the content from [url]. If [url] is relative, uses
@@ -187,4 +213,28 @@ class _ViewPropInliner extends RecursiveAstVisitor<Object> {
187213
return '';
188214
});
189215
}
216+
217+
/// Adds a url to be inlined and requests its content.
218+
///
219+
/// `varBase` is the base of the variable name the inlined value will be
220+
/// assigned to.
221+
/// Returns the created [_InlinedValue] object.
222+
_InlinedValue _addInlineValue(String url, {String varBase}) {
223+
final val = new _InlinedValue(
224+
'${varBase}${_inlinedValues.length}', _readOrEmptyString(url));
225+
_inlinedValues.add(val);
226+
return val;
227+
}
228+
}
229+
230+
class _InlinedValue {
231+
final String name;
232+
final Future<String> futureValue;
233+
234+
_InlinedValue(this.name, this.futureValue);
235+
236+
/// Returns a const declaration of the inlined value.
237+
Future<String> asyncToString() async {
238+
return "const $name = r'''${await futureValue}''';";
239+
}
190240
}

‎modules_dart/transform/test/transform/inliner_for_test/absolute_url_expression_files/expected/hello.dart‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@ import 'package:angular2/angular2.dart'
44
show Component, Directive, View, NgElement;
55

66
@Component(selector: 'hello-app')
7-
@View(
8-
template: r'''{{greeting}}''',
9-
styles: const [r'''.greeting { .color: blue; }''',])
7+
@View(template: _template0, styles: const [_style1,])
108
class HelloCmp {}
119

1210
@Injectable() hello() {}
11+
const _template0 = r'''{{greeting}}''';
12+
const _style1 = r'''.greeting { .color: blue; }''';

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

Lines changed: 55 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
library angular2.test.transform.inliner_for_test.all_tests;
22

33
import 'dart:async';
4+
import 'dart:convert' show LineSplitter;
45

56
import 'package:barback/barback.dart';
67
import 'package:code_transformers/tests.dart';
@@ -18,6 +19,7 @@ import '../common/recording_logger.dart';
1819

1920
main() {
2021
allTests();
22+
endToEndTests();
2123
}
2224

2325
DartFormatter formatter = new DartFormatter();
@@ -36,7 +38,7 @@ void allTests() {
3638
absoluteReader, _assetId('url_expression_files/hello.dart'));
3739
expect(output).toBeNotNull();
3840
expect(() => formatter.format(output)).not.toThrow();
39-
expect(output).toContain("template: r'''{{greeting}}'''");
41+
expect(output).toContain("r'''{{greeting}}'''");
4042
});
4143

4244
it(
@@ -57,9 +59,8 @@ void allTests() {
5759
expect(output).toBeNotNull();
5860
expect(() => formatter.format(output)).not.toThrow();
5961

60-
expect(output).toContain("template: r'''{{greeting}}'''");
61-
expect(output).toContain("styles: const ["
62-
"r'''.greeting { .color: blue; }''', ]");
62+
expect(output).toContain("r'''{{greeting}}'''");
63+
expect(output).toContain("r'''.greeting { .color: blue; }'''");
6364
});
6465

6566
it('should inline multiple `styleUrls` values expressed as absolute urls.',
@@ -114,6 +115,56 @@ void allTests() {
114115
expect(output).toContain('@Attribute(\'thing\')');
115116
});
116117

118+
it('should maintain line numbers for long `templateUrl` values', () async {
119+
// Regression test for https://github.com/angular/angular/issues/5281
120+
final templateUrlVal =
121+
'supersupersupersupersupersupersupersupersupersupersupersuper'
122+
'superlongtemplate.html';
123+
absoluteReader.addAsset(
124+
_assetId('multiline_template/$templateUrlVal'), '{{greeting}}');
125+
var output = await _testInline(
126+
absoluteReader, _assetId('multiline_template/hello.dart'));
127+
expect(output).toBeNotNull();
128+
expect(() => formatter.format(output)).not.toThrow();
129+
expect(output)
130+
..toContain("r'''{{greeting}}'''")
131+
..toContain('template: _template0\n');
132+
});
133+
134+
it('should maintain line numbers when replacing values', () async {
135+
// Regression test for https://github.com/angular/angular/issues/5281
136+
final templateUrlVal =
137+
'supersupersupersupersupersupersupersupersupersupersupersuper'
138+
'superlongtemplate.html';
139+
final t1Styles = '.body { color: green; }';
140+
final t2Styles = '.div { color: red; }';
141+
absoluteReader.addAsset(
142+
_assetId('multiline_template/$templateUrlVal'), '{{greeting}}');
143+
absoluteReader.addAsset(
144+
_assetId('multiline_template/pretty_longish_template.css'), t1Styles);
145+
absoluteReader.addAsset(
146+
_assetId('multiline_template/other_pretty_longish_template.css'),
147+
t2Styles);
148+
var output = await _testInline(
149+
absoluteReader, _assetId('multiline_template/hello.dart'));
150+
expect(output).toBeNotNull();
151+
expect(() => formatter.format(output)).not.toThrow();
152+
expect(output)
153+
..toContain("r'''{{greeting}}'''")
154+
..toContain("r'''$t1Styles'''")
155+
..toContain("r'''$t2Styles'''");
156+
157+
final splitter = const LineSplitter();
158+
final inputLines =
159+
splitter.convert(_readFile('multiline_template/hello.dart'));
160+
final outputLines = splitter.convert(output);
161+
162+
expect(outputLines.indexOf('class HelloCmp {}'))
163+
.toEqual(inputLines.indexOf('class HelloCmp {}'));
164+
});
165+
}
166+
167+
void endToEndTests() {
117168
_runAbsoluteUrlEndToEndTest();
118169
_runMultiStylesEndToEndTest();
119170
}

‎modules_dart/transform/test/transform/inliner_for_test/multiline_template/hello.dart‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
library playground.src.hello_world.url_expression_files;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
6+
@Component(selector: 'hello-app')
7+
@View(
8+
templateUrl: 'supersupersupersupersupersupersupersupersupersupersupersuper'
9+
'superlongtemplate.html',
10+
styleUrls: const [
11+
'pretty_longish_template.css',
12+
'other_pretty_longish_template.css'
13+
])
14+
class HelloCmp {}

‎modules_dart/transform/test/transform/inliner_for_test/multiple_style_urls_files/expected/hello.dart‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,9 @@ import 'package:angular2/angular2.dart'
44
show Component, Directive, View, NgElement;
55

66
@Component(selector: 'hello-app')
7-
@View(
8-
template: r'''{{greeting}}''',
9-
styles: const [
10-
r'''.greeting { .color: blue; }''',
11-
r'''.hello { .color: red; }''',
12-
])
7+
@View(template: _template0, styles: const [_style1, _style2,])
138
class HelloCmp {}
9+
10+
const _template0 = r'''{{greeting}}''';
11+
const _style1 = r'''.greeting { .color: blue; }''';
12+
const _style2 = r'''.hello { .color: red; }''';

‎modules_dart/transform/test/transform/transform.server.spec.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,5 +32,6 @@ main() {
3232
describe('Url Resolver', urlResolver.allTests);
3333
// NOTE(kegluneq): These use `code_transformers#testPhases`, which is not
3434
// designed to work with `guinness`.
35+
group('Inliner For Test e2e', inliner.endToEndTests);
3536
group('Transformer Pipeline', integration.allTests);
3637
}

0 commit comments

Comments
 (0)