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
This commit is contained in:
Tim Blasi 2015-10-02 15:54:32 -07:00
parent 01fb06a377
commit decdbea7d7
9 changed files with 216 additions and 59 deletions

View File

@ -50,6 +50,7 @@ class _ExtractQueryFieldsFromPropMetadata extends Object
bool _hasQueryAnnotation(list) { bool _hasQueryAnnotation(list) {
var res = false; var res = false;
list.elements.forEach((item) { list.elements.forEach((item) {
if (item is! InstanceCreationExpression) return;
var n = item.constructorName.toString(); var n = item.constructorName.toString();
if (n == "ContentChild" || if (n == "ContentChild" ||
n == "ViewChild" || n == "ViewChild" ||

View File

@ -36,9 +36,12 @@ class AnnotationVisitor extends SimpleAstVisitor<AnnotationModel> {
..isComponent = isComponent ..isComponent = isComponent
..isDirective = isDirective ..isDirective = isDirective
..isInjectable = isInjectable ..isInjectable = isInjectable
..isView = isView; ..isView = isView
..isConstObject = node.arguments == null;
if (node.arguments != null) { // This annotation is a constant instance creation expression,
// e.g. @Injectable(), rather than a const object, e.g. @override.
if (!model.isConstObject) {
for (var arg in node.arguments.arguments) { for (var arg in node.arguments.arguments) {
if (arg is NamedExpression) { if (arg is NamedExpression) {
model.namedParameters.add(new NamedParameter() model.namedParameters.add(new NamedParameter()
@ -60,7 +63,11 @@ abstract class AnnotationWriterMixin {
StringBuffer get buffer; StringBuffer get buffer;
void writeAnnotationModel(AnnotationModel model) { void writeAnnotationModel(AnnotationModel model) {
if (model.parameters != null || model.namedParameters != null) { if (model.isConstObject) {
// This is a const instance, not a ctor invocation and does not need a
// const instance creation expression.
buffer.write(model.name);
} else {
buffer.write('const ${model.name}('); buffer.write('const ${model.name}(');
var first = true; var first = true;
for (var param in model.parameters) { for (var param in model.parameters) {
@ -83,10 +90,6 @@ abstract class AnnotationWriterMixin {
buffer.write('${param.name}: ${param.value}'); buffer.write('${param.name}: ${param.value}');
} }
buffer.write(')'); buffer.write(')');
} else {
// This is a const instance, not a ctor invocation and does not need a
// const instance creation expression.
buffer.write(model.name);
} }
} }
} }

View File

@ -8,67 +8,91 @@ import 'package:protobuf/protobuf.dart';
class NamedParameter extends GeneratedMessage { class NamedParameter extends GeneratedMessage {
static final BuilderInfo _i = new BuilderInfo('NamedParameter') static final BuilderInfo _i = new BuilderInfo('NamedParameter')
..a(1, 'name', PbFieldType.QS) ..a(1, 'name', PbFieldType.QS)
..a(2, 'value', PbFieldType.QS) ..a(2, 'value', PbFieldType.QS);
;
NamedParameter() : super(); NamedParameter() : super();
NamedParameter.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r); NamedParameter.fromBuffer(List<int> i,
NamedParameter.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r); [ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromBuffer(i, r);
NamedParameter.fromJson(String i,
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromJson(i, r);
NamedParameter clone() => new NamedParameter()..mergeFromMessage(this); NamedParameter clone() => new NamedParameter()..mergeFromMessage(this);
BuilderInfo get info_ => _i; BuilderInfo get info_ => _i;
static NamedParameter create() => new NamedParameter(); static NamedParameter create() => new NamedParameter();
static PbList<NamedParameter> createRepeated() => new PbList<NamedParameter>(); static PbList<NamedParameter> createRepeated() =>
new PbList<NamedParameter>();
static NamedParameter getDefault() { static NamedParameter getDefault() {
if (_defaultInstance == null) _defaultInstance = new _ReadonlyNamedParameter(); if (_defaultInstance == null) _defaultInstance =
new _ReadonlyNamedParameter();
return _defaultInstance; return _defaultInstance;
} }
static NamedParameter _defaultInstance; static NamedParameter _defaultInstance;
static void $checkItem(NamedParameter v) { static void $checkItem(NamedParameter v) {
if (v is !NamedParameter) checkItemFailed(v, 'NamedParameter'); if (v is! NamedParameter) checkItemFailed(v, 'NamedParameter');
} }
String get name => getField(1); String get name => getField(1);
void set name(String v) { setField(1, v); } void set name(String v) {
setField(1, v);
}
bool hasName() => hasField(1); bool hasName() => hasField(1);
void clearName() => clearField(1); void clearName() => clearField(1);
String get value => getField(2); String get value => getField(2);
void set value(String v) { setField(2, v); } void set value(String v) {
setField(2, v);
}
bool hasValue() => hasField(2); bool hasValue() => hasField(2);
void clearValue() => clearField(2); void clearValue() => clearField(2);
} }
class _ReadonlyNamedParameter extends NamedParameter with ReadonlyMessageMixin {} class _ReadonlyNamedParameter extends NamedParameter with ReadonlyMessageMixin {
}
class AnnotationModel extends GeneratedMessage { class AnnotationModel extends GeneratedMessage {
static final BuilderInfo _i = new BuilderInfo('AnnotationModel') static final BuilderInfo _i = new BuilderInfo('AnnotationModel')
..a(1, 'name', PbFieldType.QS) ..a(1, 'name', PbFieldType.QS)
..p(2, 'parameters', PbFieldType.PS) ..p(2, 'parameters', PbFieldType.PS)
..pp(3, 'namedParameters', PbFieldType.PM, NamedParameter.$checkItem, NamedParameter.create) ..pp(3, 'namedParameters', PbFieldType.PM, NamedParameter.$checkItem,
NamedParameter.create)
..a(4, 'isView', PbFieldType.OB) ..a(4, 'isView', PbFieldType.OB)
..a(5, 'isDirective', PbFieldType.OB) ..a(5, 'isDirective', PbFieldType.OB)
..a(6, 'isComponent', PbFieldType.OB) ..a(6, 'isComponent', PbFieldType.OB)
..a(7, 'isInjectable', PbFieldType.OB) ..a(7, 'isInjectable', PbFieldType.OB)
; ..a(8, 'isConstObject', PbFieldType.OB);
AnnotationModel() : super(); AnnotationModel() : super();
AnnotationModel.fromBuffer(List<int> i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r); AnnotationModel.fromBuffer(List<int> i,
AnnotationModel.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r); [ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromBuffer(i, r);
AnnotationModel.fromJson(String i,
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
: super.fromJson(i, r);
AnnotationModel clone() => new AnnotationModel()..mergeFromMessage(this); AnnotationModel clone() => new AnnotationModel()..mergeFromMessage(this);
BuilderInfo get info_ => _i; BuilderInfo get info_ => _i;
static AnnotationModel create() => new AnnotationModel(); static AnnotationModel create() => new AnnotationModel();
static PbList<AnnotationModel> createRepeated() => new PbList<AnnotationModel>(); static PbList<AnnotationModel> createRepeated() =>
new PbList<AnnotationModel>();
static AnnotationModel getDefault() { static AnnotationModel getDefault() {
if (_defaultInstance == null) _defaultInstance = new _ReadonlyAnnotationModel(); if (_defaultInstance == null) _defaultInstance =
new _ReadonlyAnnotationModel();
return _defaultInstance; return _defaultInstance;
} }
static AnnotationModel _defaultInstance; static AnnotationModel _defaultInstance;
static void $checkItem(AnnotationModel v) { static void $checkItem(AnnotationModel v) {
if (v is !AnnotationModel) checkItemFailed(v, 'AnnotationModel'); if (v is! AnnotationModel) checkItemFailed(v, 'AnnotationModel');
} }
String get name => getField(1); String get name => getField(1);
void set name(String v) { setField(1, v); } void set name(String v) {
setField(1, v);
}
bool hasName() => hasField(1); bool hasName() => hasField(1);
void clearName() => clearField(1); void clearName() => clearField(1);
@ -77,27 +101,48 @@ class AnnotationModel extends GeneratedMessage {
List<NamedParameter> get namedParameters => getField(3); List<NamedParameter> get namedParameters => getField(3);
bool get isView => getField(4); bool get isView => getField(4);
void set isView(bool v) { setField(4, v); } void set isView(bool v) {
setField(4, v);
}
bool hasIsView() => hasField(4); bool hasIsView() => hasField(4);
void clearIsView() => clearField(4); void clearIsView() => clearField(4);
bool get isDirective => getField(5); bool get isDirective => getField(5);
void set isDirective(bool v) { setField(5, v); } void set isDirective(bool v) {
setField(5, v);
}
bool hasIsDirective() => hasField(5); bool hasIsDirective() => hasField(5);
void clearIsDirective() => clearField(5); void clearIsDirective() => clearField(5);
bool get isComponent => getField(6); bool get isComponent => getField(6);
void set isComponent(bool v) { setField(6, v); } void set isComponent(bool v) {
setField(6, v);
}
bool hasIsComponent() => hasField(6); bool hasIsComponent() => hasField(6);
void clearIsComponent() => clearField(6); void clearIsComponent() => clearField(6);
bool get isInjectable => getField(7); bool get isInjectable => getField(7);
void set isInjectable(bool v) { setField(7, v); } void set isInjectable(bool v) {
setField(7, v);
}
bool hasIsInjectable() => hasField(7); bool hasIsInjectable() => hasField(7);
void clearIsInjectable() => clearField(7); void clearIsInjectable() => clearField(7);
bool get isConstObject => getField(8);
void set isConstObject(bool v) {
setField(8, v);
}
bool hasIsConstObject() => hasField(8);
void clearIsConstObject() => clearField(8);
} }
class _ReadonlyAnnotationModel extends AnnotationModel with ReadonlyMessageMixin {} class _ReadonlyAnnotationModel extends AnnotationModel
with ReadonlyMessageMixin {}
const NamedParameter$json = const { const NamedParameter$json = const {
'1': 'NamedParameter', '1': 'NamedParameter',
@ -112,17 +157,24 @@ const AnnotationModel$json = const {
'2': const [ '2': const [
const {'1': 'name', '3': 1, '4': 2, '5': 9}, const {'1': 'name', '3': 1, '4': 2, '5': 9},
const {'1': 'parameters', '3': 2, '4': 3, '5': 9}, const {'1': 'parameters', '3': 2, '4': 3, '5': 9},
const {'1': 'named_parameters', '3': 3, '4': 3, '5': 11, '6': '.angular2.src.transform.common.model.proto.NamedParameter'}, const {
'1': 'named_parameters',
'3': 3,
'4': 3,
'5': 11,
'6': '.angular2.src.transform.common.model.proto.NamedParameter'
},
const {'1': 'is_view', '3': 4, '4': 1, '5': 8}, const {'1': 'is_view', '3': 4, '4': 1, '5': 8},
const {'1': 'is_directive', '3': 5, '4': 1, '5': 8}, const {'1': 'is_directive', '3': 5, '4': 1, '5': 8},
const {'1': 'is_component', '3': 6, '4': 1, '5': 8}, const {'1': 'is_component', '3': 6, '4': 1, '5': 8},
const {'1': 'is_injectable', '3': 7, '4': 1, '5': 8}, const {'1': 'is_injectable', '3': 7, '4': 1, '5': 8},
const {'1': 'is_const_object', '3': 8, '4': 1, '5': 8},
], ],
}; };
/** /**
* Generated with: * Generated with:
* annotation_model.proto (5ca037df4c4d3e5d2771a177c9f5ea9dc8ae5f91) * annotation_model.proto (93cb7c1fba2e56d937fec054b6e119a2a2b9afe7)
* libprotoc 2.5.0 * libprotoc 2.5.0
* dart-protoc-plugin (cc35f743de982a4916588b9c505dd21c7fe87d17) * dart-protoc-plugin (cc35f743de982a4916588b9c505dd21c7fe87d17)
*/ */

View File

@ -25,11 +25,16 @@ message AnnotationModel {
// account, that is, this should be true if `is_component` is true. // account, that is, this should be true if `is_component` is true.
optional bool is_directive = 5; optional bool is_directive = 5;
// Whether htis is a `Component` annotation. // Whether this is a `Component` annotation.
optional bool is_component = 6; optional bool is_component = 6;
// Whether this is an `Injectable` annotation. This takes inheritance into // Whether this is an `Injectable` annotation. This takes inheritance into
// account, that is, this should be true if `is_directive` and/or // account, that is, this should be true if `is_directive` and/or
// `is_component` is true. // `is_component` is true.
optional bool is_injectable = 7; optional bool is_injectable = 7;
// Whether this annotation is a constant object (for example, `@override`) as
// opposed to a const instance creation expression
// (for example, `@Injectable()`).
optional bool is_const_object = 8;
} }

View File

@ -16,49 +16,65 @@ void allTests() {
it('should generate a setter for an `inputs` property in an annotation.', it('should generate a setter for an `inputs` property in an annotation.',
() async { () async {
var inputPath = 'bind_generator/basic_bind_files/bar.ng_deps.dart'; var inputPath = 'basic_bind_files/bar.ng_deps.dart';
var expected = formatter.format( var expected = _readFile('basic_bind_files/expected/bar.ng_deps.dart');
readFile('bind_generator/basic_bind_files/expected/bar.ng_deps.dart'));
var output = formatter.format( var output = formatter
await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); .format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected); expect(output).toEqual(expected);
}); });
it( it(
'should generate a single setter when multiple annotations bind to the ' 'should generate a single setter when multiple annotations bind to the '
'same `inputs` property.', () async { 'same `inputs` property.', () async {
var inputPath = var inputPath = 'duplicate_bind_name_files/soup.ng_deps.dart';
'bind_generator/duplicate_bind_name_files/soup.ng_deps.dart'; var expected =
var expected = formatter.format(readFile( _readFile('duplicate_bind_name_files/expected/soup.ng_deps.dart');
'bind_generator/duplicate_bind_name_files/expected/soup.ng_deps.dart'));
var output = formatter.format( var output = formatter
await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); .format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected); expect(output).toEqual(expected);
}); });
it('should generate setters for queries defined in the class annotation.', it('should generate setters for queries defined in the class annotation.',
() async { () async {
var inputPath = var inputPath = 'queries_class_annotation_files/bar.ng_deps.dart';
'bind_generator/queries_class_annotation_files/bar.ng_deps.dart'; var expected =
var expected = formatter.format(readFile( _readFile('queries_class_annotation_files/expected/bar.ng_deps.dart');
'bind_generator/queries_class_annotation_files/expected/bar.ng_deps.dart'));
var output = formatter.format( var output = formatter
await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); .format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected); expect(output).toEqual(expected);
}); });
it('should generate setters for queries defined via prop annotations.', it('should generate setters for queries defined via prop annotations.',
() async { () async {
var inputPath = var inputPath = 'queries_prop_annotations_files/bar.ng_deps.dart';
'bind_generator/queries_prop_annotations_files/bar.ng_deps.dart'; var expected =
var expected = formatter.format(readFile( _readFile('queries_prop_annotations_files/expected/bar.ng_deps.dart');
'bind_generator/queries_prop_annotations_files/expected/bar.ng_deps.dart'));
var output = formatter.format( var output = formatter
await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); .format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected);
});
it('should gracefully handle const objects as prop annotations.', () async {
var inputPath = 'queries_override_annotation_files/bar.ng_deps.dart';
var expected = formatter.format(_readFile(
'queries_override_annotation_files/expected/bar.ng_deps.dart'));
var output = formatter
.format(await createNgSettersAndGetters(reader, _assetId(inputPath)));
expect(output).toEqual(expected); expect(output).toEqual(expected);
}); });
} }
AssetId _assetId(String path) => new AssetId('a', 'bind_generator/$path');
String _readFile(String path) {
var code = readFile('bind_generator/$path');
if (path.endsWith('.dart')) {
code = formatter.format(code);
}
return code;
}

View File

@ -0,0 +1,21 @@
library bar.ng_deps.dart;
import 'bar.dart';
import 'package:angular2/src/core/metadata.dart';
var _visited = false;
void initReflector(reflector) {
if (_visited) return;
_visited = true;
reflector
..registerType(
ToolTip,
new ReflectionInfo(
const [const Directive(selector: '[tool-tip]')],
const [],
() => new ToolTip(),
null,
const {
'queryField': const [override]
}));
}

View File

@ -0,0 +1,21 @@
library bar.ng_deps.dart;
import 'bar.dart';
import 'package:angular2/src/core/metadata.dart';
var _visited = false;
void initReflector(reflector) {
if (_visited) return;
_visited = true;
reflector
..registerType(
ToolTip,
new ReflectionInfo(
const [const Directive(selector: '[tool-tip]')],
const [],
() => new ToolTip(),
null,
const {
'queryField': const [override]
}));
}

View File

@ -9,6 +9,7 @@ import 'package:angular2/src/core/linker/interfaces.dart' show LifecycleHooks;
import 'package:angular2/src/core/dom/html_adapter.dart'; import 'package:angular2/src/core/dom/html_adapter.dart';
import 'package:angular2/src/transform/directive_processor/rewriter.dart'; import 'package:angular2/src/transform/directive_processor/rewriter.dart';
import 'package:angular2/src/transform/common/annotation_matcher.dart'; import 'package:angular2/src/transform/common/annotation_matcher.dart';
import 'package:angular2/src/transform/common/code/ng_deps_code.dart';
import 'package:angular2/src/transform/common/asset_reader.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/common/logging.dart' as log;
import 'package:angular2/src/transform/common/model/reflection_info_model.pb.dart'; import 'package:angular2/src/transform/common/model/reflection_info_model.pb.dart';
@ -210,9 +211,32 @@ void allTests() {
expect(model.reflectables.first.propertyMetadata.isNotEmpty).toBeTrue(); expect(model.reflectables.first.propertyMetadata.isNotEmpty).toBeTrue();
expect(model.reflectables.first.propertyMetadata.first.name) expect(model.reflectables.first.propertyMetadata.first.name)
.toEqual('getVal'); .toEqual('getVal');
expect(model.reflectables.first.propertyMetadata.first.annotations
.firstWhere((a) => a.name == 'GetDecorator', orElse: () => null)) var getDecoratorAnnotation = model
.toBeNotNull(); .reflectables.first.propertyMetadata.first.annotations
.firstWhere((a) => a.name == 'GetDecorator', orElse: () => null);
expect(getDecoratorAnnotation).toBeNotNull();
expect(getDecoratorAnnotation.isConstObject).toBeFalse();
});
it('should gracefully handle const instances of annotations', () async {
// Regression test for i/4481
var model = await _testCreateModel('prop_metadata_files/override.dart');
expect(model.reflectables.first.propertyMetadata).toBeNotNull();
expect(model.reflectables.first.propertyMetadata.isNotEmpty).toBeTrue();
expect(model.reflectables.first.propertyMetadata.first.name)
.toEqual('getVal');
var overrideAnnotation = model
.reflectables.first.propertyMetadata.first.annotations
.firstWhere((a) => a.name == 'override', orElse: () => null);
expect(overrideAnnotation).toBeNotNull();
expect(overrideAnnotation.isConstObject).toBeTrue();
var buf = new StringBuffer();
new NgDepsWriter(buf).writeAnnotationModel(overrideAnnotation);
expect(buf.toString()).toEqual('override');
}); });
it('should be recorded on setters', () async { it('should be recorded on setters', () async {

View File

@ -0,0 +1,14 @@
library override;
import 'package:angular2/src/core/metadata.dart';
@Component(selector: '[getters]')
@View(template: '')
class FieldComponent implements ValGetter {
@override
String get getVal => 'a';
}
abstract class ValGetter {
String get getVal;
}