fixes #427, static fields emitted outside the scope of their class

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, SplayTreeSet;
 
 import 'package:analyzer/analyzer.dart' hide ConstantEvaluator;
 import 'package:analyzer/src/generated/ast.dart' hide ConstantEvaluator;
@@ skipping 62 matching lines @@
 
   /// The variable for the current catch clause
   SimpleIdentifier _catchParameter;
 
   /// In an async* function, this represents the stream controller parameter.
   JS.TemporaryId _asyncStarController;
 
   /// Imported libraries, and the temporaries used to refer to them.
   final _imports = new Map<LibraryElement, JS.TemporaryId>();
   final _exports = new Set<String>();
-  final _lazyFields = <VariableDeclaration>[];
   final _properties = <FunctionDeclaration>[];
   final _privateNames = new HashMap<String, JS.TemporaryId>();
   final _moduleItems = <JS.Statement>[];
   final _temps = new HashMap<Element, JS.TemporaryId>();
   final _qualifiedIds = new List<Tuple2<Element, JS.MaybeQualifiedId>>();
 
   /// The name for the library's exports inside itself.
   /// `exports` was chosen as the most similar to ES module patterns.
   final _dartxVar = new JS.Identifier('dartx');
   final _exportsVar = new JS.TemporaryId('exports');
@@ skipping 53 matching lines @@
 
     for (var unit in library.partsThenLibrary) {
       _constField = new ConstFieldVisitor(types, unit);
 
       for (var decl in unit.declarations) {
         if (decl is TopLevelVariableDeclaration) {
           visitTopLevelVariableDeclaration(decl);
         } else {
           _loader.loadDeclaration(decl, decl.element);
         }
-        if (decl is ClassDeclaration) {
-          // Static fields can be emitted into the top-level code, so they need
-          // to potentially be ordered independently of the class.
-          for (var member in decl.members) {
-            if (member is FieldDeclaration) {
-              visitFieldDeclaration(member);
-            }
-          }
-        }
       }
     }
 
     // Flush any unwritten fields/properties.
-    _flushLazyFields(_moduleItems);
     _flushLibraryProperties(_moduleItems);
 
     // Mark all qualified names as qualified or not, depending on if they need
     // to be loaded lazily or not.
     for (var elementIdPairs in _qualifiedIds) {
       var element = elementIdPairs.e0;
       var id = elementIdPairs.e1;
       id.setQualified(!_loader.isLoaded(element));
     }
 
