Skip to content

Commit 1e3cd02

Browse files
committed
fix(StaticMetadataExtractor): Map members annotations to all annotations
Closes dart-archive#904
1 parent 6d6e9ee commit 1e3cd02

File tree

4 files changed

+82
-73
lines changed

4 files changed

+82
-73
lines changed

lib/tools/source_metadata_extractor.dart

+11-13
Original file line numberDiff line numberDiff line change
@@ -125,14 +125,15 @@ class DirectiveMetadataCollectingVisitor {
125125
// Ignore non-class annotations.
126126
// TODO(pavelj): this is not a safe check for the type of the
127127
// annotations, but good enough for now.
128-
if (ann.name.name != 'NgComponent'
129-
&& ann.name.name != 'NgDirective') return;
128+
if (ann.name.name != 'NgComponent' && ann.name.name != 'NgDirective') {
129+
return;
130+
}
130131

131132
bool isComponent = ann.name.name == 'NgComponent';
132133

133134
meta = new DirectiveMetadata()
134-
..className = clazz.name.name
135-
..type = isComponent ? COMPONENT : DIRECTIVE;
135+
..className = clazz.name.name
136+
..type = isComponent ? COMPONENT : DIRECTIVE;
136137
metadata.add(meta);
137138

138139
ann.arguments.arguments.forEach((Expression arg) {
@@ -175,20 +176,17 @@ class DirectiveMetadataCollectingVisitor {
175176
mbr.metadata.forEach((Annotation ann) {
176177
if (_attrAnnotationsToSpec.containsKey(ann.name.name)) {
177178
String fieldName;
178-
if (mbr is FieldDeclaration) {
179-
fieldName = mbr.fields.variables.first.name.name;
180-
} else {
181-
// MethodDeclaration
182-
fieldName = (mbr as MethodDeclaration).name.name;
183-
}
179+
fieldName == mbr is FieldDeclaration ?
180+
mbr.fields.variables.first.name.name :
181+
(mbr as MethodDeclaration).name.name;
184182
StringLiteral attNameLiteral = ann.arguments.arguments.first;
185183
if (meta.attributeMappings
186-
.containsKey(attNameLiteral.stringValue)) {
184+
.containsKey(attNameLiteral.stringValue)) {
187185
throw 'Attribute mapping already defined for '
188-
'${clazz.name}.$fieldName';
186+
'${clazz.name}.$fieldName';
189187
}
190188
meta.attributeMappings[attNameLiteral.stringValue] =
191-
_attrAnnotationsToSpec[ann.name.name] + fieldName;
189+
o_attrAnnotationsToSpec[ann.name.name] + fieldName;
192190
}
193191
});
194192
}

lib/tools/transformer/expression_generator.dart

+14-12
Original file line numberDiff line numberDiff line change
@@ -84,26 +84,28 @@ class ExpressionGenerator extends Transformer with ResolverTransformer {
8484
// Get all of the contents of templates in @NgComponent(templateUrl:'...')
8585
gatherReferencedUris(transform, resolver, options, templatesOnly: true)
8686
.then((templates) {
87-
templates.values.forEach(controller.add);})
87+
templates.values.forEach(controller.add);
88+
})
8889
.then((_) {
8990
// Add any HTML files referencing this Dart file.
90-
return _findHtmlEntry(transform);})
91+
return _findHtmlEntry(transform);
92+
})
9193
.then((htmlRefId) {
9294
if (htmlRefId != null) assets.add(htmlRefId);
93-
Future.wait(
94-
// Add any manually specified HTML files.
95-
assets
96-
.map((id) => transform.readInputAsString(id))
97-
.map((future) =>
98-
future
95+
Future
96+
.wait(
97+
// Add any manually specified HTML files.
98+
assets
99+
.map((id) => transform.readInputAsString(id))
100+
.map((future) => future
99101
.then(controller.add)
100102
.catchError((e) {
101103
transform.logger.warning('Unable to find $id from '
102-
'html_files in pubspec.yaml.');
104+
t'html_files in pubspec.yaml.');
103105
})))
104-
.then((_) {
105-
controller.close();
106-
});
106+
.then((_) {
107+
controller.close();
108+
});
107109
});
108110

109111
return controller.stream;

lib/tools/transformer/metadata_extractor.dart

+44-45
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ class AnnotatedType {
3232
Resolver resolver, Map<LibraryElement, String> prefixes) {
3333
sink.write(' ${prefixes[type.library]}${type.name}: const [\n');
3434
var writer = new _AnnotationWriter(sink, prefixes);
35-
annotations.forEach((annotation) {
35+
for (var annotation in annotations) {
3636
sink.write(' ');
3737
if (writer.writeAnnotation(annotation)) {
3838
sink.write(',\n');
@@ -42,7 +42,7 @@ class AnnotatedType {
4242
asset: resolver.getSourceAssetId(annotation.parent.element),
4343
span: resolver.getSourceSpan(annotation.parent.element));
4444
}
45-
});
45+
}
4646
sink.write(' ],\n');
4747
}
4848
}
@@ -325,53 +325,52 @@ class AnnotationExtractor {
325325
return;
326326
}
327327

