refactor(dart/transform): Remove unnecessary getter/setter codegen

Currently the transformer generates all getters and setters even when creating pre-generated change detectors, which remove the need for them. Generate getters and setters via the model provided by `ProtoViewDto`, which contains enough information to allow omitting unnecessary getters and setters from code output. Allow generating getters, setters, and method names which are Dart pseudo keywords. Closes angular#3489

Author: Tim Blasi

modules/angular2/src/change_detection/parser/parser.ts

@@ -490,14 +490,11 @@ export class _ParseAST {
  new MethodCall(receiver, id, fn, args);
 
  } else {
- let getter = this.reflector.getter(id);
- let setter = this.reflector.setter(id);
-
  if (isSafe) {
  if (this.optionalOperator("=")) {
  this.error("The '?.' operator cannot be used in the assignment");
  } else {
- return new SafePropertyRead(receiver, id, getter);
+ return new SafePropertyRead(receiver, id, this.reflector.getter(id));
  }
  } else {
  if (this.optionalOperator("=")) {
@@ -506,9 +503,9 @@ export class _ParseAST {
  }
 
  let value = this.parseConditional();
- return new PropertyWrite(receiver, id, setter, value);
+ return new PropertyWrite(receiver, id, this.reflector.setter(id), value);
  } else {
- return new PropertyRead(receiver, id, getter);
+ return new PropertyRead(receiver, id, this.reflector.getter(id));
  }
  }
  }

modules/angular2/src/core/compiler/element_injector.ts

@@ -44,6 +44,7 @@ import {ChangeDetector, ChangeDetectorRef} from 'angular2/src/change_detection/c
 import {QueryList} from './query_list';
 import {reflector} from 'angular2/src/reflection/reflection';
 import {RenderDirectiveMetadata} from 'angular2/src/render/api';
+import {EventConfig} from 'angular2/src/render/dom/util';
 import {PipeBinding} from '../pipes/pipe_binding';
 
 var _staticKeys;
@@ -303,18 +304,8 @@ function _createEventEmitterAccessors(bwv: BindingWithVisibility): EventEmitterA
  if (!(binding instanceof DirectiveBinding)) return [];
  var db = <DirectiveBinding>binding;
  return ListWrapper.map(db.eventEmitters, eventConfig => {
- let fieldName;
- let eventName;
- var colonIdx = eventConfig.indexOf(':');
- if (colonIdx > -1) {
- // long format: 'fieldName: eventName'
- fieldName = StringWrapper.substring(eventConfig, 0, colonIdx).trim();
- eventName = StringWrapper.substring(eventConfig, colonIdx + 1).trim();
- } else {
- // short format: 'name' when fieldName and eventName are the same
- fieldName = eventName = eventConfig;
- }
- return new EventEmitterAccessor(eventName, reflector.getter(fieldName));
+ var parsedEvent = EventConfig.parse(eventConfig);
+ return new EventEmitterAccessor(parsedEvent.eventName, reflector.getter(parsedEvent.fieldName));
  });
 }
 

modules/angular2/src/render/dom/compiler/directive_parser.ts

@@ -10,7 +10,7 @@ import {CompileElement} from './compile_element';
 import {CompileControl} from './compile_control';
 
 import {RenderDirectiveMetadata} from '../../api';
