From 5d2af54730d4dc782f78edb80260df24758c27d0 Mon Sep 17 00:00:00 2001 From: Tim Blasi Date: Tue, 26 May 2015 16:53:14 -0700 Subject: [PATCH] feat(dart/transform): Improve constant evaluation Use `package:analyzer`'s `ConstantEvaluator` to read from the AST. This cleanly builds values for us from adjacent strings, interpolations, etc. --- .../src/transform/bind_generator/visitor.dart | 27 ++---- .../common/directive_metadata_reader.dart | 86 +++++++------------ .../directive_processor/visitors.dart | 18 ++-- .../compile_step_factory.dart | 1 - .../view_definition_creator.dart | 5 +- .../adjacent_strings_files/foo.ng_deps.dart | 16 ++++ .../all_tests.dart | 11 +++ .../directive_processor/all_tests.dart | 3 + .../expected/hello.ng_deps.dart | 20 +++++ .../split_url_expression_files/hello.dart | 8 ++ .../split_url_expression_files/template.html | 1 + 11 files changed, 114 insertions(+), 82 deletions(-) create mode 100644 modules/angular2/test/transform/directive_metadata_extractor/adjacent_strings_files/foo.ng_deps.dart create mode 100644 modules/angular2/test/transform/directive_processor/split_url_expression_files/expected/hello.ng_deps.dart create mode 100644 modules/angular2/test/transform/directive_processor/split_url_expression_files/hello.dart create mode 100644 modules/angular2/test/transform/directive_processor/split_url_expression_files/template.html diff --git a/modules/angular2/src/transform/bind_generator/visitor.dart b/modules/angular2/src/transform/bind_generator/visitor.dart index 6e65edd03d..2b8b6cf60c 100644 --- a/modules/angular2/src/transform/bind_generator/visitor.dart +++ b/modules/angular2/src/transform/bind_generator/visitor.dart @@ -8,29 +8,20 @@ import 'package:angular2/src/transform/common/logging.dart'; /// values found. class ExtractSettersVisitor extends Object with RecursiveAstVisitor { final Map bindMappings = {}; - - void _extractFromMapLiteral(MapLiteral map) { - map.entries.forEach((entry) { - // TODO(kegluneq): Remove this restriction - if (entry.key is SimpleStringLiteral && - entry.value is SimpleStringLiteral) { - bindMappings[stringLiteralToString(entry.key)] = - stringLiteralToString(entry.value); - } else { - logger.error('`properties` currently only supports string literals ' - '(${entry})'); - } - }); - } + final ConstantEvaluator _evaluator = new ConstantEvaluator(); @override Object visitNamedExpression(NamedExpression node) { if ('${node.name.label}' == 'properties') { - // TODO(kegluneq): Remove this restriction. - if (node.expression is MapLiteral) { - _extractFromMapLiteral(node.expression); + var evaluated = node.expression.accept(_evaluator); + if (evaluated is Map) { + evaluated.forEach((key, value) { + if (value != null) { + bindMappings[key] = '$value'; + } + }); } else { - logger.error('`properties` currently only supports map literals'); + logger.error('`properties` currently only supports Map values'); } return null; } diff --git a/modules/angular2/src/transform/common/directive_metadata_reader.dart b/modules/angular2/src/transform/common/directive_metadata_reader.dart index 3234664663..1f0787eba4 100644 --- a/modules/angular2/src/transform/common/directive_metadata_reader.dart +++ b/modules/angular2/src/transform/common/directive_metadata_reader.dart @@ -49,6 +49,7 @@ num _getDirectiveType(String annotationName, Element element) { class _DirectiveMetadataVisitor extends Object with RecursiveAstVisitor { DirectiveMetadata meta; + final ConstantEvaluator _evaluator = new ConstantEvaluator(); void _createEmptyMetadata(num type) { assert(type >= 0); @@ -130,13 +131,12 @@ class _DirectiveMetadataVisitor extends Object } String _expressionToString(Expression node, String nodeDescription) { - // TODO(kegluneq): Accept more options. - if (node is! SimpleStringLiteral) { - throw new FormatException( - 'Angular 2 currently only supports string literals ' + var value = node.accept(_evaluator); + if (value is! String) { + throw new FormatException('Angular 2 could not understand the value ' 'in $nodeDescription.', '$node' /* source */); } - return stringLiteralToString(node); + return value; } void _populateSelector(Expression selectorValue) { @@ -153,72 +153,52 @@ class _DirectiveMetadataVisitor extends Object void _populateCompileChildren(Expression compileChildrenValue) { _checkMeta(); - if (compileChildrenValue is! BooleanLiteral) { + var evaluated = compileChildrenValue.accept(_evaluator); + if (evaluated is! bool) { throw new FormatException( - 'Angular 2 currently only supports boolean literal values for ' + 'Angular 2 expects a bool but could not understand the value for ' 'Directive#compileChildren.', '$compileChildrenValue' /* source */); } - meta.compileChildren = (compileChildrenValue as BooleanLiteral).value; + meta.compileChildren = evaluated; + } + + /// Evaluates the [Map] represented by `expression` and adds all `key`, + /// `value` pairs to `map`. If `expression` does not evaluate to a [Map], + /// throws a descriptive [FormatException]. + void _populateMap(Expression expression, Map map, String propertyName) { + var evaluated = expression.accept(_evaluator); + if (evaluated is! Map) { + throw new FormatException( + 'Angular 2 expects a Map but could not understand the value for ' + '$propertyName.', '$expression' /* source */); + } + evaluated.forEach((key, value) { + if (value != null) { + map[key] = '$value'; + } + }); } void _populateProperties(Expression propertiesValue) { _checkMeta(); - if (propertiesValue is! MapLiteral) { - throw new FormatException( - 'Angular 2 currently only supports map literal values for ' - 'Directive#properties.', '$propertiesValue' /* source */); - } - for (MapLiteralEntry entry in (propertiesValue as MapLiteral).entries) { - var sKey = _expressionToString(entry.key, 'Directive#properties keys'); - var sVal = _expressionToString(entry.value, 'Direcive#properties values'); - meta.properties[sKey] = sVal; - } + _populateMap(propertiesValue, meta.properties, 'Directive#properties'); } void _populateHostListeners(Expression hostListenersValue) { _checkMeta(); - if (hostListenersValue is! MapLiteral) { - throw new FormatException( - 'Angular 2 currently only supports map literal values for ' - 'Directive#hostListeners.', '$hostListenersValue' /* source */); - } - for (MapLiteralEntry entry in (hostListenersValue as MapLiteral).entries) { - var sKey = _expressionToString(entry.key, 'Directive#hostListeners keys'); - var sVal = - _expressionToString(entry.value, 'Directive#hostListeners values'); - meta.hostListeners[sKey] = sVal; - } + _populateMap( + hostListenersValue, meta.hostListeners, 'Directive#hostListeners'); } void _populateHostProperties(Expression hostPropertyValue) { _checkMeta(); - if (hostPropertyValue is! MapLiteral) { - throw new FormatException( - 'Angular 2 currently only supports map literal values for ' - 'Directive#hostProperties.', '$hostPropertyValue' /* source */); - } - for (MapLiteralEntry entry in (hostPropertyValue as MapLiteral).entries) { - var sKey = - _expressionToString(entry.key, 'Directive#hostProperties keys'); - var sVal = - _expressionToString(entry.value, 'Directive#hostProperties values'); - meta.hostProperties[sKey] = sVal; - } + _populateMap( + hostPropertyValue, meta.hostProperties, 'Directive#hostProperties'); } void _populateHostAttributes(Expression hostAttributeValue) { _checkMeta(); - if (hostAttributeValue is! MapLiteral) { - throw new FormatException( - 'Angular 2 currently only supports map literal values for ' - 'Directive#hostAttributes.', '$hostAttributeValue' /* source */); - } - for (MapLiteralEntry entry in (hostAttributeValue as MapLiteral).entries) { - var sKey = - _expressionToString(entry.key, 'Directive#hostAttributes keys'); - var sVal = - _expressionToString(entry.value, 'Directive#hostAttributes values'); - meta.hostAttributes[sKey] = sVal; - } + _populateMap( + hostAttributeValue, meta.hostAttributes, 'Directive#hostAttributes'); } } diff --git a/modules/angular2/src/transform/directive_processor/visitors.dart b/modules/angular2/src/transform/directive_processor/visitors.dart index b876027b8e..6719e448b5 100644 --- a/modules/angular2/src/transform/directive_processor/visitors.dart +++ b/modules/angular2/src/transform/directive_processor/visitors.dart @@ -210,6 +210,7 @@ bool _isViewAnnotation(Annotation node) => '${node.name}' == 'View'; class AnnotationsTransformVisitor extends ToSourceVisitor { final AsyncStringWriter writer; final XHR _xhr; + final ConstantEvaluator _evaluator = new ConstantEvaluator(); bool _processingView = false; AnnotationsTransformVisitor(AsyncStringWriter writer, this._xhr) @@ -257,14 +258,15 @@ class AnnotationsTransformVisitor extends ToSourceVisitor { return super.visitNamedExpression(node); } var keyString = '${node.name.label}'; - if (keyString == 'templateUrl' && node.expression is SimpleStringLiteral) { - var url = stringLiteralToString(node.expression); - writer.print("template: r'''"); - writer.asyncPrint(_xhr.get(url)); - writer.print("'''"); - return null; - } else { - return super.visitNamedExpression(node); + if (keyString == 'templateUrl') { + var url = node.expression.accept(_evaluator); + if (url is String) { + writer.print("template: r'''"); + writer.asyncPrint(_xhr.get(url)); + writer.print("'''"); + return null; + } } + return super.visitNamedExpression(node); } } diff --git a/modules/angular2/src/transform/template_compiler/compile_step_factory.dart b/modules/angular2/src/transform/template_compiler/compile_step_factory.dart index df6e26559b..72e0dc7366 100644 --- a/modules/angular2/src/transform/template_compiler/compile_step_factory.dart +++ b/modules/angular2/src/transform/template_compiler/compile_step_factory.dart @@ -17,7 +17,6 @@ class CompileStepFactory implements base.CompileStepFactory { List createSteps( ViewDefinition template, List subTaskPromises) { - // TODO(kegluneq): Add other compile steps from default_steps.dart. return [ new ViewSplitter(_parser), new PropertyBindingParser(_parser), diff --git a/modules/angular2/src/transform/template_compiler/view_definition_creator.dart b/modules/angular2/src/transform/template_compiler/view_definition_creator.dart index 8aa4280c51..5db7b34266 100644 --- a/modules/angular2/src/transform/template_compiler/view_definition_creator.dart +++ b/modules/angular2/src/transform/template_compiler/view_definition_creator.dart @@ -155,6 +155,7 @@ class _ViewDefinitionCreator { class _TemplateExtractVisitor extends Object with RecursiveAstVisitor { ViewDefinition viewDef = null; final Map _metadataMap; + final ConstantEvaluator _evaluator = new ConstantEvaluator(); _TemplateExtractVisitor(this._metadataMap); @@ -191,13 +192,13 @@ class _TemplateExtractVisitor extends Object with RecursiveAstVisitor { // `templateUrl` property. if (viewDef == null) return null; - if (node.expression is! SimpleStringLiteral) { + var valueString = node.expression.accept(_evaluator); + if (valueString is! String) { logger.error( 'Angular 2 currently only supports string literals in directives.' ' Source: ${node}'); return null; } - var valueString = stringLiteralToString(node.expression); if (keyString == 'templateUrl') { if (viewDef.absUrl != null) { logger.error( diff --git a/modules/angular2/test/transform/directive_metadata_extractor/adjacent_strings_files/foo.ng_deps.dart b/modules/angular2/test/transform/directive_metadata_extractor/adjacent_strings_files/foo.ng_deps.dart new file mode 100644 index 0000000000..3a1505dc41 --- /dev/null +++ b/modules/angular2/test/transform/directive_metadata_extractor/adjacent_strings_files/foo.ng_deps.dart @@ -0,0 +1,16 @@ +library bar.ng_deps.dart; + +import 'foo.dart'; +import 'package:angular2/src/core/annotations/annotations.dart'; + +var _visited = false; +void initReflector(reflector) { + if (_visited) return; + _visited = true; + reflector + ..registerType(FooComponent, { + 'factory': () => new FooComponent(), + 'parameters': const [], + 'annotations': const [const Component(selector: '[fo' 'o]')] + }); +} diff --git a/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart b/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart index 5ade4298cb..2e932032c4 100644 --- a/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart +++ b/modules/angular2/test/transform/directive_metadata_extractor/all_tests.dart @@ -101,6 +101,17 @@ void allTests() { expect(extractedMeta.selector).toEqual('[foo]'); }); + it('should generate `DirectiveMetadata` from .ng_deps.dart files that use ' + 'automatic adjacent string concatenation.', () async { + var extracted = await extractDirectiveMetadata(reader, new AssetId('a', + 'directive_metadata_extractor/adjacent_strings_files/' + 'foo.ng_deps.dart')); + expect(extracted).toContain('FooComponent'); + + var extractedMeta = extracted['FooComponent']; + expect(extractedMeta.selector).toEqual('[foo]'); + }); + it('should include `DirectiveMetadata` from exported files.', () async { var extracted = await extractDirectiveMetadata(reader, new AssetId( 'a', 'directive_metadata_extractor/export_files/foo.ng_deps.dart')); diff --git a/modules/angular2/test/transform/directive_processor/all_tests.dart b/modules/angular2/test/transform/directive_processor/all_tests.dart index 1a777fe331..cfc1347700 100644 --- a/modules/angular2/test/transform/directive_processor/all_tests.dart +++ b/modules/angular2/test/transform/directive_processor/all_tests.dart @@ -39,6 +39,9 @@ void allTests() { _testNgDeps( 'should inline `templateUrl` values.', 'url_expression_files/hello.dart'); + + _testNgDeps('should inline `templateUrl`s expressed as adjacent strings.', + 'split_url_expression_files/hello.dart'); } void _testNgDeps(String name, String inputPath, diff --git a/modules/angular2/test/transform/directive_processor/split_url_expression_files/expected/hello.ng_deps.dart b/modules/angular2/test/transform/directive_processor/split_url_expression_files/expected/hello.ng_deps.dart new file mode 100644 index 0000000000..b877935fb0 --- /dev/null +++ b/modules/angular2/test/transform/directive_processor/split_url_expression_files/expected/hello.ng_deps.dart @@ -0,0 +1,20 @@ +library examples.src.hello_world.index_common_dart.ng_deps.dart; + +import 'hello.dart'; +import 'package:angular2/angular2.dart' + show bootstrap, Component, Directive, View, NgElement; + +var _visited = false; +void initReflector(reflector) { + if (_visited) return; + _visited = true; + reflector + ..registerType(HelloCmp, { + 'factory': () => new HelloCmp(), + 'parameters': const [], + 'annotations': const [ + const Component(selector: 'hello-app'), + const View(template: r'''{{greeting}}''') + ] + }); +} diff --git a/modules/angular2/test/transform/directive_processor/split_url_expression_files/hello.dart b/modules/angular2/test/transform/directive_processor/split_url_expression_files/hello.dart new file mode 100644 index 0000000000..14790703b2 --- /dev/null +++ b/modules/angular2/test/transform/directive_processor/split_url_expression_files/hello.dart @@ -0,0 +1,8 @@ +library examples.src.hello_world.index_common_dart; + +import 'package:angular2/angular2.dart' + show bootstrap, Component, Directive, View, NgElement; + +@Component(selector: 'hello-app') +@View(templateUrl: 'templ' 'ate.html') +class HelloCmp {} diff --git a/modules/angular2/test/transform/directive_processor/split_url_expression_files/template.html b/modules/angular2/test/transform/directive_processor/split_url_expression_files/template.html new file mode 100644 index 0000000000..d75013393f --- /dev/null +++ b/modules/angular2/test/transform/directive_processor/split_url_expression_files/template.html @@ -0,0 +1 @@ +{{greeting}} \ No newline at end of file