Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

Commit

Permalink
feat(annotation): Annotations on superclasses are honored
Browse files Browse the repository at this point in the history
This patch supports the following scenario:

```
class Base {
  @Ngattr('foo')
  var foo;
}

@ngdirective(selector:'my-directive')
class MyDirective extends Base {
  @Ngattr('bar')
  var bar;
}
```

MyDirective will now have both properties, foo and bar.

Closes #829
  • Loading branch information
mvuksano authored and mhevery committed Apr 18, 2014
1 parent f79696d commit eee4191
Show file tree
Hide file tree
Showing 8 changed files with 208 additions and 60 deletions.
27 changes: 16 additions & 11 deletions lib/core/registry_dynamic.dart
Original file line number Diff line number Diff line change
Expand Up @@ -57,24 +57,29 @@ class DynamicMetadataExtractor implements MetadataExtractor {


Map<String, DirectiveAnnotation> fieldMetadataExtractor(Type type) =>
_fieldMetadataCache.putIfAbsent(type, () => _fieldMetadataExtractor(type));
_fieldMetadataCache.putIfAbsent(type, () => _fieldMetadataExtractor(reflectType(type)));

Map<String, DirectiveAnnotation> _fieldMetadataExtractor(Type type) {
ClassMirror cm = reflectType(type);
final fields = <String, DirectiveAnnotation>{};
cm.declarations.forEach((Symbol name, DeclarationMirror decl) {
if (decl is VariableMirror ||
decl is MethodMirror && (decl.isGetter || decl.isSetter)) {
var fieldName = MirrorSystem.getName(name);
if (decl is MethodMirror && decl.isSetter) {
Map<String, DirectiveAnnotation> _fieldMetadataExtractor(ClassMirror cm) {
var fields = <String, DirectiveAnnotation>{};
if(cm.superclass != null) {
fields.addAll(_fieldMetadataExtractor(cm.superclass));
} else {
fields = {};
}
Map<Symbol, DeclarationMirror> declarations = cm.declarations;
declarations.forEach((symbol, dm) {
if(dm is VariableMirror ||
dm is MethodMirror && (dm.isGetter || dm.isSetter)) {
var fieldName = MirrorSystem.getName(symbol);
if (dm is MethodMirror && dm.isSetter) {
// Remove "=" from the end of the setter.
fieldName = fieldName.substring(0, fieldName.length - 1);
}
decl.metadata.forEach((InstanceMirror meta) {
dm.metadata.forEach((InstanceMirror meta) {
if (_fieldAnnotations.contains(meta.type)) {
if (fields.containsKey(fieldName)) {
throw 'Attribute annotation for $fieldName is defined more '
'than once in $type';
'than once in ${cm.reflectedType}';
}
fields[fieldName] = meta.reflectee as DirectiveAnnotation;
}
Expand Down
68 changes: 43 additions & 25 deletions lib/tools/source_metadata_extractor.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
library angular.source_metadata_extractor ;

import 'package:analyzer/src/generated/ast.dart';
import 'package:analyzer/src/generated/element.dart';

import 'package:angular/tools/source_crawler.dart';
import 'package:angular/tools/common.dart';
Expand Down Expand Up @@ -181,34 +182,51 @@ class DirectiveMetadataCollectingAstVisitor extends RecursiveAstVisitor {
}
});

// Check fields/getters/setter for presense of attr mapping annotations.
clazz.members.forEach((ClassMember member) {
if (member is FieldDeclaration ||
(member is MethodDeclaration &&
(member.isSetter || member.isGetter))) {
member.metadata.forEach((Annotation ann) {
if (_attrAnnotationsToSpec.containsKey(ann.name.name)) {
String fieldName;
if (member is FieldDeclaration) {
fieldName = member.fields.variables.first.name.name;
} else { // MethodDeclaration
fieldName = (member as MethodDeclaration).name.name;
}
StringLiteral attNameLiteral = ann.arguments.arguments.first;
if (meta.attributeMappings
.containsKey(attNameLiteral.stringValue)) {
throw 'Attribute mapping already defined for '
'${clazz.name}.$fieldName';
}
meta.attributeMappings[attNameLiteral.stringValue] =
_attrAnnotationsToSpec[ann.name.name] + fieldName;
}
});
}
});
if (meta != null) _walkSuperclassChain(clazz, meta, _extractMappingsFromClass);
});

return super.visitClassDeclaration(clazz);
}

_walkSuperclassChain(ClassDeclaration clazz, DirectiveMetadata meta,
metadataExtractor(ClassDeclaration clazz, DirectiveMetadata meta)) {
while (clazz != null) {
metadataExtractor(clazz, meta);
if (clazz.element != null && clazz.element.supertype != null) {
clazz = clazz.element.supertype.element.node;
} else {
clazz = null;
}
}
}

_extractMappingsFromClass(ClassDeclaration clazz, DirectiveMetadata meta) {
// Check fields/getters/setter for presence of attr mapping annotations.
clazz.members.forEach((ClassMember member) {
if (member is FieldDeclaration ||
(member is MethodDeclaration &&
(member.isSetter || member.isGetter))) {
member.metadata.forEach((Annotation ann) {
if (_attrAnnotationsToSpec.containsKey(ann.name.name)) {
String fieldName;
if (member is FieldDeclaration) {
fieldName = member.fields.variables.first.name.name;
} else { // MethodDeclaration
fieldName = (member as MethodDeclaration).name.name;
}
StringLiteral attNameLiteral = ann.arguments.arguments.first;
if (meta.attributeMappings
.containsKey(attNameLiteral.stringValue)) {
throw 'Attribute mapping already defined for '
'${clazz.name}.$fieldName';
}
meta.attributeMappings[attNameLiteral.stringValue] =
_attrAnnotationsToSpec[ann.name.name] + fieldName;
}
});
}
});
}
}

class DirectiveMetadataCollectingVisitor {
Expand Down
38 changes: 23 additions & 15 deletions lib/tools/transformer/metadata_extractor.dart
Original file line number Diff line number Diff line change
Expand Up @@ -242,20 +242,31 @@ class AnnotationExtractor {

/// Extracts all of the annotations for the specified class.
AnnotatedType extractAnnotations(ClassElement cls) {
if (resolver.getImportUri(cls.library, from: outputId) == null) {
warn('Dropping annotations for ${cls.name} because the '
'containing file cannot be imported (must be in a lib folder).', cls);
return null;
}

var classElement = cls;
var visitor = new _AnnotationVisitor(_annotationElements);
cls.node.accept(visitor);
while (classElement != null) {
if (resolver.getImportUri(classElement.library, from: outputId) == null) {
warn('Dropping annotations for ${classElement.name} because the '
'containing file cannot be imported (must be in a lib folder).', classElement);
return null;
}
if (classElement.node != null) {
classElement.node.accept(visitor);
}

if (classElement.supertype != null) {
visitor.visitingSupertype = true;
classElement = classElement.supertype.element;
} else {
classElement = null;
}
}

if (!visitor.hasAnnotations) return null;

var type = new AnnotatedType(cls);
type.annotations = visitor.classAnnotations
.where((annotation) {
.where((Annotation annotation) {
var element = annotation.element;
if (element != null && !element.isPublic) {
warn('Annotation $annotation is not public.',
Expand All @@ -277,6 +288,7 @@ class AnnotationExtractor {
element.enclosingElement.type.isAssignableTo(formatterType.type);
}).toList();

if (type.annotations.isEmpty) return null;

var memberAnnotations = {};
visitor.memberAnnotations.forEach((memberName, annotations) {
Expand All @@ -293,8 +305,6 @@ class AnnotationExtractor {
_foldMemberAnnotations(memberAnnotations, type);
}

if (type.annotations.isEmpty) return null;

return type;
}

Expand All @@ -309,10 +319,6 @@ class AnnotationExtractor {
return element.enclosingElement.type.isAssignableTo(
directiveType.type);
});
if (ngAnnotations.isEmpty) {
warn('Found field annotation but no class directives.', type.type);
return;
}

var mapType = resolver.getType('dart.core.Map').type;
// Find acceptable constructors- ones which take a param named 'map'
Expand Down Expand Up @@ -410,15 +416,17 @@ class _AnnotationVisitor extends GeneralizingAstVisitor {
final List<Element> allowedMemberAnnotations;
final List<Annotation> classAnnotations = [];
final Map<String, List<Annotation>> memberAnnotations = {};
var visitingSupertype = false;

_AnnotationVisitor(this.allowedMemberAnnotations);

void visitAnnotation(Annotation annotation) {
var parent = annotation.parent;
if (parent is! Declaration) return;

if (parent.element is ClassElement) {
if (parent.element is ClassElement && !visitingSupertype) {
classAnnotations.add(annotation);

} else if (allowedMemberAnnotations.contains(annotation.element)) {
if (parent is MethodDeclaration) {
memberAnnotations.putIfAbsent(parent.name.name, () => [])
Expand Down
13 changes: 5 additions & 8 deletions perf/pubspec.lock
Original file line number Diff line number Diff line change
Expand Up @@ -30,18 +30,15 @@ packages:
code_transformers:
description: code_transformers
source: hosted
version: "0.1.1"
version: "0.1.3"
collection:
description: collection
source: hosted
version: "0.9.1"
di:
description:
ref: null
resolved-ref: a2de00d84934f4610d1f9e108c39614e66a773f5
url: "git://github.com/angular/di.dart.git"
source: git
version: "0.0.34"
description: di
source: hosted
version: "0.0.37"
html5lib:
description: html5lib
source: hosted
Expand Down Expand Up @@ -73,7 +70,7 @@ packages:
route_hierarchical:
description: route_hierarchical
source: hosted
version: "0.4.15"
version: "0.4.18"
shadow_dom:
description: shadow_dom
source: hosted
Expand Down
38 changes: 38 additions & 0 deletions test/core/core_directive_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,29 @@ void main() {
'in Bad2Component');
});
});

describe("Inheritance", () {
var element;
var nodeAttrs;

beforeEachModule((Module module) {
module..type(Sub)..type(Base);
});

it("should extract attr map from annotated component which inherits other component", (DirectiveMap directives) {
var annotations = directives.annotationsFor(Sub);
expect(annotations.length).toEqual(1);
expect(annotations[0] is Directive).toBeTruthy();

Directive annotation = annotations[0];
expect(annotation.selector).toEqual('[sub]');
expect(annotation.map).toEqual({
"foo": "=>foo",
"bar": "=>bar",
"baz": "=>baz"
});
});
});
});
}

Expand Down Expand Up @@ -141,3 +164,18 @@ class Bad2Component {
@NgOneWay('foo')
set foo(val) {}
}

@Decorator(selector: '[sub]')
class Sub extends Base {
@NgOneWay('bar')
String bar;
}

class Base {
@NgOneWay('baz')
String baz;

@NgOneWay('foo')
String foo;
}

2 changes: 1 addition & 1 deletion test/core/registry_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -61,4 +61,4 @@ class MyAnnotation {
}

@MyAnnotation('A') @MyAnnotation('B') class A1 {}
@MyAnnotation('A') class A2 {}
@MyAnnotation('A') class A2 {}
38 changes: 38 additions & 0 deletions test/tools/transformer/expression_generator_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,39 @@ main() {
symbols: []);
});

it('should generate expressions for variables found in superclass', () {
return generates(phases,
inputs: {
'a|web/main.dart': '''
import 'package:angular/angular.dart';
@NgComponent(
templateUrl: 'lib/foo.html',
selector: 'my-component')
class FooComponent extends BarComponent {
@NgAttr('foo')
var foo;
}
class BarComponent {
@NgAttr('bar')
var bar;
}
main() {}
''',
'a|lib/foo.html': '''
<div>{{template.foo}}</div>
<div>{{template.bar}}</div>''',
'a|web/index.html': '''
<script src='main.dart' type='application/dart'></script>''',
'angular|lib/angular.dart': libAngular,
},
getters: ['foo', 'bar', 'template'],
setters: ['foo', 'bar', 'template'],
symbols: []);
});

it('should apply additional HTML files', () {
htmlFiles.add('web/dummy.html');
htmlFiles.add('/packages/b/bar.html');
Expand Down Expand Up @@ -186,4 +219,9 @@ library angular.core.annotation_src;
class Component {
const Component({String templateUrl, String selector});
}
class NgAttr {
final _mappingSpec = '@';
const NgAttr(String attrName);
}
''';
Loading

0 comments on commit eee4191

Please sign in to comment.