dart2js: Refactoring, documentation, and a few bugfixes in Namer class.

pkg/compiler/lib/src/js_backend/minify_namer.dart

 // Copyright (c) 2011, 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.
 
 part of js_backend;
 
 /**
  * Assigns JavaScript identifiers to Dart variables, class-names and members.
  */
 class MinifyNamer extends Namer {
@@ skipping 13 matching lines @@
   final ALPHABET_CHARACTERS = 52;  // a-zA-Z.
   final ALPHANUMERIC_CHARACTERS = 62;  // a-zA-Z0-9.
 
   _FieldNamingRegistry fieldRegistry;
 
   // You can pass an invalid identifier to this and unlike its non-minifying
   // counterpart it will never return the proposedName as the new fresh name.
   String getFreshName(String proposedName,
                       Set<String> usedNames,
                       Map<String, String> suggestedNames,
-                      {bool ensureSafe: true}) {
-    var freshName;
-    var suggestion = suggestedNames[proposedName];
+                      {bool ensureSafe, bool sanitizeForAnnotations}) {
+    String freshName;
+    String suggestion = suggestedNames[proposedName];
     if (suggestion != null && !usedNames.contains(suggestion)) {
       freshName = suggestion;
     } else {
       freshName = _getUnusedName(proposedName, usedNames,
           suggestedNames.values);
     }
     usedNames.add(freshName);
     return freshName;
   }
 
-  String getClosureVariableName(String name, int id) {
+  String getClosureVariableName(String _, int id) {
     if (id < ALPHABET_CHARACTERS) {
-      return new String.fromCharCodes([_letterNumber(id)]);
+      String name = new String.fromCharCodes([_letterNumber(id)]);
+      // The names 'g' and 's' are the getter/setter for the 'call' property.
+      // These cannot be used as a closure fields.
+      if (!_hasBannedPrefix(name)) return name;

[asgerf, 2015/01/30 15:20:29]

I believe this was a bug, although I admit I was never able to reproduce it on
master.

     }
-    return "${getMappedInstanceName('closure')}_$id";
+    // Fall back to a slightly longer name.
+    String basename = disambiguateMember(null, 'closure');
+    return '${basename}_$id';
   }
 
   // From issue 7554.  These should not be used on objects (as instance
   // variables) because they clash with names from the DOM. However, it is
   // OK to use them as fields, as we only access fields directly if we know
   // the receiver type.
-  static const _reservedNativeProperties = const <String>[
+  static const List<String> _reservedNativeProperties = const <String>[
       'Q', 'a', 'b', 'c', 'd', 'e', 'f', 'r', 'x', 'y', 'z',
       // 2-letter:
       'ch', 'cx', 'cy', 'db', 'dx', 'dy', 'fr', 'fx', 'fy', 'go', 'id', 'k1',
       'k2', 'k3', 'k4', 'r1', 'r2', 'rx', 'ry', 'x1', 'x2', 'y1', 'y2',
       // 3-letter:
       'add', 'all', 'alt', 'arc', 'CCW', 'cmp', 'dir', 'end', 'get', 'in1',
       'in2', 'INT', 'key', 'log', 'low', 'm11', 'm12', 'm13', 'm14', 'm21',
       'm22', 'm23', 'm24', 'm31', 'm32', 'm33', 'm34', 'm41', 'm42', 'm43',
       'm44', 'max', 'min', 'now', 'ONE', 'put', 'red', 'rel', 'rev', 'RGB',
       'sdp', 'set', 'src', 'tag', 'top', 'uid', 'uri', 'url', 'URL',
       // 4-letter:
       'abbr', 'atob', 'Attr', 'axes', 'axis', 'back', 'BACK', 'beta', 'bias',
       'Blob', 'blue', 'blur', 'BLUR', 'body', 'BOOL', 'BOTH', 'btoa', 'BYTE',
       'cite', 'clip', 'code', 'cols', 'cues', 'data', 'DECR', 'DONE', 'face',
       'file', 'File', 'fill', 'find', 'font', 'form', 'gain', 'hash', 'head',
       'high', 'hint', 'host', 'href', 'HRTF', 'IDLE', 'INCR', 'info', 'INIT',
       'isId', 'item', 'KEEP', 'kind', 'knee', 'lang', 'left', 'LESS', 'line',
       'link', 'list', 'load', 'loop', 'mode', 'name', 'Node', 'None', 'NONE',
       'only', 'open', 'OPEN', 'ping', 'play', 'port', 'rect', 'Rect', 'refX',
       'refY', 'RGBA', 'root', 'rows', 'save', 'seed', 'seek', 'self', 'send',
       'show', 'SINE', 'size', 'span', 'stat', 'step', 'stop', 'tags', 'text',
       'Text', 'time', 'type', 'view', 'warn', 'wrap', 'ZERO'];
 
   void reserveBackendNames() {
-    for (var name in _reservedNativeProperties) {
+    Map<String,String> publicMembers =
+        instanceMembers.putIfAbsent(null, () => new Map<String, String>());
+    for (String name in _reservedNativeProperties) {
       if (name.length < 2) {
-        instanceNameMap[name] = name;
+        publicMembers[name] = name;
       }
       usedInstanceNames.add(name);
       // Getter and setter names are autogenerated by prepending 'g' and 's' to
       // field names.  Therefore there are some field names we don't want to
       // use.  It is implicit in the next line that the banned prefix is
       // only one character.
       if (_hasBannedPrefix(name)) usedInstanceNames.add(name.substring(1));
     }
 
     // These popular names are present in most programs and deserve
@@ skipping 41 matching lines @@
   void _populateSuggestedNames(Map<String, String> suggestionMap,
                                Set<String> used,
                                List<String> suggestions) {
     int c = $a - 1;
     String letter;
     for (String name in suggestions) {
       do {
         assert(c != $Z);
         c = (c == $z) ? $A : c + 1;
         letter = new String.fromCharCodes([c]);
-      } while (used.contains(letter));
+      } while (used.contains(letter) || _hasBannedPrefix(letter));

[asgerf, 2015/01/30 15:20:29]

Same as above, it can clash with the getter for the 'call' property.

I believe this was also a bug. It caused problems on my branch, but again, for
some reason I could not produce it on master.

       assert(suggestionMap[name] == null);
       suggestionMap[name] = letter;
     }
   }
 
 
   // This gets a minified name based on a hash of the proposed name.  This
   // is slightly less efficient than just getting the next name in a series,
   // but it means that small changes in the input program will give smallish
   // changes in the output, which can be useful for diffing etc.
   String _getUnusedName(String proposedName, Set<String> usedNames,
                         Iterable<String> suggestions) {
     int hash = _calculateHash(proposedName);
     // Avoid very small hashes that won't try many names.
     hash = hash < 1000 ? hash * 314159 : hash;  // Yes, it's prime.
 
     // Try other n-character names based on the hash.  We try one to three
     // character identifiers.  For each length we try around 10 different names
     // in a predictable order determined by the proposed name.  This is in order
     // to make the renamer stable: small changes in the input should nornally
     // result in relatively small changes in the output.
-    for (var n = 1; n <= 3; n++) {
+    for (int n = 1; n <= 3; n++) {
       int h = hash;
       while (h > 10) {
-        var codes = <int>[_letterNumber(h)];
+        List<int> codes = <int>[_letterNumber(h)];
         int h2 = h ~/ ALPHABET_CHARACTERS;
-        for (var i = 1; i < n; i++) {
+        for (int i = 1; i < n; i++) {
           codes.add(_alphaNumericNumber(h2));
           h2 ~/= ALPHANUMERIC_CHARACTERS;
         }
         final candidate = new String.fromCharCodes(codes);
         if (!usedNames.contains(candidate) &&
             !jsReserved.contains(candidate) &&
             !_hasBannedPrefix(candidate) &&
             (n != 1 || !suggestions.contains(candidate))) {
           return candidate;
         }
@@ skipping 21 matching lines @@
       h &= 0xffffffff;
       h ^= h >> 6;
       h &= 0xffffffff;
     }
     return h;
   }
 
   /// If we can't find a hash based name in the three-letter space, then base
   /// the name on a letter and a counter.
   String _badName(int hash, Set<String> usedNames) {
-    var startLetter = new String.fromCharCodes([_letterNumber(hash)]);
-    var name;
-    var i = 0;
+    String startLetter = new String.fromCharCodes([_letterNumber(hash)]);
+    String name;
+    int i = 0;
     do {
       name = "$startLetter${i++}";
     } while (usedNames.contains(name));
     // We don't need to check for banned prefix because the name is in the form
     // xnnn, where nnn is a number.  There can be no getter or setter called
     // gnnn since that would imply a numeric field name.
     return name;
   }
 
   int _letterNumber(int x) {
@@ skipping 186 matching lines @@
   Map<Entity, String> get names => registry.globalNames;
 
   _MixinFieldNamingScope.mixin(ClassElement cls, _FieldNamingRegistry registry)
     : super.rootScope(cls, registry);
 
   _MixinFieldNamingScope.mixedIn(MixinApplicationElement container,
       _FieldNamingScope superScope, _FieldNamingRegistry registry)
     : super.inherit(container, superScope, registry);
 
   String _nextName() {
-    var proposed = super._nextName();
+    String proposed = super._nextName();
     return proposed + r'$';
   }
 }
 
 /**
  * [BoxFieldElement] fields work differently in that they do not belong to an
  * actual class but an anonymous box associated to a [Local]. As there is no
  * inheritance chain, we do not need to compute fields a priori but can assign
  * names on the fly.
  */
 class _BoxFieldNamingScope extends _FieldNamingScope {
   _BoxFieldNamingScope(Local box, _FieldNamingRegistry registry) :
     super.rootScope(box, registry);
 
   bool containsField(_) => true;
 
   String operator[](Element field) {
     if (!names.containsKey(field)) add(field);
     return names[field];
   }
 }