X Tutup
Skip to content

Commit decdbea

Browse files
author
Tim Blasi
committed
fix(dart/transformer): Correctly handle const object annotations
Previously, annotations which were const objects (as opposed to const instance creation expressions) were incorrectly output as instance creation expressions. Before: ``` const override() // A const instance creation expression ``` After ``` override // A const instance ``` Closes #4481
1 parent 01fb06a commit decdbea

File tree

9 files changed

+216
-59
lines changed

9 files changed

+216
-59
lines changed

‎modules_dart/transform/lib/src/transform/bind_generator/generator.dart‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ class _ExtractQueryFieldsFromPropMetadata extends Object
5050
bool _hasQueryAnnotation(list) {
5151
var res = false;
5252
list.elements.forEach((item) {
53+
if (item is! InstanceCreationExpression) return;
5354
var n = item.constructorName.toString();
5455
if (n == "ContentChild" ||
5556
n == "ViewChild" ||

‎modules_dart/transform/lib/src/transform/common/code/annotation_code.dart‎

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,12 @@ class AnnotationVisitor extends SimpleAstVisitor<AnnotationModel> {
3636
..isComponent = isComponent
3737
..isDirective = isDirective
3838
..isInjectable = isInjectable
39-
..isView = isView;
39+
..isView = isView
40+
..isConstObject = node.arguments == null;
4041

41-
if (node.arguments != null) {
42+
// This annotation is a constant instance creation expression,
43+
// e.g. @Injectable(), rather than a const object, e.g. @override.
44+
if (!model.isConstObject) {
4245
for (var arg in node.arguments.arguments) {
4346
if (arg is NamedExpression) {
4447
model.namedParameters.add(new NamedParameter()
@@ -60,7 +63,11 @@ abstract class AnnotationWriterMixin {
6063
StringBuffer get buffer;
6164

6265
void writeAnnotationModel(AnnotationModel model) {
63-
if (model.parameters != null || model.namedParameters != null) {
66+
if (model.isConstObject) {
67+
// This is a const instance, not a ctor invocation and does not need a
68+
// const instance creation expression.
69+
buffer.write(model.name);
70+
} else {
6471
buffer.write('const ${model.name}(');
6572
var first = true;
6673
for (var param in model.parameters) {
@@ -83,10 +90,6 @@ abstract class AnnotationWriterMixin {
8390
buffer.write('${param.name}: ${param.value}');
8491
}
8592
buffer.write(')');
86-
} else {
87-
// This is a const instance, not a ctor invocation and does not need a
88-
// const instance creation expression.
89-
buffer.write(model.name);
9093
}
9194
}
9295
}

‎modules_dart/transform/lib/src/transform/common/model/annotation_model.pb.dart‎

Lines changed: 77 additions & 25 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎modules_dart/transform/lib/src/transform/common/model/annotation_model.proto‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,11 +25,16 @@ message AnnotationModel {
2525
// account, that is, this should be true if `is_component` is true.
2626
optional bool is_directive = 5;
2727

28-
// Whether htis is a `Component` annotation.
28+
// Whether this is a `Component` annotation.
2929
optional bool is_component = 6;
3030

3131
// Whether this is an `Injectable` annotation. This takes inheritance into
3232
// account, that is, this should be true if `is_directive` and/or
3333
// `is_component` is true.
3434
optional bool is_injectable = 7;
35+
36+
// Whether this annotation is a constant object (for example, `@override`) as
37+
// opposed to a const instance creation expression
38+
// (for example, `@Injectable()`).
39+
optional bool is_const_object = 8;
3540
}

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

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -16,49 +16,65 @@ void allTests() {
1616

1717
it('should generate a setter for an `inputs` property in an annotation.',
1818
() async {
19-
var inputPath = 'bind_generator/basic_bind_files/bar.ng_deps.dart';
20-
var expected = formatter.format(
21-
readFile('bind_generator/basic_bind_files/expected/bar.ng_deps.dart'));
19+
var inputPath = 'basic_bind_files/bar.ng_deps.dart';
20+
var expected = _readFile('basic_bind_files/expected/bar.ng_deps.dart');
2221

23-
var output = formatter.format(
24-
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
22+
var output = formatter
23+
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
2524
expect(output).toEqual(expected);
2625
});
2726

2827
it(
2928
'should generate a single setter when multiple annotations bind to the '
3029
'same `inputs` property.', () async {
31-
var inputPath =
32-
'bind_generator/duplicate_bind_name_files/soup.ng_deps.dart';
33-
var expected = formatter.format(readFile(
34-
'bind_generator/duplicate_bind_name_files/expected/soup.ng_deps.dart'));
30+
var inputPath = 'duplicate_bind_name_files/soup.ng_deps.dart';
31+
var expected =
32+
_readFile('duplicate_bind_name_files/expected/soup.ng_deps.dart');
3533

36-
var output = formatter.format(
37-
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
34+
var output = formatter
35+
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
3836
expect(output).toEqual(expected);
3937
});
4038

4139
it('should generate setters for queries defined in the class annotation.',
4240
() async {
43-
var inputPath =
44-
'bind_generator/queries_class_annotation_files/bar.ng_deps.dart';
45-
var expected = formatter.format(readFile(
46-
'bind_generator/queries_class_annotation_files/expected/bar.ng_deps.dart'));
41+
var inputPath = 'queries_class_annotation_files/bar.ng_deps.dart';
42+
var expected =
43+
_readFile('queries_class_annotation_files/expected/bar.ng_deps.dart');
4744

48-
var output = formatter.format(
49-
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
45+
var output = formatter
46+
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
5047
expect(output).toEqual(expected);
5148
});
5249

5350
it('should generate setters for queries defined via prop annotations.',
5451
() async {
55-
var inputPath =
56-
'bind_generator/queries_prop_annotations_files/bar.ng_deps.dart';
57-
var expected = formatter.format(readFile(
58-
'bind_generator/queries_prop_annotations_files/expected/bar.ng_deps.dart'));
52+
var inputPath = 'queries_prop_annotations_files/bar.ng_deps.dart';
53+
var expected =
54+
_readFile('queries_prop_annotations_files/expected/bar.ng_deps.dart');
5955

60-
var output = formatter.format(
61-
await createNgSettersAndGetters(reader, new AssetId('a', inputPath)));
56+
var output = formatter
57+
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
6258
expect(output).toEqual(expected);
6359
});
60+
61+
it('should gracefully handle const objects as prop annotations.', () async {
62+
var inputPath = 'queries_override_annotation_files/bar.ng_deps.dart';
63+
var expected = formatter.format(_readFile(
64+
'queries_override_annotation_files/expected/bar.ng_deps.dart'));
65+
66+
var output = formatter
67+
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
68+
expect(output).toEqual(expected);
69+
});
70+
}
71+
72+
AssetId _assetId(String path) => new AssetId('a', 'bind_generator/$path');
73+
74+
String _readFile(String path) {
75+
var code = readFile('bind_generator/$path');
76+
if (path.endsWith('.dart')) {
77+
code = formatter.format(code);
78+
}
79+
return code;
6480
}

‎modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/bar.ng_deps.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
library bar.ng_deps.dart;
2+
3+
import 'bar.dart';
4+
import 'package:angular2/src/core/metadata.dart';
5+
6+
var _visited = false;
7+
void initReflector(reflector) {
8+
if (_visited) return;
9+
_visited = true;
10+
reflector
11+
..registerType(
12+
ToolTip,
13+
new ReflectionInfo(
14+
const [const Directive(selector: '[tool-tip]')],
15+
const [],
16+
() => new ToolTip(),
17+
null,
18+
const {
19+
'queryField': const [override]
20+
}));
21+
}

‎modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/expected/bar.ng_deps.dart‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
library bar.ng_deps.dart;
2+
3+
import 'bar.dart';
4+
import 'package:angular2/src/core/metadata.dart';
5+
6+
var _visited = false;
7+
void initReflector(reflector) {
8+
if (_visited) return;
9+
_visited = true;
10+
reflector
11+
..registerType(
12+
ToolTip,
13+
new ReflectionInfo(
14+
const [const Directive(selector: '[tool-tip]')],
15+
const [],
16+
() => new ToolTip(),
17+
null,
18+
const {
19+
'queryField': const [override]
20+
}));
21+
}

0 commit comments

Comments
 (0)
X Tutup