[refactor] js_codegen: remove currentClass, simplify visitSimpleIdentifier

lib/src/codegen/js_codegen.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.
 
 library dev_compiler.src.codegen.js_codegen;
 
 import 'dart:collection' show HashSet, HashMap;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 38 matching lines @@
   final TypeRules rules;
 
   /// The global extension method table.
   final HashMap<String, List<InterfaceType>> _extensionMethods;
 
   /// The variable for the target of the current `..` cascade expression.
   SimpleIdentifier _cascadeTarget;
   /// The variable for the current catch clause
   SimpleIdentifier _catchParameter;
 
-  ClassDeclaration currentClass;
   ConstantEvaluator _constEvaluator;
 
   final _exports = new Set<String>();
   final _lazyFields = <VariableDeclaration>[];
   final _properties = <FunctionDeclaration>[];
   final _privateNames = new HashMap<String, JSTemporary>();
   final _pendingPrivateNames = <JSTemporary>[];
   final _extensionMethodNames = new HashSet<String>();
   final _pendingExtensionMethodNames = <String>[];
   final _temps = new HashMap<Element, JSTemporary>();
@@ skipping 246 matching lines @@
 
   @override
   JS.Statement visitClassDeclaration(ClassDeclaration node) {
     // If we've already emitted this class, skip it.
     var type = node.element.type;
     if (_pendingClasses.remove(node.element) == null) return null;
 
     var jsName = getAnnotationValue(node, _isJsNameAnnotation);
     if (jsName != null) return _emitJsType(node.name.name, jsName);
 
-    currentClass = node;
-
     var ctors = <ConstructorDeclaration>[];
     var fields = <FieldDeclaration>[];
     var staticFields = <FieldDeclaration>[];
     for (var member in node.members) {
       if (member is ConstructorDeclaration) {
         ctors.add(member);
       } else if (member is FieldDeclaration) {
         (member.isStatic ? staticFields : fields).add(member);
       }
     }
 
     var classExpr = new JS.ClassExpression(new JS.Identifier(type.name),
         _classHeritage(node), _emitClassMethods(node, ctors, fields));
 
     var body =
         _finishClassMembers(node.element, classExpr, ctors, staticFields);
-    currentClass = null;
 
     return _finishClassDef(type, body);
   }
 
   @override
   JS.Statement visitEnumDeclaration(EnumDeclaration node) =>
       _unimplementedCall("Unimplemented enum: $node").toStatement();
 
   /// Given a class element and body, complete the class declaration.
   /// This handles generic type parameters, laziness (in library-cycle cases),
