From 64e8f93f3232cb9fcde49d630fc807df3ae64485 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Tue, 15 Sep 2015 16:38:38 -0700 Subject: [PATCH] feat(dart/transform): Record property metadata Update the transformer to generate code registering annotations on class properties, getters, and setters. Closes #1800, #3267, #4003 --- .../common/code/reflection_info_code.dart | 125 ++++++++++++++++-- .../model/reflection_info_model.pb.dart | 46 ++++++- .../common/model/reflection_info_model.proto | 11 ++ .../directive_processor/all_tests.dart | 55 ++++++++ .../prop_metadata_files/fields.dart | 8 ++ .../prop_metadata_files/getters.dart | 8 ++ .../getters_and_setters.dart | 10 ++ .../prop_metadata_files/setters.dart | 8 ++ 8 files changed, 260 insertions(+), 11 deletions(-) create mode 100644 modules_dart/transform/test/transform/directive_processor/prop_metadata_files/fields.dart create mode 100644 modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters.dart create mode 100644 modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters_and_setters.dart create mode 100644 modules_dart/transform/test/transform/directive_processor/prop_metadata_files/setters.dart diff --git a/modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart b/modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart index 3a14d8b880..f426906324 100644 --- a/modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart +++ b/modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart @@ -5,6 +5,7 @@ import 'package:angular2/src/transform/common/annotation_matcher.dart'; import 'package:angular2/src/transform/common/logging.dart'; import 'package:angular2/src/transform/common/model/reflection_info_model.pb.dart'; import 'package:angular2/src/transform/common/names.dart'; +import 'package:angular2/src/transform/common/property_utils.dart'; import 'package:barback/barback.dart' show AssetId; import 'annotation_code.dart'; @@ -16,20 +17,26 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor { /// The file we are processing. final AssetId assetId; - final AnnotationVisitor _annotationVisitor; - final ParameterVisitor _parameterVisitor = new ParameterVisitor(); - - /// Whether an Angular 2 `Reflection` has been found. - bool _foundNgReflection = false; - /// Responsible for testing whether [Annotation]s are those recognized by /// Angular 2, for example `@Component`. final AnnotationMatcher _annotationMatcher; - ReflectionInfoVisitor(AssetId assetId, AnnotationMatcher annotationMatcher) - : this.assetId = assetId, - _annotationMatcher = annotationMatcher, - _annotationVisitor = new AnnotationVisitor(assetId, annotationMatcher); + final AnnotationVisitor _annotationVisitor; + final ParameterVisitor _parameterVisitor = new ParameterVisitor(); + final _PropertyMetadataVisitor _propMetadataVisitor; + + /// Whether an Angular 2 `Reflection` has been found. + bool _foundNgReflection = false; + + ReflectionInfoVisitor._(this.assetId, this._annotationMatcher, + this._annotationVisitor, this._propMetadataVisitor); + + factory ReflectionInfoVisitor( + AssetId assetId, AnnotationMatcher annotationMatcher) { + var annotationVisitor = new AnnotationVisitor(assetId, annotationMatcher); + return new ReflectionInfoVisitor._(assetId, annotationMatcher, + annotationVisitor, new _PropertyMetadataVisitor(annotationVisitor)); + } bool get shouldCreateNgDeps => _foundNgReflection; @@ -92,9 +99,44 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor { model.interfaces.addAll(node.implementsClause.interfaces .map((interface) => '${interface.name}')); } + + // Record annotations attached to properties. + for (var member in node.members) { + var propMetaList = member.accept(_propMetadataVisitor); + if (propMetaList != null) { + model.propertyMetadata.addAll(propMetaList); + } + } + _coalesce(model.propertyMetadata); + return model; } + // If a class has a getter & a setter with the same name and each has + // individual metadata, collapse to a single entry. + void _coalesce(List propertyMetadata) { + if (propertyMetadata.isEmpty) return; + + var firstSeenIdxMap = {}; + firstSeenIdxMap[propertyMetadata[0].name] = 0; + var i = 1; + while (i < propertyMetadata.length) { + var propName = propertyMetadata[i].name; + if (firstSeenIdxMap.containsKey(propName)) { + var propNameIdx = firstSeenIdxMap[propName]; + // We have seen this name before, combine the metadata lists. + propertyMetadata[propNameIdx] + .annotations + .addAll(propertyMetadata[i].annotations); + // Remove the higher index, okay since we directly check `length` above. + propertyMetadata.removeAt(i); + } else { + firstSeenIdxMap[propName] = i; + ++i; + } + } + } + @override ReflectionInfoModel visitFunctionDeclaration(FunctionDeclaration node) { if (!node.metadata @@ -126,6 +168,53 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor { } } +/// Visitor responsible for parsing [ClassMember]s into +/// [PropertyMetadataModel]s. +class _PropertyMetadataVisitor + extends SimpleAstVisitor> { + final AnnotationVisitor _annotationVisitor; + + _PropertyMetadataVisitor(this._annotationVisitor); + + @override + List visitFieldDeclaration(FieldDeclaration node) { + var retVal = null; + for (var variable in node.fields.variables) { + var propModel = new PropertyMetadataModel()..name = '${variable.name}'; + for (var meta in node.metadata) { + var annotationModel = meta.accept(_annotationVisitor); + if (annotationModel != null) { + propModel.annotations.add(annotationModel); + } + } + if (propModel.annotations.isNotEmpty) { + if (retVal == null) { + retVal = []; + } + retVal.add(propModel); + } + } + return retVal; + } + + @override + List visitMethodDeclaration(MethodDeclaration node) { + if (node.isGetter || node.isSetter) { + var propModel = new PropertyMetadataModel()..name = '${node.name}'; + for (var meta in node.metadata) { + var annotationModel = meta.accept(_annotationVisitor); + if (annotationModel != null) { + propModel.annotations.add(annotationModel); + } + } + if (propModel.annotations.isNotEmpty) { + return [propModel]; + } + } + return null; + } +} + /// Defines the format in which an [ReflectionInfoModel] is expressed as Dart /// code in a `.ng_deps.dart` file. abstract class ReflectionWriterMixin @@ -171,9 +260,25 @@ abstract class ReflectionWriterMixin _writeListWithSeparator(model.parameters, writeParameterModelForImpl, prefix: '(', suffix: ')'); // Interfaces + var hasPropertyMetadata = + model.propertyMetadata != null && model.propertyMetadata.isNotEmpty; if (model.interfaces != null && model.interfaces.isNotEmpty) { _writeListWithSeparator(model.interfaces, buffer.write, prefix: ',\nconst [', suffix: ']'); + } else if (hasPropertyMetadata) { + buffer.write(',\nconst []'); + } + // Property Metadata + if (hasPropertyMetadata) { + buffer.write(',\nconst {'); + for (var propMeta in model.propertyMetadata) { + if (propMeta != model.propertyMetadata.first) { + buffer.write(', '); + } + _writeListWithSeparator(propMeta.annotations, writeAnnotationModel, + prefix: "\n'${sanitize(propMeta.name)}': const [", suffix: ']'); + } + buffer.write('}'); } } buffer.writeln(')\n)'); diff --git a/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart b/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart index 59e754599c..d40efcb067 100644 --- a/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart +++ b/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart @@ -7,6 +7,38 @@ import 'package:protobuf/protobuf.dart'; import 'annotation_model.pb.dart'; import 'parameter_model.pb.dart'; +class PropertyMetadataModel extends GeneratedMessage { + static final BuilderInfo _i = new BuilderInfo('PropertyMetadataModel') + ..a(1, 'name', PbFieldType.QS) + ..pp(2, 'annotations', PbFieldType.PM, AnnotationModel.$checkItem, AnnotationModel.create) + ; + + PropertyMetadataModel() : super(); + PropertyMetadataModel.fromBuffer(List i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromBuffer(i, r); + PropertyMetadataModel.fromJson(String i, [ExtensionRegistry r = ExtensionRegistry.EMPTY]) : super.fromJson(i, r); + PropertyMetadataModel clone() => new PropertyMetadataModel()..mergeFromMessage(this); + BuilderInfo get info_ => _i; + static PropertyMetadataModel create() => new PropertyMetadataModel(); + static PbList createRepeated() => new PbList(); + static PropertyMetadataModel getDefault() { + if (_defaultInstance == null) _defaultInstance = new _ReadonlyPropertyMetadataModel(); + return _defaultInstance; + } + static PropertyMetadataModel _defaultInstance; + static void $checkItem(PropertyMetadataModel v) { + if (v is !PropertyMetadataModel) checkItemFailed(v, 'PropertyMetadataModel'); + } + + String get name => getField(1); + void set name(String v) { setField(1, v); } + bool hasName() => hasField(1); + void clearName() => clearField(1); + + List get annotations => getField(2); +} + +class _ReadonlyPropertyMetadataModel extends PropertyMetadataModel with ReadonlyMessageMixin {} + class ReflectionInfoModel extends GeneratedMessage { static final BuilderInfo _i = new BuilderInfo('ReflectionInfoModel') ..a(1, 'name', PbFieldType.QS) @@ -15,6 +47,7 @@ class ReflectionInfoModel extends GeneratedMessage { ..pp(4, 'annotations', PbFieldType.PM, AnnotationModel.$checkItem, AnnotationModel.create) ..pp(5, 'parameters', PbFieldType.PM, ParameterModel.$checkItem, ParameterModel.create) ..p(6, 'interfaces', PbFieldType.PS) + ..pp(7, 'propertyMetadata', PbFieldType.PM, PropertyMetadataModel.$checkItem, PropertyMetadataModel.create) ; ReflectionInfoModel() : super(); @@ -53,10 +86,20 @@ class ReflectionInfoModel extends GeneratedMessage { List get parameters => getField(5); List get interfaces => getField(6); + + List get propertyMetadata => getField(7); } class _ReadonlyReflectionInfoModel extends ReflectionInfoModel with ReadonlyMessageMixin {} +const PropertyMetadataModel$json = const { + '1': 'PropertyMetadataModel', + '2': const [ + const {'1': 'name', '3': 1, '4': 2, '5': 9}, + const {'1': 'annotations', '3': 2, '4': 3, '5': 11, '6': '.angular2.src.transform.common.model.proto.AnnotationModel'}, + ], +}; + const ReflectionInfoModel$json = const { '1': 'ReflectionInfoModel', '2': const [ @@ -66,12 +109,13 @@ const ReflectionInfoModel$json = const { const {'1': 'annotations', '3': 4, '4': 3, '5': 11, '6': '.angular2.src.transform.common.model.proto.AnnotationModel'}, const {'1': 'parameters', '3': 5, '4': 3, '5': 11, '6': '.angular2.src.transform.common.model.proto.ParameterModel'}, const {'1': 'interfaces', '3': 6, '4': 3, '5': 9}, + const {'1': 'propertyMetadata', '3': 7, '4': 3, '5': 11, '6': '.angular2.src.transform.common.model.proto.PropertyMetadataModel'}, ], }; /** * Generated with: - * reflection_info_model.proto (fd01d8a29e6bccccc343ef975829fd7cb6a63312) + * reflection_info_model.proto (71d723738054f1276f792a2672a956ef9be94a4c) * libprotoc 2.5.0 * dart-protoc-plugin (cc35f743de982a4916588b9c505dd21c7fe87d17) */ diff --git a/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto b/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto index 2ff1e11992..589c07c8e2 100644 --- a/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto +++ b/modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto @@ -5,6 +5,14 @@ import "parameter_model.proto"; package angular2.src.transform.common.model.proto; +message PropertyMetadataModel { + // The name of the property with metadata attached. + required string name = 1; + + // The metadata attached to the property. + repeated AnnotationModel annotations = 2; +} + message ReflectionInfoModel { // The (potentially prefixed) name of this Injectable. // This can be a `Type` or a function name. @@ -21,4 +29,7 @@ message ReflectionInfoModel { repeated ParameterModel parameters = 5; repeated string interfaces = 6; + + // Entries for all properties with associated metadata. + repeated PropertyMetadataModel propertyMetadata = 7; } 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 812c8ec64f..a87a486c3e 100644 --- a/modules_dart/transform/test/transform/directive_processor/all_tests.dart +++ b/modules_dart/transform/test/transform/directive_processor/all_tests.dart @@ -292,6 +292,61 @@ void allTests() { }); }); + describe('property metadata', () { + it('should be recorded on fields', () async { + var model = await _testCreateModel('prop_metadata_files/fields.dart'); + + expect(model.reflectables.first.propertyMetadata).toBeNotNull(); + expect(model.reflectables.first.propertyMetadata.isNotEmpty).toBeTrue(); + expect(model.reflectables.first.propertyMetadata.first.name) + .toEqual('field'); + expect(model.reflectables.first.propertyMetadata.first.annotations + .firstWhere((a) => a.name == 'FieldDecorator', + orElse: () => null)).toBeNotNull(); + }); + + it('should be recorded on getters', () async { + var model = await _testCreateModel('prop_metadata_files/getters.dart'); + + expect(model.reflectables.first.propertyMetadata).toBeNotNull(); + 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(); + }); + + it('should be recorded on setters', () async { + var model = await _testCreateModel('prop_metadata_files/setters.dart'); + + expect(model.reflectables.first.propertyMetadata).toBeNotNull(); + expect(model.reflectables.first.propertyMetadata.isNotEmpty).toBeTrue(); + expect(model.reflectables.first.propertyMetadata.first.name) + .toEqual('setVal'); + expect(model.reflectables.first.propertyMetadata.first.annotations + .firstWhere((a) => a.name == 'SetDecorator', orElse: () => null)) + .toBeNotNull(); + }); + + it('should be coalesced when getters and setters have the same name', + () async { + var model = await _testCreateModel( + 'prop_metadata_files/getters_and_setters.dart'); + + expect(model.reflectables.first.propertyMetadata).toBeNotNull(); + expect(model.reflectables.first.propertyMetadata.length).toBe(1); + expect(model.reflectables.first.propertyMetadata.first.name) + .toEqual('myVal'); + expect(model.reflectables.first.propertyMetadata.first.annotations + .firstWhere((a) => a.name == 'GetDecorator', orElse: () => null)) + .toBeNotNull(); + expect(model.reflectables.first.propertyMetadata.first.annotations + .firstWhere((a) => a.name == 'SetDecorator', orElse: () => null)) + .toBeNotNull(); + }); + }); + it('should not throw/hang on invalid urls', () async { var logger = new RecordingLogger(); var model = diff --git a/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/fields.dart b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/fields.dart new file mode 100644 index 0000000000..6433e5c178 --- /dev/null +++ b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/fields.dart @@ -0,0 +1,8 @@ +library fields; + +import 'package:angular2/src/core/metadata.dart'; + +@Component(selector: '[fields]') +class FieldComponent { + @FieldDecorator("field") String field; +} diff --git a/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters.dart b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters.dart new file mode 100644 index 0000000000..02caa3bc8e --- /dev/null +++ b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters.dart @@ -0,0 +1,8 @@ +library fields; + +import 'package:angular2/src/core/metadata.dart'; + +@Component(selector: '[getters]') +class FieldComponent { + @GetDecorator("get") String get getVal => 'a'; +} diff --git a/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters_and_setters.dart b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters_and_setters.dart new file mode 100644 index 0000000000..425523da9e --- /dev/null +++ b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/getters_and_setters.dart @@ -0,0 +1,10 @@ +library fields; + +import 'package:angular2/src/core/metadata.dart'; + +@Component(selector: '[getters-and-setters]') +class FieldComponent { + String _val; + @GetDecorator("get") String get myVal => _val; + @SetDecorator("set") String set myVal(val) => _val = val; +} diff --git a/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/setters.dart b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/setters.dart new file mode 100644 index 0000000000..88dd7f921f --- /dev/null +++ b/modules_dart/transform/test/transform/directive_processor/prop_metadata_files/setters.dart @@ -0,0 +1,8 @@ +library fields; + +import 'package:angular2/src/core/metadata.dart'; + +@Component(selector: '[setters]') +class FieldComponent { + @SetDecorator("set") String set setVal(val) => null; +}