-import {dashCaseToCamelCase, camelCaseToDashCase, EVENT_TARGET_SEPARATOR} from '../util';
+import {EventConfig, dashCaseToCamelCase, camelCaseToDashCase} from '../util';
 import {DirectiveBuilder, ElementBinderBuilder} from '../view/proto_view_builder';
 
 /**
@@ -146,12 +146,9 @@ export class DirectiveParser implements CompileStep {
 
  _bindDirectiveEvent(eventName, action, compileElement, directiveBinderBuilder) {
  var ast = this._parser.parseAction(action, compileElement.elementDescription);
- if (StringWrapper.contains(eventName, EVENT_TARGET_SEPARATOR)) {
- var parts = eventName.split(EVENT_TARGET_SEPARATOR);
- directiveBinderBuilder.bindEvent(parts[1], ast, parts[0]);
- } else {
- directiveBinderBuilder.bindEvent(eventName, ast);
- }
+ var parsedEvent = EventConfig.parse(eventName);
+ var targetName = parsedEvent.isLongForm ? parsedEvent.fieldName : null;
+ directiveBinderBuilder.bindEvent(parsedEvent.eventName, ast, targetName);
  }
 
  _bindHostProperty(hostPropertyName, expression, compileElement, directiveBinderBuilder) {

modules/angular2/src/render/dom/util.ts

@@ -29,6 +29,27 @@ export function dashCaseToCamelCase(input: string): string {
  (m) => { return m[1].toUpperCase(); });
 }
 
+export class EventConfig {
+ constructor(public fieldName: string, public eventName: string, public isLongForm: boolean) {}
+
+ static parse(eventConfig: string): EventConfig {
+ var fieldName = eventConfig, eventName = eventConfig, isLongForm = false;
+ var separatorIdx = eventConfig.indexOf(EVENT_TARGET_SEPARATOR);
+ if (separatorIdx > -1) {
+ // long format: 'fieldName: eventName'
+ fieldName = StringWrapper.substring(eventConfig, 0, separatorIdx).trim();
+ eventName = StringWrapper.substring(eventConfig, separatorIdx + 1).trim();
+ isLongForm = true;
+ }
+ return new EventConfig(fieldName, eventName, isLongForm);
+ }
+
+ getFullName(): string {
+ return this.isLongForm ? `${this.fieldName}${EVENT_TARGET_SEPARATOR}${this.eventName}` :
+ this.eventName;
+ }
+}
+
 // Attention: This is on the hot path, so don't use closures or default values!
 export function queryBoundElements(templateContent: Node, isSingleElementChild: boolean):
  Element[] {

modules/angular2/src/render/dom/view/proto_view_builder.ts

@@ -262,7 +262,7 @@ export class DirectiveBuilder {
  }
 }
 
-export class EventBuilder extends AstTransformer {
+class EventBuilder extends AstTransformer {
  locals: List<AST> = [];
  localEvents: List<Event> = [];
  globalEvents: List<Event> = [];

modules/angular2/src/transform/bind_generator/generator.dart

@@ -31,7 +31,7 @@ Future<String> createNgSettersAndGetters(
  return '$out';
 }
 
-// TODO(kegluneq): De-dupe from template_compiler/generator.dart.
+// TODO(kegluneq): De-dupe from template_compiler/generator.dart, #3589.
 
 /// Consumes the map generated by {@link _createPropertiesMap} to codegen
 /// setters.

modules/angular2/src/transform/common/property_utils.dart

@@ -3,7 +3,10 @@ library angular2.transform.common.property_utils;
 import 'package:analyzer/src/generated/scanner.dart' show Keyword;
 
 /// Whether `name` is a valid property name.
-bool isValid(String name) => !Keyword.keywords.containsKey(name);
+bool isValid(String name) {
+ var keyword = Keyword.keywords[name];
+ return keyword == null || keyword.isPseudoKeyword;
+}
 
 /// Prepares `name` to be emitted inside a string.
 String sanitize(String name) => name.replaceAll('\$', '\\\$');

modules/angular2/src/transform/template_compiler/generator.dart

@@ -23,8 +23,9 @@ import 'package:barback/barback.dart';
 
 import 'change_detector_codegen.dart' as change;
 import 'compile_step_factory.dart';
-import 'reflection_capabilities.dart';
-import 'reflector_register_codegen.dart' as reg;
+import 'reflection/codegen.dart' as reg;
+import 'reflection/processor.dart' as reg;
+import 'reflection/reflection_capabilities.dart';
 import 'view_definition_creator.dart';
 
 /// Reads the `.ng_deps.dart` file represented by `entryPoint` and parses any
@@ -42,23 +43,22 @@ Future<String> processTemplates(AssetReader reader, AssetId entryPoint,
  var extractor = new _TemplateExtractor(new DomElementSchemaRegistry(),
  new TemplateCloner(-1), new XhrImpl(reader, entryPoint));
 
- var registrations = new reg.Codegen();
+ final processor = new reg.Processor();
+
  var changeDetectorClasses = new change.Codegen();
  for (var rType in viewDefResults.viewDefinitions.keys) {
  var viewDefEntry = viewDefResults.viewDefinitions[rType];
- var result = await extractor.extractTemplates(viewDefEntry.viewDef);
- if (result == null) continue;
+ var protoView = await extractor.extractTemplates(viewDefEntry.viewDef);
+ if (protoView == null) continue;
 
  if (generateRegistrations) {
- // TODO(kegluneq): Generate these getters & setters based on the
- // `ProtoViewDto` rather than querying the `ReflectionCapabilities`.
- registrations.generate(result.recording);
+ processor.process(viewDefEntry, protoView);
  }
- if (result.protoView != null && generateChangeDetectors) {
+ if (generateChangeDetectors) {
  var saved = reflector.reflectionCapabilities;
  reflector.reflectionCapabilities = const NullReflectionCapabilities();
  var defs = getChangeDetectorDefinitions(viewDefEntry.hostMetadata,
- result.protoView, viewDefEntry.viewDef.directives);
+ protoView, viewDefEntry.viewDef.directives);
  for (var i = 0; i < defs.length; ++i) {
  changeDetectorClasses.generate('${rType.typeName}',
  '_${rType.typeName}_ChangeDetector$i', defs[i]);
@@ -67,6 +67,10 @@ Future<String> processTemplates(AssetReader reader, AssetId entryPoint,
  }
  }
 
+ // TODO(kegluneq): Do not hard-code `false` here once i/3436 is fixed.
+ final registrations = new reg.Codegen(generateChangeDetectors: false);
+ registrations.generate(processor);
+
  var code = viewDefResults.ngDeps.code;
  if (registrations.isEmpty && changeDetectorClasses.isEmpty) return code;
  var importInjectIdx =
@@ -93,19 +97,16 @@ class _TemplateExtractor {
  ElementSchemaRegistry _schemaRegistry;
  TemplateCloner _templateCloner;
 
- _TemplateExtractor(ElementSchemaRegistry schemaRegistry,
- TemplateCloner templateCloner, XHR xhr)
+ _TemplateExtractor(this._schemaRegistry, this._templateCloner, XHR xhr)
  : _factory = new CompileStepFactory(new ng.Parser(new ng.Lexer())) {
  var urlResolver = new UrlResolver();
  var styleUrlResolver = new StyleUrlResolver(urlResolver);
  var styleInliner = new StyleInliner(xhr, styleUrlResolver, urlResolver);
 
  _loader = new ViewLoader(xhr, styleInliner, styleUrlResolver);
- _schemaRegistry = schemaRegistry;
- _templateCloner = templateCloner;
  }
 
- Future<_ExtractResult> extractTemplates(ViewDefinition viewDef) async {
+ Future<ProtoViewDto> extractTemplates(ViewDefinition viewDef) async {
  // Check for "imperative views".
  if (viewDef.template == null && viewDef.templateAbsUrl == null) return null;
 
@@ -115,8 +116,7 @@ class _TemplateExtractor {
  // operations between saving and restoring it, otherwise we can get into
  // a bad state. See issue #2359 for additional context.
  var savedReflectionCapabilities = reflector.reflectionCapabilities;
- var recordingCapabilities = new RecordingReflectionCapabilities();
- reflector.reflectionCapabilities = recordingCapabilities;
+ reflector.reflectionCapabilities = const NullReflectionCapabilities();
 
  var pipeline = new CompilePipeline(_factory.createSteps(viewDef));
 
@@ -130,13 +130,6 @@ class _TemplateExtractor {
 
  reflector.reflectionCapabilities = savedReflectionCapabilities;
 
- return new _ExtractResult(recordingCapabilities, protoViewDto);
+ return protoViewDto;
  }
 }
-
-class _ExtractResult {
- final RecordingReflectionCapabilities recording;
- final ProtoViewDto protoView;
-
- _ExtractResult(this.recording, this.protoView);
-}

modules/angular2/src/transform/template_compiler/reflector_register_codegen.dart renamed to modules/angular2/src/transform/template_compiler/reflection/codegen.dart

@@ -1,32 +1,48 @@
-library angular2.transform.template_compiler.reflector_register_codegen;
+library angular2.transform.template_compiler.reflection.codegen;
 
 import 'package:angular2/src/transform/common/names.dart';
 import 'package:angular2/src/transform/common/property_utils.dart' as prop;
-import 'reflection_capabilities.dart';
+
+import 'model.dart';
 
 class Codegen {
  final StringBuffer _buf = new StringBuffer();
 
- void generate(RecordingReflectionCapabilities recording) {
- if (recording != null) {
- var calls = _generateGetters(recording.getterNames);
+ /// Whether we are pre-generating change detectors.
+ /// If we have pre-generated change detectors, we need
+ final bool generateChangeDetectors;
+
+ Codegen({this.generateChangeDetectors});
+
+ void generate(CodegenModel model) {
+ if (model != null) {
+ var calls = _generateGetters(_extractNames(model.getterNames));
  if (calls.isNotEmpty) {
  _buf.write('..${REGISTER_GETTERS_METHOD_NAME}'
  '({${calls.join(', ')}})');
  }
- calls = _generateSetters(recording.setterNames);
+ calls = _generateSetters(_extractNames(model.setterNames));
  if (calls.isNotEmpty) {
  _buf.write('..${REGISTER_SETTERS_METHOD_NAME}'
  '({${calls.join(', ')}})');
  }
- calls = _generateMethods(recording.methodNames);
+ calls = _generateMethods(_extractNames(model.methodNames));
  if (calls.isNotEmpty) {
  _buf.write('..${REGISTER_METHODS_METHOD_NAME}'
  '({${calls.join(', ')}})');
  }
  }
  }
 
+ Iterable<String> _extractNames(Iterable<ReflectiveAccessor> accessors) {
+ var names = accessors.where((accessor) {
+ return accessor.isStaticallyNecessary || !generateChangeDetectors;
+ }).map((accessor) => accessor.sanitizedName);
+ var nameList = names.toList();
+ nameList.sort();
+ return nameList;
+ }
+
  bool get isEmpty => _buf.isEmpty;
 
  @override

modules/angular2/src/transform/template_compiler/reflection/model.dart

@@ -0,0 +1,49 @@
+library angular2.transform.template_compiler.reflection.model;
+
+import 'package:angular2/src/render/dom/util.dart';
+
+/// Defines the names of getters, setters, and methods which need to be
+/// available to Angular 2 via the `reflector` at runtime.
+/// See [angular2.src.reflection.reflector] for details.
+abstract class CodegenModel {
+ Iterable<ReflectiveAccessor> get getterNames;
+ Iterable<ReflectiveAccessor> get methodNames;
+ Iterable<ReflectiveAccessor> get setterNames;
+}
+
+/// Wraps a getter, setter, or method that we may need to access reflectively in
+/// an Angular2 app.
+/// This is essentially a wrapper for `sanitizedName`, which is the name of the
+/// actual getter, setter, or method that will be registered. Note that
+/// `operator==` and `hashCode` basically forward to `sanitizedName`.
+class ReflectiveAccessor {
+ /// The value in the Ast determining that we need this accessor. This is the
+ /// value that is actually present in the template, which may not directly
+ /// correspond to the model on the `Component`.
+ final String astValue;
+
+ /// The sanitized name of this accessor. This is the name of the getter,
+ /// setter, or method on the `Component`.
+ final String sanitizedName;
+
+ /// Whether this getter, setter, or method is still necessary when we have
+ /// pre-generated change detectors.
+ final bool isStaticallyNecessary;
+
+ ReflectiveAccessor(String astValue, {this.isStaticallyNecessary})
+ : this.astValue = astValue,
+ this.sanitizedName = sanitizePropertyName(astValue);
+
+ @override
+ bool operator ==(other) {
+ if (other is! ReflectiveAccessor) return false;
+ return sanitizedName == other.sanitizedName;
+ }
+
+ @override
+ int get hashCode => sanitizedName.hashCode;
+}
+
+String sanitizePropertyName(String name) {
+ return dashCaseToCamelCase(EventConfig.parse(name).fieldName);
+}