Emit forwarding getter/setter when overriding just a getter or setter.

lib/src/compiler/code_generator.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 import 'dart:collection' show HashMap, HashSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/dart/ast/token.dart' show Token, TokenType;
 import 'package:analyzer/dart/element/element.dart';
 import 'package:analyzer/dart/element/type.dart';
@@ skipping 55 matching lines @@
   /// The list of output module items, in the order they need to be emitted in.
   final _moduleItems = <JS.ModuleItem>[];
 
   /// The global extension type table.
   final ExtensionTypeSet _extensionTypes;
 
   /// Information that is precomputed for this library, indicates which fields
   /// need storage slots.
   HashSet<FieldElement> _fieldsNeedingStorage;
 
+  /// Information that indicates which properties are partial overrides of
+  /// superclass properties (e.g., getter-only override of a setter or
+  /// vice-versa).
+  HashSet<FieldElement> _propertyOverrides;

[Jennifer Messerly, 2016/04/20 21:16:39]

if following my suggestion in the next file, this and _fieldsNeedingStorage
won't be needed :)

[Harry Terkelsen, 2016/04/20 23:33:27]

Acknowledged.

+
   /// The variable for the target of the current `..` cascade expression.
   ///
   /// Usually a [SimpleIdentifier], but it can also be other expressions
   /// that are safe to evaluate multiple times, such as `this`.
   Expression _cascadeTarget;
 
   /// The variable for the current catch clause
   SimpleIdentifier _catchParameter;
 
   /// In an async* function, this represents the stream controller parameter.
@@ skipping 91 matching lines @@
     }
 
     return new Tuple2(printer.getText(), sourceMap?.build(unit.name + '.js'));
   }
 
   JS.Program _emitModule(List<CompilationUnit> compilationUnits) {
     if (_moduleItems.isNotEmpty) {
       throw new StateError('Can only call emitModule once.');
     }
 
-    _fieldsNeedingStorage = findFieldsNeedingStorage(
+    _fieldsNeedingStorage = new HashSet<FieldElement>();
+    _propertyOverrides = new HashSet<FieldElement>();
+
+    findFieldsNeedingStorage(_fieldsNeedingStorage, _propertyOverrides,
         compilationUnits.map((u) => u.element), _extensionTypes);
 
     // Transform the AST to make coercions explicit.
     compilationUnits = CoercionReifier.reify(compilationUnits);
 
     // Initialize our library variables.
     var items = <JS.ModuleItem>[];
     for (var unit in compilationUnits) {
       var library = unit.element.library;
       if (unit.element != library.definingCompilationUnit) continue;
@@ skipping 577 matching lines @@
 
     bool hasJsPeer = findAnnotation(element, isJsPeerInterface) != null;
 
     bool hasIterator = false;
     for (var m in node.members) {
       if (m is ConstructorDeclaration) {
         jsMethods.add(_emitConstructor(m, type, fields, isObject));
       } else if (m is MethodDeclaration) {
         jsMethods.add(_emitMethodDeclaration(type, m));
 
+        var methodElement = m.element;
+        if (methodElement is PropertyAccessorElement) {

[Jennifer Messerly, 2016/04/20 21:16:39]

it would be good to refactor this into a method

_emitSuperAccessorWrapper?

[Harry Terkelsen, 2016/04/20 23:33:27]

Done.

+          var fieldElement = methodElement.variable;
+          if (_propertyOverrides.contains(fieldElement)) {

[Jennifer Messerly, 2016/04/20 21:16:39]

here we can just call make a call to ask if our property needed an override. 

+            // Generate a corresponding virtual getter / setter.
+            var name = _elementMemberName(methodElement,
+                allowExtensions: _extensionTypes.contains(type.element));
+            if (m.isGetter) {
+              // Generate a setter
+              var value = new JS.TemporaryId('value');
+              var fn = new JS.Fun(

[Jennifer Messerly, 2016/04/20 21:16:39]

this could be done a bit simpler:

var fn = js.call('function(#) { super.# = #; }', [value, name, value]);

In this case, we know there's no user variable named "value" in scope, so you
could even skip the temp:

var fn = js.call('function(value) { super.# = value; }', [name]);

[Harry Terkelsen, 2016/04/20 23:33:28]

Done.

+                  [value], js.statement('{ super[#] = #; }', [name, value]));
+              jsMethods.add(new JS.Method(name, fn, isSetter: true));
+            } else {
+              // Generate a getter
+              var fn =

[Jennifer Messerly, 2016/04/20 21:16:39]

same here. if using js_ast, I find it's easier to just use it the maximum it
can.

BTW, the reason we need the `new JS.Method` explicitly is I didn't make a
`js.method` helper yet. But we can parse methods so it likely wouldn't be hard,
if we wanted to do that. That would simplify even further:

    jsMethods.add(js.method('#() { return super.#; }', name, name));

I think the "hard" part would be JS_AST parser allowing #-interpolation for the
method name.

[Harry Terkelsen, 2016/04/20 23:33:27]

Done.

+                  new JS.Fun([], js.statement('{ return super[#]; }', [name]));
+              jsMethods.add(new JS.Method(name, fn, isGetter: true));
+            }
+          }
+        }
+
         if (!hasJsPeer && m.isGetter && m.name.name == 'iterator') {
           hasIterator = true;
           jsMethods.add(_emitIterable(type));
         }
       } else if (m is FieldDeclaration && _extensionTypes.contains(element)) {
         jsMethods.addAll(_emitNativeFieldAccessors(m));
       }
     }
 
     // If the type doesn't have an `iterator`, but claims to implement Iterable,
@@ skipping 2962 matching lines @@
 }
 
 bool isLibraryPrefix(Expression node) =>
     node is SimpleIdentifier && node.staticElement is PrefixElement;
 
 LibraryElement _getLibrary(AnalysisContext c, String uri) =>
     c.computeLibraryElement(c.sourceFactory.forUri(uri));
 
 bool _isDartRuntime(LibraryElement l) =>
     l.isInSdk && l.source.uri.toString() == 'dart:_runtime';