328-
// Default to using the first acceptable annotation- not sure if
329-
// more than one should ever occur.
330-
var sourceAnnotation = acceptableAnnotations.first;
331-
332-
// Clone the annotation so we don't modify the one in the persistent AST.
333-
var index = type.annotations.indexOf(sourceAnnotation);
334-
var annotation = new AstCloner().visitAnnotation(sourceAnnotation);
335-
ResolutionCopier.copyResolutionData(sourceAnnotation, annotation);
336-
type.annotations[index] = annotation;
337-
338-
var mapArg = annotation.arguments.arguments.firstWhere(
339-
(arg) => (arg is NamedExpression) && (arg.name.label.name == 'map'),
340-
orElse: () => null);
341-
342-
// If we don't have a 'map' parameter yet, add one.
343-
if (mapArg == null) {
344-
var map = new MapLiteral(null, null, null, [], null);
345-
var label = new Label(new SimpleIdentifier(
346-
new _GeneratedToken(TokenType.STRING, 'map')),
347-
new _GeneratedToken(TokenType.COLON, ':'));
348-
mapArg = new NamedExpression(label, map);
349-
annotation.arguments.arguments.add(mapArg);
350-
}
328+
// Merge attribute annotations in all of the class annotations
329+
acceptableAnnotations.forEach((srcAnnotation) {
330+
// Clone the annotation so we don't modify the one in the persistent AST.
331+
var index = type.annotations.indexOf(srcAnnotation);
332+
var annotation = new AstCloner().visitAnnotation(srcAnnotation);
333+
ResolutionCopier.copyResolutionData(srcAnnotation, annotation);
334+
type.annotations[index] = annotation;
335+
336+
var mapArg = annotation.arguments.arguments.firstWhere(
337+
(arg) => (arg is NamedExpression) && (arg.name.label.name == 'map'),
338+
orElse: () => null);
339+
340+
// If we don't have a 'map' parameter yet, add one.
341+
if (mapArg == null) {
342+
var map = new MapLiteral(null, null, null, [], null);
343+
var label = new Label(new SimpleIdentifier(
344+
new _GeneratedToken(TokenType.STRING, 'map')),
345+
new _GeneratedToken(TokenType.COLON, ':'));
346+
mapArg = new NamedExpression(label, map);
347+
annotation.arguments.arguments.add(mapArg);
348+
}
351349

352-
var map = mapArg.expression;
353-
if (map is! MapLiteral) {
354-
warn('Expected \'map\' argument of $annotation to be a map literal',
355-
type.type);
356-
return;
357-
}
358-
memberAnnotations.forEach((memberName, annotation) {
359-
var key = annotation.arguments.arguments.first;
360-
// If the key already exists then it means we have two annotations for
361-
// same member.
362-
if (map.entries.any((entry) => entry.key.toString() == key.toString())) {
363-
warn('Directive $annotation already contains an entry for $key',
364-
type.type);
350+
var map = mapArg.expression;
351+
if (map is! MapLiteral) {
352+
warn('Expected \'map\' argument of $annotation to be a map literal',
353+
type.type);
365354
return;
366355
}
356+
memberAnnotations.forEach((memberName, annotation) {
357+
var key = annotation.arguments.arguments.first;
358+
// If the key already exists then it means we have two annotations for
359+
// same member.
360+
if (map.entries.any((entry) => entry.key.toString() == key.toString())) {
361+
warn('Directive $annotation already contains an entry for $key',
362+
type.type);
363+
return;
364+
}
367365

368-
var typeName = annotation.element.enclosingElement.name;
369-
var value = '${_annotationToMapping[typeName]}$memberName';
370-
var entry = new MapLiteralEntry(
371-
key,
372-
new _GeneratedToken(TokenType.COLON, ':'),
373-
new SimpleStringLiteral(stringToken(value), value));
374-
map.entries.add(entry);
366+
var typeName = annotation.element.enclosingElement.name;
367+
var value = '${_annotationToMapping[typeName]}$memberName';
368+
var entry = new MapLiteralEntry(
369+
key,
370+
new _GeneratedToken(TokenType.COLON, ':'),
371+
new SimpleStringLiteral(stringToken(value), value));
372+
map.entries.add(entry);
373+
});
375374
});
376375
}
377376

test/tools/transformer/metadata_generator_spec.dart

+13-3
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,12 @@ main() {
115115
'a|web/main.dart': '''
116116
import 'package:angular/angular.dart';
117117
118-
@NgDirective(map: {'another-expression': '=>anotherExpression'})
118+
@NgDirective(
119+
selector: 'first',
120+
map: {'first-expression': '=>anotherExpression'})
121+
@NgDirective(
122+
selector: 'second',
123+
map: {'second-expression': '=>anotherExpression'})
119124
class Engine {
120125
set anotherExpression(Function) {}
121126
@@ -132,8 +137,13 @@ main() {
132137
],
133138
classes: {
134139
'import_0.Engine': [
135-
'const import_1.NgDirective(map: const {'
136-
'\'another-expression\': \'=>anotherExpression\', '
140+
'const import_1.NgDirective(selector: \'first\', '
141+
'map: const {'
142+
'\'first-expression\': \'=>anotherExpression\', '
143+
'\'two-way-stuff\': \'<=>twoWayStuff\'})',
144+
'const import_1.NgDirective(selector: \'second\', '
145+
'map: const {'
146+
'\'second-expression\': \'=>anotherExpression\', '
137147
'\'two-way-stuff\': \'<=>twoWayStuff\'})',
138148
]
139149
});

0 commit comments

Comments
 (0)