Skip to content

Commit 2604402

Browse files
author
Tim Blasi
committed
feat(dart/transform): Parse directives dependencies from the Dart ast
Previously, we parsed dependencies out of a the stringified value of `directives`, which is brittle and error-prone. Move this parsing into `DirectiveProcessor` where we have the full Dart ast to help.
1 parent ca5e31b commit 2604402

File tree

9 files changed

+221
-135
lines changed

9 files changed

+221
-135
lines changed

modules_dart/transform/lib/src/transform/common/code/reflection_info_code.dart

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,10 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
8383

8484
if (node.metadata != null) {
8585
node.metadata.forEach((node) {
86+
final extractedDirectives = _extractDirectives(node);
87+
if (extractedDirectives != null) {
88+
model.directives.addAll(extractedDirectives);
89+
}
8690
model.annotations.add(_annotationVisitor.visitAnnotation(node));
8791
});
8892
}
@@ -137,6 +141,36 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
137141
}
138142
}
139143

144+
Iterable<PrefixedDirective> _extractDirectives(Annotation node) {
145+
var shouldProcess = _annotationMatcher.isComponent(node, assetId);
146+
shouldProcess = shouldProcess || _annotationMatcher.isView(node, assetId);
147+
148+
if (node.arguments == null && node.arguments.arguments == null) return null;
149+
final directivesNode = node.arguments.arguments.firstWhere((arg) {
150+
return arg is NamedExpression && '${arg.name.label}' == 'directives';
151+
}, orElse: () => null);
152+
if (directivesNode == null) return null;
153+
154+
if (directivesNode.expression is! ListLiteral) {
155+
logger.warning('Angular 2 expects a list literal for `directives` '
156+
'but found a ${directivesNode.expression.runtimeType}');
157+
return null;
158+
}
159+
final directives = <PrefixedDirective>[];
160+
for (var dep in (directivesNode.expression as ListLiteral).elements) {
161+
if (dep is PrefixedIdentifier) {
162+
directives.add(new PrefixedDirective()
163+
..prefix = '${dep.prefix}'
164+
..name = '${dep.identifier}');
165+
} else if (dep is Identifier) {
166+
directives.add(new PrefixedDirective()..name = '${dep}');
167+
} else {
168+
logger.warning('Found unexpected value $dep in `directives`.');
169+
}
170+
}
171+
return directives;
172+
}
173+
140174
@override
141175
ReflectionInfoModel visitFunctionDeclaration(FunctionDeclaration node) {
142176
if (!node.metadata

modules_dart/transform/lib/src/transform/common/model/reflection_info_model.pb.dart

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,55 @@ class PropertyMetadataModel extends GeneratedMessage {
5252
class _ReadonlyPropertyMetadataModel extends PropertyMetadataModel
5353
with ReadonlyMessageMixin {}
5454

55+
class PrefixedDirective extends GeneratedMessage {
56+
static final BuilderInfo _i = new BuilderInfo('PrefixedDirective')
57+
..a(1, 'prefix', PbFieldType.OS)
58+
..a(2, 'name', PbFieldType.OS)
59+
..hasRequiredFields = false;
60+
61+
PrefixedDirective() : super();
62+
PrefixedDirective.fromBuffer(List<int> i,
63+
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
64+
: super.fromBuffer(i, r);
65+
PrefixedDirective.fromJson(String i,
66+
[ExtensionRegistry r = ExtensionRegistry.EMPTY])
67+
: super.fromJson(i, r);
68+
PrefixedDirective clone() => new PrefixedDirective()..mergeFromMessage(this);
69+
BuilderInfo get info_ => _i;
70+
static PrefixedDirective create() => new PrefixedDirective();
71+
static PbList<PrefixedDirective> createRepeated() =>
72+
new PbList<PrefixedDirective>();
73+
static PrefixedDirective getDefault() {
74+
if (_defaultInstance == null) _defaultInstance =
75+
new _ReadonlyPrefixedDirective();
76+
return _defaultInstance;
77+
}
78+
79+
static PrefixedDirective _defaultInstance;
80+
static void $checkItem(PrefixedDirective v) {
81+
if (v is! PrefixedDirective) checkItemFailed(v, 'PrefixedDirective');
82+
}
83+
84+
String get prefix => $_get(0, 1, '');
85+
void set prefix(String v) {
86+
$_setString(0, 1, v);
87+
}
88+
89+
bool hasPrefix() => $_has(0, 1);
90+
void clearPrefix() => clearField(1);
91+
92+
String get name => $_get(1, 2, '');
93+
void set name(String v) {
94+
$_setString(1, 2, v);
95+
}
96+
97+
bool hasName() => $_has(1, 2);
98+
void clearName() => clearField(2);
99+
}
100+
101+
class _ReadonlyPrefixedDirective extends PrefixedDirective
102+
with ReadonlyMessageMixin {}
103+
55104
class ReflectionInfoModel extends GeneratedMessage {
56105
static final BuilderInfo _i = new BuilderInfo('ReflectionInfoModel')
57106
..a(1, 'name', PbFieldType.QS)
@@ -63,7 +112,9 @@ class ReflectionInfoModel extends GeneratedMessage {
63112
ParameterModel.create)
64113
..p(6, 'interfaces', PbFieldType.PS)
65114
..pp(7, 'propertyMetadata', PbFieldType.PM,
66-
PropertyMetadataModel.$checkItem, PropertyMetadataModel.create);
115+
PropertyMetadataModel.$checkItem, PropertyMetadataModel.create)
116+
..pp(8, 'directives', PbFieldType.PM, PrefixedDirective.$checkItem,
117+
PrefixedDirective.create);
67118

68119
ReflectionInfoModel() : super();
69120
ReflectionInfoModel.fromBuffer(List<int> i,
@@ -120,6 +171,8 @@ class ReflectionInfoModel extends GeneratedMessage {
120171
List<String> get interfaces => $_get(5, 6, null);
121172

122173
List<PropertyMetadataModel> get propertyMetadata => $_get(6, 7, null);
174+
175+
List<PrefixedDirective> get directives => $_get(7, 8, null);
123176
}
124177

125178
class _ReadonlyReflectionInfoModel extends ReflectionInfoModel
@@ -139,6 +192,14 @@ const PropertyMetadataModel$json = const {
139192
],
140193
};
141194

195+
const PrefixedDirective$json = const {
196+
'1': 'PrefixedDirective',
197+
'2': const [
198+
const {'1': 'prefix', '3': 1, '4': 1, '5': 9},
199+
const {'1': 'name', '3': 2, '4': 1, '5': 9},
200+
],
201+
};
202+
142203
const ReflectionInfoModel$json = const {
143204
'1': 'ReflectionInfoModel',
144205
'2': const [
@@ -167,12 +228,19 @@ const ReflectionInfoModel$json = const {
167228
'5': 11,
168229
'6': '.angular2.src.transform.common.model.proto.PropertyMetadataModel'
169230
},
231+
const {
232+
'1': 'directives',
233+
'3': 8,
234+
'4': 3,
235+
'5': 11,
236+
'6': '.angular2.src.transform.common.model.proto.PrefixedDirective'
237+
},
170238
],
171239
};
172240

173241
/**
174242
* Generated with:
175-
* reflection_info_model.proto (71d723738054f1276f792a2672a956ef9be94a4c)
243+
* reflection_info_model.proto (e81bf93b6872b2bd5fabc6625be2560bacc3d186)
176244
* libprotoc 2.6.1
177245
* dart-protoc-plugin (af5fc2bf1de367a434c3b1847ab260510878ffc0)
178246
*/

modules_dart/transform/lib/src/transform/common/model/reflection_info_model.proto

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,15 @@ message PropertyMetadataModel {
1313
repeated AnnotationModel annotations = 2;
1414
}
1515

16+
message PrefixedDirective {
17+
// The prefix used to reference this Directive, if any.
18+
optional string prefix = 1;
19+
20+
// The name of the Directive or directive alias.
21+
// See https://goo.gl/d8XPt0 for info on directive aliases.
22+
optional string name = 2;
23+
}
24+
1625
message ReflectionInfoModel {
1726
// The (potentially prefixed) name of this Injectable.
1827
// This can be a `Type` or a function name.
@@ -32,4 +41,8 @@ message ReflectionInfoModel {
3241

3342
// Entries for all properties with associated metadata.
3443
repeated PropertyMetadataModel propertyMetadata = 7;
44+
45+
// Directive dependencies parsed from the @View or @Component `directives`
46+
// parameter.
47+
repeated PrefixedDirective directives = 8;
3548
}

modules_dart/transform/lib/src/transform/template_compiler/compile_data_creator.dart

Lines changed: 5 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -38,58 +38,6 @@ class CompileDataResults {
3838
this.ngMeta, this.viewDefinitions, this.directiveMetadatas);
3939
}
4040

41-
class _PrefixedDirectiveName {
42-
final String prefix;
43-
final String directiveName;
44-
45-
_PrefixedDirectiveName(String prefix, this.directiveName)
46-
: this.prefix = prefix == null ? '' : prefix;
47-
48-
@override
49-
String toString() {
50-
if (prefix.isEmpty) {
51-
return directiveName;
52-
} else {
53-
return '${prefix}.${directiveName}';
54-
}
55-
}
56-
}
57-
58-
String _directivesValue(ReflectionInfoModel model, bool test(AnnotationModel)) {
59-
final viewAnnotation = model.annotations.firstWhere(test, orElse: () => null);
60-
if (viewAnnotation == null) return null;
61-
final directivesParam = viewAnnotation.namedParameters
62-
.firstWhere((p) => p.name == 'directives', orElse: () => null);
63-
return directivesParam != null ? directivesParam.value : null;
64-
}
65-
66-
// TODO(kegluneq): Parse this value when creating [NgDepsModel]?
67-
/// Find the `directives` parameter on the @View annotation and make sure that
68-
/// all necessary [CompileDirectiveMetadata] objects are available.
69-
Iterable<_PrefixedDirectiveName> _getDirectiveDeps(ReflectionInfoModel model) {
70-
var directives = _directivesValue(model, (m) => m.isView);
71-
if (directives == null) {
72-
directives = _directivesValue(model, (m) => m.isComponent);
73-
}
74-
if (directives == null) return const [];
75-
directives = directives.trim();
76-
for (var toTrim in ['const [', '[']) {
77-
if (directives.startsWith(toTrim) && directives.endsWith(']')) {
78-
directives =
79-
directives.substring(toTrim.length, directives.length - 1).trim();
80-
}
81-
}
82-
if (directives.length == 0) return const [];
83-
return directives.split(',').map((p) {
84-
var parts = p.trim().split('.');
85-
if (parts.length == 1) {
86-
return new _PrefixedDirectiveName(null, parts[0]);
87-
} else {
88-
return new _PrefixedDirectiveName(parts[0], parts[1]);
89-
}
90-
});
91-
}
92-
9341
/// Creates [ViewDefinition] objects for all `View` `Directive`s defined in
9442
/// `entryPoint`.
9543
class _CompileDataCreator {
@@ -127,7 +75,7 @@ class _CompileDataCreator {
12775
if (compileDirectiveMetadata.template != null) {
12876
final compileDatum = new NormalizedComponentWithViewDirectives(
12977
compileDirectiveMetadata, <CompileDirectiveMetadata>[]);
130-
for (var dep in _getDirectiveDeps(reflectable)) {
78+
for (var dep in reflectable.directives) {
13179
if (!ngMetaMap.containsKey(dep.prefix)) {
13280
logger.warning(
13381
'Missing prefix "${dep.prefix}" '
@@ -137,11 +85,10 @@ class _CompileDataCreator {
13785
}
13886
final depNgMeta = ngMetaMap[dep.prefix];
13987

140-
if (depNgMeta.types.containsKey(dep.directiveName)) {
141-
compileDatum.directives.add(depNgMeta.types[dep.directiveName]);
142-
} else if (depNgMeta.aliases.containsKey(dep.directiveName)) {
143-
compileDatum.directives
144-
.addAll(depNgMeta.flatten(dep.directiveName));
88+
if (depNgMeta.types.containsKey(dep.name)) {
89+
compileDatum.directives.add(depNgMeta.types[dep.name]);
90+
} else if (depNgMeta.aliases.containsKey(dep.name)) {
91+
compileDatum.directives.addAll(depNgMeta.flatten(dep.name));
14592
} else {
14693
logger.warning('Could not find Directive entry for $dep. '
14794
'Please be aware that Dart transformers have limited support for '

modules_dart/transform/test/transform/directive_processor/all_tests.dart

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import 'package:angular2/src/transform/common/annotation_matcher.dart';
1010
import 'package:angular2/src/transform/common/code/ng_deps_code.dart';
1111
import 'package:angular2/src/transform/common/asset_reader.dart';
1212
import 'package:angular2/src/transform/common/logging.dart' as log;
13+
import 'package:angular2/src/transform/common/model/ng_deps_model.pb.dart';
1314
import 'package:angular2/src/transform/common/model/reflection_info_model.pb.dart';
1415
import 'package:angular2/src/transform/common/ng_meta.dart';
1516
import 'package:barback/barback.dart';
@@ -441,6 +442,58 @@ void allTests() {
441442
.toContain('[soup]');
442443
});
443444
});
445+
446+
describe('directives', () {
447+
final reflectableNamed = (NgDepsModel model, String name) {
448+
return model.reflectables
449+
.firstWhere((r) => r.name == name, orElse: () => null);
450+
};
451+
452+
it('should populate `directives` from @View value specified second.',
453+
() async {
454+
var model =
455+
(await _testCreateModel('directives_files/components.dart')).ngDeps;
456+
final componentFirst = reflectableNamed(model, 'ComponentFirst');
457+
expect(componentFirst).toBeNotNull();
458+
expect(componentFirst.directives).toBeNotNull();
459+
expect(componentFirst.directives.length).toEqual(2);
460+
expect(componentFirst.directives.first)
461+
.toEqual(new PrefixedDirective()..name = 'Dep');
462+
expect(componentFirst.directives[1]).toEqual(new PrefixedDirective()
463+
..name = 'Dep'
464+
..prefix = 'dep2');
465+
});
466+
467+
it('should populate `directives` from @View value specified first.',
468+
() async {
469+
var model =
470+
(await _testCreateModel('directives_files/components.dart')).ngDeps;
471+
final viewFirst = reflectableNamed(model, 'ViewFirst');
472+
expect(viewFirst).toBeNotNull();
473+
expect(viewFirst.directives).toBeNotNull();
474+
expect(viewFirst.directives.length).toEqual(2);
475+
expect(viewFirst.directives.first).toEqual(new PrefixedDirective()
476+
..name = 'Dep'
477+
..prefix = 'dep2');
478+
expect(viewFirst.directives[1])
479+
.toEqual(new PrefixedDirective()..name = 'Dep');
480+
});
481+
482+
it('should populate `directives` from @Component value with no @View.',
483+
() async {
484+
var model =
485+
(await _testCreateModel('directives_files/components.dart')).ngDeps;
486+
final componentOnly = reflectableNamed(model, 'ComponentOnly');
487+
expect(componentOnly).toBeNotNull();
488+
expect(componentOnly.directives).toBeNotNull();
489+
expect(componentOnly.directives.length).toEqual(2);
490+
expect(componentOnly.directives.first)
491+
.toEqual(new PrefixedDirective()..name = 'Dep');
492+
expect(componentOnly.directives[1]).toEqual(new PrefixedDirective()
493+
..name = 'Dep'
494+
..prefix = 'dep2');
495+
});
496+
});
444497
}
445498

446499
Future<NgMeta> _testCreateModel(String inputPath,
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
library angular2.test.transform.directive_processor.directive_files.components;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
import 'dep1.dart';
6+
import 'dep2.dart' as dep2;
7+
8+
@Component(selector: 'component-first')
9+
@View(template: '<dep1></dep1><dep2></dep2>', directives: [Dep, dep2.Dep])
10+
class ComponentFirst {}
11+
12+
@View(template: '<dep1></dep1><dep2></dep2>', directives: [dep2.Dep, Dep])
13+
@Component(selector: 'view-first')
14+
class ViewFirst {}
15+
16+
@Component(
17+
selector: 'component-only',
18+
template: '<dep1></dep1><dep2></dep2>',
19+
directives: [Dep, dep2.Dep])
20+
class ComponentOnly {}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
library angular2.test.transform.directive_processor.directive_files.dep1;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
6+
@Component(selector: 'dep1')
7+
@View(template: 'Dep1')
8+
class Dep {}
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
library angular2.test.transform.directive_processor.directive_files.dep2;
2+
3+
import 'package:angular2/angular2.dart'
4+
show Component, Directive, View, NgElement;
5+
6+
@Component(selector: 'dep2')
7+
@View(template: 'Dep2')
8+
class Dep {}

0 commit comments

Comments
 (0)