@@ skipping 627 matching lines @@
     return new JS.Block([
       js.comment("// Function ${func.name.name}: ${func.element.type}\n"),
       new JS.FunctionDeclaration(name, _visit(func.functionExpression))
     ]);
   }
 
   /// Writes a simple identifier. This can handle implicit `this` as well as
   /// going through the qualified library name if necessary.
   @override
   JS.Expression visitSimpleIdentifier(SimpleIdentifier node) {
-    var e = node.staticElement;
-    if (e == null) {
+    var accessor = node.staticElement;
+    if (accessor == null) {
       return js.commentExpression(
           'Unimplemented unknown name', new JS.Identifier(node.name));
     }
 
-    var variable = e is PropertyAccessorElement ? e.variable : e;

[Jennifer Messerly, 2015/04/16 22:50:30]

"variable" was misleading ... this is just the "real" original declaring
element. We ignore the less useful PropertyAccess (strangely they aren't
ClassMemberElements)

-    var name = variable.name;
+    // Get the original declaring element. If we had a property accessor, this
+    // indirects back to a (possibly synthetic) field.
+    var element = accessor;
+    if (element is PropertyAccessorElement) element = accessor.variable;

[Jennifer Messerly, 2015/04/16 22:50:30]

after this point we always use `element`.

+    var name = element.name;
 
     // library member
-    if (e.enclosingElement is CompilationUnitElement &&
-        (e.library != libraryInfo.library ||
-            variable is TopLevelVariableElement && !variable.isConst)) {
-      return js.call('#.#', [_libraryName(e.library), name]);
+    if (element.enclosingElement is CompilationUnitElement &&
+        (element.library != libraryInfo.library ||
+            element is TopLevelVariableElement && !element.isConst)) {
+      return js.call('#.#', [_libraryName(element.library), name]);
     }
 
-    // instance member
-    if (currentClass != null && _needsImplicitThis(e)) {
-      return js.call(
-          'this.#', _emitMemberName(name, type: currentClass.element.type));
-    }
+    // Unqualified class member. This could mean implicit-this, or implicit
+    // call to a static from the same class.
+    if (element is ClassMemberElement && element is! ConstructorElement) {
+      bool isStatic = element.isStatic;
+      var type = element.enclosingElement.type;
 
-    // static member
-    if (e is ExecutableElement &&
-        e.isStatic &&
-        variable.enclosingElement is ClassElement) {
-      var className = (variable.enclosingElement as ClassElement).name;
-      return js.call('#.#', [className, _emitMemberName(name, isStatic: true)]);
+      // For instance methods, we add implicit-this.
+      // For static methods, we add the raw type name, without generics or
+      // library prefix. We don't need those because static calls can't use
+      // the generic type.
+      var target = isStatic ? new JS.Identifier(type.name) : new JS.This();
+      var member = _emitMemberName(name, isStatic: isStatic, type: type);
+      return new JS.PropertyAccess(target, member);
     }
 
     // initializing formal parameter, e.g. `Point(this.x)`
-    if (e is ParameterElement && e.isInitializingFormal && e.isPrivate) {
+    if (element is ParameterElement &&
+        element.isInitializingFormal &&
+        element.isPrivate) {
       /// Rename private names so they don't shadow the private field symbol.
       /// The renamer would handle this, but it would prefer to rename the
       /// temporary used for the private symbol. Instead rename the parameter.
-      return _getTemp(e, '${name.substring(1)}');
+      return _getTemp(element, '${name.substring(1)}');
     }
 
-    if (_isTemporary(e)) {
+    if (_isTemporary(element)) {
       if (name[0] == '#') {
         return new JS.InterpolatedExpression(name.substring(1));
       } else {
-        return _getTemp(e, name);
+        return _getTemp(element, name);
       }
     }
 
     return new JS.Identifier(name);
   }
 
   JSTemporary _getTemp(Object key, String name) =>
       _temps.putIfAbsent(key, () => new JSTemporary(name));
 
   JS.ArrayInitializer _emitTypeNames(List<DartType> types) {
@@ skipping 1235 matching lines @@
 
   /// Choose a canonical name from the library element.
   /// This never uses the library's name (the identifier in the `library`
   /// declaration) as it doesn't have any meaningful rules enforced.
   JS.Identifier _libraryName(LibraryElement library) {
     if (library == libraryInfo.library) return _exportsVar;
     return new JS.Identifier(jsLibraryName(library));
   }
 
   DartType getStaticType(Expression e) => rules.getStaticType(e);
-
-  static bool _needsImplicitThis(Element e) =>
-      e is PropertyAccessorElement && !e.variable.isStatic ||
-          e is ClassMemberElement && !e.isStatic && e is! ConstructorElement;
 }
 
 class JSGenerator extends CodeGenerator {
   final JSCodeOptions options;
 
   /// For fast lookup of extension methods, we first check the name, then do a
   /// (possibly expensive) subtype test to see if it matches one of the types
   /// that declares that method.
   final _extensionMethods = new HashMap<String, List<InterfaceType>>();
 
@@ skipping 55 matching lines @@
   return filepath;
 }
 
 // TODO(jmesserly): validate the library. See issue #135.
 bool _isJsNameAnnotation(DartObjectImpl value) => value.type.name == 'JsName';
 
 // TODO(jacobr): we would like to do something like the following
 // but we don't have summary support yet.
 // bool _supportJsExtensionMethod(AnnotatedNode node) =>
 //    _getAnnotation(node, "SupportJsExtensionMethod") != null;