@@ skipping 52 matching lines @@
     // TODO(jmesserly): scriptTag support.
     // Enable this if we know we're targetting command line environment?
     // It doesn't work in browser.
     // var jsBin = compiler.options.runnerOptions.v8Binary;
     // String scriptTag = null;
     // if (library.library.scriptTag != null) scriptTag = '/usr/bin/env $jsBin';
     return new JS.Program(<JS.Statement>[moduleDef]);
   }
 
   void _emitModuleItem(AstNode node) {
-    // Attempt to group adjacent fields/properties.
-    if (node is! VariableDeclaration) _flushLazyFields(_moduleItems);
+    // Attempt to group adjacent properties.
     if (node is! FunctionDeclaration) _flushLibraryProperties(_moduleItems);
 
     var code = _visit(node);
     if (code != null) _moduleItems.add(code);
   }
 
   @override
   void visitLibraryDirective(LibraryDirective node) {
     assert(_jsModuleValue == null);
 
@@ skipping 172 matching lines @@
   @override
   JS.Statement visitClassDeclaration(ClassDeclaration node) {
     var classElem = node.element;
     var type = classElem.type;
     var jsName = findAnnotation(classElem, isJSAnnotation);
 
     if (jsName != null) return _emitJsType(node.name.name, jsName);
 
     var ctors = <ConstructorDeclaration>[];
     var fields = <FieldDeclaration>[];
+    var staticFields = <FieldDeclaration>[];
     var methods = <MethodDeclaration>[];
     for (var member in node.members) {
       if (member is ConstructorDeclaration) {
         ctors.add(member);
-      } else if (member is FieldDeclaration && !member.isStatic) {
-        fields.add(member);
+      } else if (member is FieldDeclaration) {
+        (member.isStatic ? staticFields : fields).add(member);
       } else if (member is MethodDeclaration) {
         methods.add(member);
       }
     }
 
     var classExpr = new JS.ClassExpression(new JS.Identifier(type.name),
         _classHeritage(classElem), _emitClassMethods(node, ctors, fields));
 
     String jsPeerName;
     var jsPeer = findAnnotation(classElem, isJsPeerInterface);
     if (jsPeer != null) {
       jsPeerName =
           getConstantField(jsPeer, 'name', types.stringType)?.toStringValue();
     }
 
-    var body = _finishClassMembers(classElem, classExpr, ctors, fields, methods,
-        node.metadata, jsPeerName);
+    var body = _finishClassMembers(classElem, classExpr, ctors, fields,
+        staticFields, methods, node.metadata, jsPeerName);
 
     var result = _finishClassDef(type, body);
 
     if (jsPeerName != null) {
       // This class isn't allowed to be lazy, because we need to set up
       // the native JS type eagerly at this point.
       // If we wanted to support laziness, we could defer the hookup until
       // the end of the Dart library cycle load.
       assert(_loader.isLoaded(classElem));
 
@@ skipping 228 matching lines @@
   }
 
   /// Emit class members that need to come after the class declaration, such
   /// as static fields. See [_emitClassMethods] for things that are emitted
   /// inside the ES6 `class { ... }` node.
   JS.Statement _finishClassMembers(
       ClassElement classElem,
       JS.ClassExpression cls,
       List<ConstructorDeclaration> ctors,
       List<FieldDeclaration> fields,
+      List<FieldDeclaration> staticFields,
       List<MethodDeclaration> methods,
       List<Annotation> metadata,
       String jsPeerName) {
     var name = classElem.name;
     var body = <JS.Statement>[];
 
     if (_extensionTypes.contains(classElem)) {
       var dartxNames = <JS.Expression>[];
       for (var m in methods) {
         if (!m.isAbstract && !m.isStatic && m.element.isPublic) {
@@ skipping 30 matching lines @@
     }
 
     // Named constructors
     for (ConstructorDeclaration member in ctors) {
       if (member.name != null && member.factoryKeyword == null) {
         body.add(js.statement('dart.defineNamedConstructor(#, #);',
             [name, _emitMemberName(member.name.name, isStatic: true)]));
       }
     }
 
-    // Instance fields, if they override getter/setter pairs
+    // Emits instance fields, if they are virtual
+    // (in other words, they override a getter/setter pair).
     for (FieldDeclaration member in fields) {
-      for (VariableDeclaration fieldDecl in member.fields.variables) {
-        var field = fieldDecl.element as FieldElement;
-        if (_fieldsNeedingStorage.contains(field)) {
-          body.add(_overrideField(field));
+      for (VariableDeclaration field in member.fields.variables) {
+        if (_fieldsNeedingStorage.contains(field.element)) {
+          body.add(_overrideField(field.element));
         }
       }
     }
 
     // Emit the signature on the class recording the runtime type information
     var extensions = _extensionsToImplement(classElem);
     {
       var tStatics = <JS.Property>[];
       var tMethods = <JS.Property>[];
       var sNames = <JS.Expression>[];
@@ skipping 69 matching lines @@
     // TODO(vsm): Make this optional per #268.
     // Metadata
     if (metadata.isNotEmpty) {
       body.add(js.statement('#[dart.metadata] = () => #;', [
         name,
         new JS.ArrayInitializer(
             new List<JS.Expression>.from(metadata.map(_instantiateAnnotation)))
       ]));
     }
 
+    // Emits static fields. These are eager initialized if possible, otherwise
+    // they are made static.

[vsm, 2016/01/27 14:29:29]

"made lazy"?

[Jennifer Messerly, 2016/01/27 17:26:28]

good catch, done!

+    var lazyStatics = <VariableDeclaration>[];
+    for (FieldDeclaration member in staticFields) {
+      for (VariableDeclaration field in member.fields.variables) {
+        JS.Statement eagerField = _emitConstantStaticField(classElem, field);
+        if (eagerField != null) {
+          body.add(eagerField);
+        } else {
+          lazyStatics.add(field);
+        }
+      }
+    }
+    if (lazyStatics.isNotEmpty) {
+      body.add(_emitLazyFields(classElem, lazyStatics));
+    }
+
     return _statement(body);
   }
 
   List<ExecutableElement> _extensionsToImplement(ClassElement element) {
     var members = <ExecutableElement>[];
     if (_extensionTypes.contains(element)) return members;
 
     // Collect all extension types we implement.
     var type = element.type;
     var types = new Set<ClassElement>();
@@ skipping 1279 matching lines @@
     return new JS.Yield(_visit(node.expression));
   }
 
   @override
   visitTopLevelVariableDeclaration(TopLevelVariableDeclaration node) {
     for (var v in node.variables.variables) {
       _loader.loadDeclaration(v, v.element);
     }
   }
 
-  /// Emits static fields.
-  ///
-  /// Instance fields are emitted in [_initializeFields].
-  ///
-  /// These are generally treated the same as top-level fields, see
-  /// [visitTopLevelVariableDeclaration].
-  @override
-  visitFieldDeclaration(FieldDeclaration node) {
-    if (!node.isStatic) return;
-
-    for (var f in node.fields.variables) {
-      _loader.loadDeclaration(f, f.element);
-    }
-  }
-
   _addExport(String name) {
     if (!_exports.add(name)) throw 'Duplicate top level name found: $name';
   }
 
   @override
   JS.Statement visitVariableDeclarationStatement(
       VariableDeclarationStatement node) {
     // Special case a single variable with an initializer.
     // This helps emit cleaner code for things like:
     //     var result = []..add(1)..add(2);
     if (node.variables.variables.length == 1) {
       var v = node.variables.variables.single;
       if (v.initializer != null) {
         var name = new JS.Identifier(v.name.name);
         return _visit(v.initializer).toVariableDeclaration(name);
       }
     }
     return _visit(node.variables).toStatement();
   }
 
   @override
   visitVariableDeclarationList(VariableDeclarationList node) {
     return new JS.VariableDeclarationList(
         'let', _visitList(node.variables) as List<JS.VariableInitialization>);
   }
 
   @override
   visitVariableDeclaration(VariableDeclaration node) {
-    if (node.element is PropertyInducingElement) return _emitStaticField(node);
+    if (node.element is PropertyInducingElement) {
+      // Static and instance fields are handled elsewhere.
+      assert(node.element is TopLevelVariableElement);
+      return _emitTopLevelField(node);
+    }
 
     var name = new JS.Identifier(node.name.name);
     return new JS.VariableInitialization(name, _visitInitializer(node));
   }
 
   bool _isFinalJSDecl(AstNode field) =>
       field is VariableDeclaration &&
       field.isFinal &&
       _isJSInvocation(field.initializer);
 
-  /// Emits a static or top-level field.
-  JS.Statement _emitStaticField(VariableDeclaration field) {
+  /// Emits a static field. If the field is a lazy static,
+  JS.Statement _emitConstantStaticField(
+      ClassElement classElem, VariableDeclaration field) {
     PropertyInducingElement element = field.element;
     assert(element.isStatic);
 
+    // If the field is constant, try to avoid the lazy static.
+    _loader.startCheckingReferences();
+    JS.Expression jsInit = _visitInitializer(field);
+    bool isLoaded = _loader.finishCheckingReferences();
+    bool eagerInit =
+        isLoaded && (field.isConst || _constField.isFieldInitConstant(field));

[vsm, 2016/01/27 14:29:29]

Could this be an '||' instead of an '&&'?  I.e., if all references are loaded,
can we eagerly initialize even if the field isn't constant?

[Jennifer Messerly, 2016/01/27 17:26:28]

The initializer must not cause visible side effects if we are to execute it
eagerly. We don't have a way of knowing side effects, so "const" is used as a
proxy.

I had the same thought though, at first. Probably it needs a comment. :)

+
+    var fieldName = field.name.name;
+    if (eagerInit && !JS.invalidStaticFieldName(fieldName)) {
+      return annotateVariable(
+          js.statement('#.# = #;', [
+            classElem.name,
+            _emitMemberName(fieldName, isStatic: true),
+            jsInit
+          ]),
+          field.element);
+    }
+
+    // This means it should be treated as a lazy field.
+    // TODO(jmesserly): we're throwing away the initializer expression,
+    // which will force us to regenerate it.
+    return null;
+  }
+
+  /// Emits a top-level field.
+  JS.Statement _emitTopLevelField(VariableDeclaration field) {
+    TopLevelVariableElement element = field.element;
+    assert(element.isStatic);
+
     bool eagerInit;
     JS.Expression jsInit;
     if (field.isConst || _constField.isFieldInitConstant(field)) {
       // If the field is constant, try and generate it at the top level.
       _loader.startTopLevel(element);
       jsInit = _visitInitializer(field);
       _loader.finishTopLevel(element);
       eagerInit = _loader.isLoaded(element);
     } else {
+      // TODO(jmesserly): we're visiting the initializer here, and again
+      // later on when we emit lazy fields. That seems busted.
       jsInit = _visitInitializer(field);
       eagerInit = false;
     }
 
     // Treat `final x = JS('', '...')` as a const (non-lazy) to help compile
     // runtime helpers.
     var isJSTopLevel = field.isFinal && _isFinalJSDecl(field);
     if (isJSTopLevel) eagerInit = true;
 
-    var fieldName = field.name.name;
-    if (element is TopLevelVariableElement) {
-      fieldName = _getJSExportName(element) ?? fieldName;
-    }
-    if ((field.isConst && eagerInit && element is TopLevelVariableElement) ||
-        isJSTopLevel) {
+    var fieldName = _getJSExportName(element) ?? field.name.name;
+    if (field.isConst && eagerInit || isJSTopLevel) {
       // constant fields don't change, so we can generate them as `let`
       // but add them to the module's exports. However, make sure we generate
       // anything they depend on first.
-
       if (isPublic(fieldName)) _addExport(fieldName);
       var declKeyword = field.isConst || field.isFinal ? 'const' : 'let';
       return annotateVariable(
           js.statement(
               '$declKeyword # = #;', [new JS.Identifier(fieldName), jsInit]),
           field.element);
     }
 
     if (eagerInit && !JS.invalidStaticFieldName(fieldName)) {
       return annotateVariable(
           js.statement('# = #;', [_visit(field.name), jsInit]), field.element);
     }
 
-    var body = <JS.Statement>[];
-    if (_lazyFields.isNotEmpty) {
-      var existingTarget = _lazyFields[0].element.enclosingElement;
-      if (existingTarget != element.enclosingElement) {
-        _flushLazyFields(body);
-      }
-    }
-
-    _lazyFields.add(field);
-
-    return _statement(body);
+    return _emitLazyFields(currentLibrary, [field]);
   }
 
   JS.Expression _visitInitializer(VariableDeclaration node) {
     var value = _visit(node.initializer);
     // explicitly initialize to null, to avoid getting `undefined`.
     // TODO(jmesserly): do this only for vars that aren't definitely assigned.
     return value ?? new JS.LiteralNull();
   }
 
-  void _flushLazyFields(List<JS.Statement> body) {
-    if (_lazyFields.isEmpty) return;
-    body.add(_emitLazyFields(_lazyFields));
-    _lazyFields.clear();
-  }
-
-  JS.Statement _emitLazyFields(List<VariableDeclaration> fields) {
+  JS.Statement _emitLazyFields(
+      Element target, List<VariableDeclaration> fields) {
     var methods = [];
     for (var node in fields) {
       var name = node.name.name;
       var element = node.element;
-      var access = _emitMemberName(name, type: element.type, isStatic: true);
+      var access = _emitMemberName(name, isStatic: true);
       methods.add(annotate(
           new JS.Method(
               access,
               js.call('function() { return #; }', _visit(node.initializer))
               as JS.Fun,
               isGetter: true),
           _findAccessor(element, getter: true)));
 
-      // TODO(jmesserly): use a dummy setter to indicate writable.
-      if (!node.isFinal) {
+      // TODO(jmesserly): currently uses a dummy setter to indicate writable.
+      if (!node.isFinal && !node.isConst) {
         methods.add(annotate(
             new JS.Method(access, js.call('function(_) {}') as JS.Fun,
                 isSetter: true),
             _findAccessor(element, getter: false)));
       }
     }
 
-    JS.Expression objExpr = _exportsVar;
-    var target = _lazyFields[0].element.enclosingElement;
+    JS.Expression objExpr;
     if (target is ClassElement) {
       objExpr = new JS.Identifier(target.type.name);
+    } else {
+      objExpr = _libraryName(target);
     }
 
     return js
         .statement('dart.defineLazyProperties(#, { # });', [objExpr, methods]);
   }
 
   PropertyAccessorElement _findAccessor(VariableElement element,
       {bool getter}) {
     var parent = element.enclosingElement;
     if (parent is ClassElement) {
@@ skipping 527 matching lines @@
     return true;
   }
 
   /// Shared code for [PrefixedIdentifier] and [PropertyAccess].
   JS.Expression _emitGet(Expression target, SimpleIdentifier memberId) {
     var member = memberId.staticElement;
     if (member is PropertyAccessorElement) {
       member = (member as PropertyAccessorElement).variable;
     }
     bool isStatic = member is ClassMemberElement && member.isStatic;
-    if (isStatic) {
-      _loader.declareBeforeUse(member);
-    }
     var name = _emitMemberName(memberId.name,
         type: getStaticType(target), isStatic: isStatic);
     if (DynamicInvoke.get(target)) {
       return js.call('dart.$DLOAD(#, #)', [_visit(target), name]);
     }
 
     String code;
     if (member != null && member is MethodElement && !isStatic) {
       // Tear-off methods: explicitly bind it.
       if (target is SuperExpression) {
@@ skipping 755 matching lines @@
 
 /// A special kind of element created by the compiler, signifying a temporary
 /// variable. These objects use instance equality, and should be shared
 /// everywhere in the tree where they are treated as the same variable.
 class TemporaryVariableElement extends LocalVariableElementImpl {
   TemporaryVariableElement.forNode(Identifier name) : super.forNode(name);
 
   int get hashCode => identityHashCode(this);
   bool operator ==(Object other) => identical(this, other);
 }