feat(dart/transform): Match runtime semantics for template values

Match [ViewResolver][]'s semantics for reading template and style values
from `@Component` and `@View` annotations.

We now warn if template and/or style values appear on an `@Component`
annotation and a `@View` annotation is present.

[ViewResolver]: 7c6130c2c5/modules/angular2/src/core/linker/view_resolver.ts
This commit is contained in:
Tim Blasi 2015-10-20 08:46:41 -07:00
parent 798be2776d
commit bdd031aecc
6 changed files with 129 additions and 22 deletions

View File

@ -25,9 +25,6 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
final ParameterVisitor _parameterVisitor = new ParameterVisitor(); final ParameterVisitor _parameterVisitor = new ParameterVisitor();
final _PropertyMetadataVisitor _propMetadataVisitor; final _PropertyMetadataVisitor _propMetadataVisitor;
/// Whether an Angular 2 `Reflection` has been found.
bool _foundNgReflection = false;
ReflectionInfoVisitor._(this.assetId, this._annotationMatcher, ReflectionInfoVisitor._(this.assetId, this._annotationMatcher,
this._annotationVisitor, this._propMetadataVisitor); this._annotationVisitor, this._propMetadataVisitor);
@ -38,8 +35,6 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
annotationVisitor, new _PropertyMetadataVisitor(annotationVisitor)); annotationVisitor, new _PropertyMetadataVisitor(annotationVisitor));
} }
bool get shouldCreateNgDeps => _foundNgReflection;
ConstructorDeclaration _getCtor(ClassDeclaration node) { ConstructorDeclaration _getCtor(ClassDeclaration node) {
int numCtorsFound = 0; int numCtorsFound = 0;
var ctor = null; var ctor = null;
@ -82,13 +77,26 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
} }
if (node.metadata != null) { if (node.metadata != null) {
var componentDirectives, viewDirectives;
node.metadata.forEach((node) { node.metadata.forEach((node) {
final extractedDirectives = _extractDirectives(node); if (_annotationMatcher.isComponent(node, assetId)) {
if (extractedDirectives != null) { componentDirectives = _extractDirectives(node);
model.directives.addAll(extractedDirectives); } else if (_annotationMatcher.isView(node, assetId)) {
viewDirectives = _extractDirectives(node);
} }
model.annotations.add(_annotationVisitor.visitAnnotation(node)); model.annotations.add(_annotationVisitor.visitAnnotation(node));
}); });
if (componentDirectives != null && componentDirectives.isNotEmpty) {
if (viewDirectives != null) {
logger.warning(
'Cannot specify view parameters on @Component when a @View '
'is present. Component name: ${model.name}',
asset: assetId);
}
model.directives.addAll(componentDirectives);
} else if (viewDirectives != null) {
model.directives.addAll(viewDirectives);
}
} }
if (ctor != null && if (ctor != null &&
ctor.parameters != null && ctor.parameters != null &&
@ -141,20 +149,26 @@ class ReflectionInfoVisitor extends RecursiveAstVisitor<ReflectionInfoModel> {
} }
} }
/// Returns [PrefixedDirective] values parsed from the value of the
/// `directives` parameter of the provided `node`.
/// This will always return a non-null value, so if there are no `directives`
/// specified on `node`, it will return an empty iterable.
Iterable<PrefixedDirective> _extractDirectives(Annotation node) { Iterable<PrefixedDirective> _extractDirectives(Annotation node) {
var shouldProcess = _annotationMatcher.isComponent(node, assetId); assert(_annotationMatcher.isComponent(node, assetId) ||
shouldProcess = shouldProcess || _annotationMatcher.isView(node, assetId); _annotationMatcher.isView(node, assetId));
if (node.arguments == null && node.arguments.arguments == null) return null; if (node.arguments == null && node.arguments.arguments == null) {
return const [];
}
final directivesNode = node.arguments.arguments.firstWhere((arg) { final directivesNode = node.arguments.arguments.firstWhere((arg) {
return arg is NamedExpression && '${arg.name.label}' == 'directives'; return arg is NamedExpression && '${arg.name.label}' == 'directives';
}, orElse: () => null); }, orElse: () => null);
if (directivesNode == null) return null; if (directivesNode == null) return const [];
if (directivesNode.expression is! ListLiteral) { if (directivesNode.expression is! ListLiteral) {
logger.warning('Angular 2 expects a list literal for `directives` ' logger.warning('Angular 2 expects a list literal for `directives` '
'but found a ${directivesNode.expression.runtimeType}'); 'but found a ${directivesNode.expression.runtimeType}');
return null; return const [];
} }
final directives = <PrefixedDirective>[]; final directives = <PrefixedDirective>[];
for (var dep in (directivesNode.expression as ListLiteral).elements) { for (var dep in (directivesNode.expression as ListLiteral).elements) {

View File

@ -10,6 +10,7 @@ import 'package:angular2/src/core/linker/interfaces.dart' show LifecycleHooks;
import 'package:angular2/src/core/metadata/view.dart' show ViewEncapsulation; import 'package:angular2/src/core/metadata/view.dart' show ViewEncapsulation;
import 'package:angular2/src/transform/common/annotation_matcher.dart'; import 'package:angular2/src/transform/common/annotation_matcher.dart';
import 'package:angular2/src/transform/common/interface_matcher.dart'; import 'package:angular2/src/transform/common/interface_matcher.dart';
import 'package:angular2/src/transform/common/logging.dart';
import 'package:barback/barback.dart' show AssetId; import 'package:barback/barback.dart' show AssetId;
import 'naive_eval.dart'; import 'naive_eval.dart';
@ -173,6 +174,21 @@ class _DirectiveMetadataVisitor extends Object
template: _template); template: _template);
} }
/// Ensures that we do not specify View values on an `@Component` annotation
/// when there is a @View annotation present.
void _validateTemplates() {
if (_cmpTemplate != null && _viewTemplate != null) {
var name = '(Unknown)';
if (_type != null && _type.name != null && _type.name.isNotEmpty) {
name = _type.name;
}
logger.warning(
'Cannot specify view parameters on @Component when a @View '
'is present. Component name: ${name}',
asset: _assetId);
}
}
@override @override
Object visitAnnotation(Annotation node) { Object visitAnnotation(Annotation node) {
var isComponent = _annotationMatcher.isComponent(node, _assetId); var isComponent = _annotationMatcher.isComponent(node, _assetId);
@ -187,14 +203,14 @@ class _DirectiveMetadataVisitor extends Object
_isComponent = isComponent; _isComponent = isComponent;
_hasMetadata = true; _hasMetadata = true;
if (isComponent) { if (isComponent) {
var t = new _CompileTemplateMetadataVisitor().visitAnnotation(node); _cmpTemplate =
if (t.template != null || t.templateUrl != null) { new _CompileTemplateMetadataVisitor().visitAnnotation(node);
_cmpTemplate = t; _validateTemplates();
}
} }
super.visitAnnotation(node); super.visitAnnotation(node);
} else if (_annotationMatcher.isView(node, _assetId)) { } else if (_annotationMatcher.isView(node, _assetId)) {
if (_viewTemplate != null) { if (_viewTemplate != null) {
// TODO(kegluneq): Support multiple views on a single class.
throw new FormatException( throw new FormatException(
'Only one View is allowed per class. ' 'Only one View is allowed per class. '
'Found unexpected "$node".', 'Found unexpected "$node".',
@ -202,6 +218,7 @@ class _DirectiveMetadataVisitor extends Object
} }
_viewTemplate = _viewTemplate =
new _CompileTemplateMetadataVisitor().visitAnnotation(node); new _CompileTemplateMetadataVisitor().visitAnnotation(node);
_validateTemplates();
} }
// Annotation we do not recognize - no need to visit. // Annotation we do not recognize - no need to visit.
@ -353,16 +370,24 @@ class _LifecycleHookVisitor extends SimpleAstVisitor<List<LifecycleHooks>> {
/// [CompileTemplateMetadata]. /// [CompileTemplateMetadata].
class _CompileTemplateMetadataVisitor class _CompileTemplateMetadataVisitor
extends RecursiveAstVisitor<CompileTemplateMetadata> { extends RecursiveAstVisitor<CompileTemplateMetadata> {
ViewEncapsulation _encapsulation = ViewEncapsulation.Emulated; ViewEncapsulation _encapsulation;
String _template = null; String _template;
String _templateUrl = null; String _templateUrl;
List<String> _styles = null; List<String> _styles;
List<String> _styleUrls = null; List<String> _styleUrls;
@override @override
CompileTemplateMetadata visitAnnotation(Annotation node) { CompileTemplateMetadata visitAnnotation(Annotation node) {
super.visitAnnotation(node); super.visitAnnotation(node);
if (_encapsulation == null &&
_template == null &&
_templateUrl == null &&
_styles == null &&
_styleUrls == null) {
return null;
}
return new CompileTemplateMetadata( return new CompileTemplateMetadata(
encapsulation: _encapsulation, encapsulation: _encapsulation,
template: _template, template: _template,

View File

@ -493,6 +493,44 @@ void allTests() {
..name = 'Dep' ..name = 'Dep'
..prefix = 'dep2'); ..prefix = 'dep2');
}); });
it('should warn if @Component has a `template` and @View is present.',
() async {
final logger = new RecordingLogger();
final model = await _testCreateModel('bad_directives_files/template.dart',
logger: logger);
var warning =
logger.logs.firstWhere((l) => l.contains('WARN'), orElse: () => null);
expect(warning).toBeNotNull();
expect(warning.toLowerCase())
.toContain('cannot specify view parameters on @component');
});
it('should warn if @Component has a `templateUrl` and @View is present.',
() async {
final logger = new RecordingLogger();
final model = await _testCreateModel(
'bad_directives_files/template_url.dart',
logger: logger);
var warning =
logger.logs.firstWhere((l) => l.contains('WARN'), orElse: () => null);
expect(warning).toBeNotNull();
expect(warning.toLowerCase())
.toContain('cannot specify view parameters on @component');
});
it('should warn if @Component has `directives` and @View is present.',
() async {
final logger = new RecordingLogger();
final model = await _testCreateModel(
'bad_directives_files/directives.dart',
logger: logger);
var warning =
logger.logs.firstWhere((l) => l.contains('WARN'), orElse: () => null);
expect(warning).toBeNotNull();
expect(warning.toLowerCase())
.toContain('cannot specify view parameters on @component');
});
}); });
} }

View File

@ -0,0 +1,10 @@
library angular2.test.transform.directive_processor.bad_directives_files.directives;
import 'package:angular2/angular2.dart'
show Component, Directive, View, NgElement;
import 'dep1.dart';
import 'dep2.dart' as dep2;
@Component(selector: 'component-first', directives: [Dep, dep2.Dep])
@View(template: '<div>Hi</div>')
class BadDirectives {}

View File

@ -0,0 +1,10 @@
library angular2.test.transform.directive_processor.bad_directives_files.template;
import 'package:angular2/angular2.dart'
show Component, Directive, View, NgElement;
import 'dep1.dart';
import 'dep2.dart' as dep2;
@Component(selector: 'component-first', template: 'bad!')
@View(template: 'good!', directives: [Dep, dep2.Dep])
class BadTemplate {}

View File

@ -0,0 +1,10 @@
library angular2.test.transform.directive_processor.bad_directives_files.template_url;
import 'package:angular2/angular2.dart'
show Component, Directive, View, NgElement;
import 'dep1.dart';
import 'dep2.dart' as dep2;
@Component(selector: 'component-first', templateUrl: 'bad_template.html')
@View(templateUrl: 'good_template.html', directives: [Dep, dep2.Dep])
class BadTemplateUrl {}