diff --git a/modules_dart/transform/lib/src/transform/bind_generator/generator.dart b/modules_dart/transform/lib/src/transform/bind_generator/generator.dart index d8422e1cfa..df9285ac39 100644 --- a/modules_dart/transform/lib/src/transform/bind_generator/generator.dart +++ b/modules_dart/transform/lib/src/transform/bind_generator/generator.dart @@ -50,6 +50,7 @@ class _ExtractQueryFieldsFromPropMetadata extends Object bool _hasQueryAnnotation(list) { var res = false; list.elements.forEach((item) { + if (item is! InstanceCreationExpression) return; var n = item.constructorName.toString(); if (n == "ContentChild" || n == "ViewChild" || diff --git a/modules_dart/transform/lib/src/transform/common/code/annotation_code.dart b/modules_dart/transform/lib/src/transform/common/code/annotation_code.dart index 76e10fc6d7..30195f5428 100644 --- a/modules_dart/transform/lib/src/transform/common/code/annotation_code.dart +++ b/modules_dart/transform/lib/src/transform/common/code/annotation_code.dart @@ -36,9 +36,12 @@ class AnnotationVisitor extends SimpleAstVisitor { ..isComponent = isComponent ..isDirective = isDirective ..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) { if (arg is NamedExpression) { model.namedParameters.add(new NamedParameter() @@ -60,7 +63,11 @@ abstract class AnnotationWriterMixin { StringBuffer get buffer; 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}('); var first = true; for (var param in model.parameters) { @@ -83,10 +90,6 @@ abstract class AnnotationWriterMixin { buffer.write('${param.name}: ${param.value}'); } 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); } } } diff --git a/modules_dart/transform/lib/src/transform/common/model/annotation_model.pb.dart b/modules_dart/transform/lib/src/transform/common/model/annotation_model.pb.dart index 14d72c9f9c..be555c8d4f 100644 --- a/modules_dart/transform/lib/src/transform/common/model/annotation_model.pb.dart +++ b/modules_dart/transform/lib/src/transform/common/model/annotation_model.pb.dart @@ -8,67 +8,91 @@ import 'package:protobuf/protobuf.dart'; class NamedParameter extends GeneratedMessage { static final BuilderInfo _i = new BuilderInfo('NamedParameter') ..a(1, 'name', PbFieldType.QS) - ..a(2, 'value', PbFieldType.QS) - ; + ..a(2, 'value', PbFieldType.QS); NamedParameter() : super(); - NamedParameter.fromBuffer(List i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r); - NamedParameter.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r); + NamedParameter.fromBuffer(List i, + [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); BuilderInfo get info_ => _i; static NamedParameter create() => new NamedParameter(); - static PbList createRepeated() => new PbList(); + static PbList createRepeated() => + new PbList(); static NamedParameter getDefault() { - if (_defaultInstance == null) _defaultInstance = new _ReadonlyNamedParameter(); + if (_defaultInstance == null) _defaultInstance = + new _ReadonlyNamedParameter(); return _defaultInstance; } + static NamedParameter _defaultInstance; static void $checkItem(NamedParameter v) { - if (v is !NamedParameter) checkItemFailed(v, 'NamedParameter'); + if (v is! NamedParameter) checkItemFailed(v, 'NamedParameter'); } 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); void clearName() => clearField(1); 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); void clearValue() => clearField(2); } -class _ReadonlyNamedParameter extends NamedParameter with ReadonlyMessageMixin {} +class _ReadonlyNamedParameter extends NamedParameter with ReadonlyMessageMixin { +} class AnnotationModel extends GeneratedMessage { static final BuilderInfo _i = new BuilderInfo('AnnotationModel') ..a(1, 'name', PbFieldType.QS) ..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(5, 'isDirective', PbFieldType.OB) ..a(6, 'isComponent', PbFieldType.OB) ..a(7, 'isInjectable', PbFieldType.OB) - ; + ..a(8, 'isConstObject', PbFieldType.OB); AnnotationModel() : super(); - AnnotationModel.fromBuffer(List i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r); - AnnotationModel.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r); + AnnotationModel.fromBuffer(List i, + [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); BuilderInfo get info_ => _i; static AnnotationModel create() => new AnnotationModel(); - static PbList createRepeated() => new PbList(); + static PbList createRepeated() => + new PbList(); static AnnotationModel getDefault() { - if (_defaultInstance == null) _defaultInstance = new _ReadonlyAnnotationModel(); + if (_defaultInstance == null) _defaultInstance = + new _ReadonlyAnnotationModel(); return _defaultInstance; } + static AnnotationModel _defaultInstance; static void $checkItem(AnnotationModel v) { - if (v is !AnnotationModel) checkItemFailed(v, 'AnnotationModel'); + if (v is! AnnotationModel) checkItemFailed(v, 'AnnotationModel'); } 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); void clearName() => clearField(1); @@ -77,27 +101,48 @@ class AnnotationModel extends GeneratedMessage { List get namedParameters => getField(3); 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); void clearIsView() => clearField(4); 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); void clearIsDirective() => clearField(5); 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); void clearIsComponent() => clearField(6); 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); 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 { '1': 'NamedParameter', @@ -112,17 +157,24 @@ const AnnotationModel$json = const { '2': const [ const {'1': 'name', '3': 1, '4': 2, '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_directive', '3': 5, '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_const_object', '3': 8, '4': 1, '5': 8}, ], }; /** * Generated with: - * annotation_model.proto (5ca037df4c4d3e5d2771a177c9f5ea9dc8ae5f91) + * annotation_model.proto (93cb7c1fba2e56d937fec054b6e119a2a2b9afe7) * libprotoc 2.5.0 * dart-protoc-plugin (cc35f743de982a4916588b9c505dd21c7fe87d17) */ diff --git a/modules_dart/transform/lib/src/transform/common/model/annotation_model.proto b/modules_dart/transform/lib/src/transform/common/model/annotation_model.proto index c02a72aa37..e1bb351603 100644 --- a/modules_dart/transform/lib/src/transform/common/model/annotation_model.proto +++ b/modules_dart/transform/lib/src/transform/common/model/annotation_model.proto @@ -25,11 +25,16 @@ message AnnotationModel { // account, that is, this should be true if `is_component` is true. optional bool is_directive = 5; - // Whether htis is a `Component` annotation. + // Whether this is a `Component` annotation. optional bool is_component = 6; // Whether this is an `Injectable` annotation. This takes inheritance into // account, that is, this should be true if `is_directive` and/or // `is_component` is true. 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; } diff --git a/modules_dart/transform/test/transform/bind_generator/all_tests.dart b/modules_dart/transform/test/transform/bind_generator/all_tests.dart index 09e09ecd4b..a098401a70 100644 --- a/modules_dart/transform/test/transform/bind_generator/all_tests.dart +++ b/modules_dart/transform/test/transform/bind_generator/all_tests.dart @@ -16,49 +16,65 @@ void allTests() { it('should generate a setter for an `inputs` property in an annotation.', () async { - var inputPath = 'bind_generator/basic_bind_files/bar.ng_deps.dart'; - var expected = formatter.format( - readFile('bind_generator/basic_bind_files/expected/bar.ng_deps.dart')); + var inputPath = 'basic_bind_files/bar.ng_deps.dart'; + var expected = _readFile('basic_bind_files/expected/bar.ng_deps.dart'); - var output = formatter.format( - await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); + var output = formatter + .format(await createNgSettersAndGetters(reader, _assetId(inputPath))); expect(output).toEqual(expected); }); it( 'should generate a single setter when multiple annotations bind to the ' 'same `inputs` property.', () async { - var inputPath = - 'bind_generator/duplicate_bind_name_files/soup.ng_deps.dart'; - var expected = formatter.format(readFile( - 'bind_generator/duplicate_bind_name_files/expected/soup.ng_deps.dart')); + var inputPath = 'duplicate_bind_name_files/soup.ng_deps.dart'; + var expected = + _readFile('duplicate_bind_name_files/expected/soup.ng_deps.dart'); - var output = formatter.format( - await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); + var output = formatter + .format(await createNgSettersAndGetters(reader, _assetId(inputPath))); expect(output).toEqual(expected); }); it('should generate setters for queries defined in the class annotation.', () async { - var inputPath = - 'bind_generator/queries_class_annotation_files/bar.ng_deps.dart'; - var expected = formatter.format(readFile( - 'bind_generator/queries_class_annotation_files/expected/bar.ng_deps.dart')); + var inputPath = 'queries_class_annotation_files/bar.ng_deps.dart'; + var expected = + _readFile('queries_class_annotation_files/expected/bar.ng_deps.dart'); - var output = formatter.format( - await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); + var output = formatter + .format(await createNgSettersAndGetters(reader, _assetId(inputPath))); expect(output).toEqual(expected); }); it('should generate setters for queries defined via prop annotations.', () async { - var inputPath = - 'bind_generator/queries_prop_annotations_files/bar.ng_deps.dart'; - var expected = formatter.format(readFile( - 'bind_generator/queries_prop_annotations_files/expected/bar.ng_deps.dart')); + var inputPath = 'queries_prop_annotations_files/bar.ng_deps.dart'; + var expected = + _readFile('queries_prop_annotations_files/expected/bar.ng_deps.dart'); - var output = formatter.format( - await createNgSettersAndGetters(reader, new AssetId('a', inputPath))); + var output = formatter + .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); }); } + +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; +} diff --git a/modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/bar.ng_deps.dart b/modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/bar.ng_deps.dart new file mode 100644 index 0000000000..b15e446d47 --- /dev/null +++ b/modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/bar.ng_deps.dart @@ -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] + })); +} diff --git a/modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/expected/bar.ng_deps.dart b/modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/expected/bar.ng_deps.dart new file mode 100644 index 0000000000..b15e446d47 --- /dev/null +++ b/modules_dart/transform/test/transform/bind_generator/queries_override_annotation_files/expected/bar.ng_deps.dart @@ -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] + })); +} diff --git a/modules_dart/transform/test/transform/directive_processor/all_tests.dart b/modules_dart/transform/test/transform/directive_processor/all_tests.dart index 214196dfc7..b376d8b8d3 100644 --- a/modules_dart/transform/test/transform/directive_processor/all_tests.dart +++ b/modules_dart/transform/test/transform/directive_processor/all_tests.dart @@ -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/transform/directive_processor/rewriter.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/logging.dart' as log; 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.first.name) .toEqual('getVal'); - expect(model.reflectables.first.propertyMetadata.first.annotations - .firstWhere((a) => a.name == 'GetDecorator', orElse: () => null)) - .toBeNotNull(); + + var getDecoratorAnnotation = model + .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 { diff --git a/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/override.dart b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/override.dart new file mode 100644 index 0000000000..4bdb4cc057 --- /dev/null +++ b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/override.dart @@ -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; +}