make function name inferrence smarter - fixes #1367

src/babel/transformation/helpers/name-method.js

@@ -20,26 +20,32 @@ var visitor = {
 
 var wrap = function (state, method, id, scope) {
   if (state.selfReference) {
-    var templateName = "property-method-assignment-wrapper";
-    if (method.generator) templateName += "-generator";
-    var template = util.template(templateName, {
-      FUNCTION: method,
-      FUNCTION_ID: id,
-      FUNCTION_KEY: scope.generateUidIdentifier(id.name)
-    });
-    template.callee._skipModulesRemap = true;
-
-    // shim in dummy params to retain function arity, if you try to read the
-    // source then you'll get the original since it's proxied so it's all good
-    var params = template.callee.body.body[0].params;
-    for (var i = 0, len = getFunctionArity(method); i < len; i++) {
-      params.push(scope.generateUidIdentifier("x"));
+    if (scope.hasBinding(id.name)) {
+      // we can just munge the local binding
+      scope.rename(id.name);
+    } else {
+      // need to add a wrapper since we can't change the references
+      var templateName = "property-method-assignment-wrapper";
+      if (method.generator) templateName += "-generator";
+      var template = util.template(templateName, {
+        FUNCTION: method,
+        FUNCTION_ID: id,
+        FUNCTION_KEY: scope.generateUidIdentifier(id.name)
+      });
+      template.callee._skipModulesRemap = true;
+
+      // shim in dummy params to retain function arity, if you try to read the
+      // source then you'll get the original since it's proxied so it's all good
+      var params = template.callee.body.body[0].params;
+      for (var i = 0, len = getFunctionArity(method); i < len; i++) {
+        params.push(scope.generateUidIdentifier("x"));
+      }
+
+      return template;
     }
-
-    return template;
-  } else {
-    method.id = id;
   }
+
+  method.id = id;
 };
 
 var visit = function (node, name, scope) {
@@ -117,6 +123,15 @@ export function bare(node, parent, scope) {
   } else if (t.isVariableDeclarator(parent)) {
     // var foo = function () {};
     id = parent.id;
+
+    if (t.isIdentifier(id)) {
+      var bindingInfo = scope.parent.getBinding(id.name);
+      if (bindingInfo && bindingInfo.constant && scope.getBinding(id.name) === bindingInfo) {
+        // always going to reference this method
+        node.id = id;
+        return;
+      }
+    }
   } else {
     return;
   }

src/babel/transformation/transformers/index.js

@@ -1,4 +1,6 @@
 export default {
+  _modules:                                require("./internal/modules"),
+
   "es7.classProperties":                   require("./es7/class-properties"),
   "es7.trailingFunctionCommas":            require("./es7/trailing-function-commas"),
   "es7.asyncFunctions":                    require("./es7/async-functions"),
@@ -24,8 +26,6 @@ export default {
   reactCompat:                             require("./other/react-compat"),
   react:                                   require("./other/react"),
 
-  _modules:                                require("./internal/modules"),
-
   // needs to be before `regenerator` due to generator comprehensions
   // needs to be before `_shadowFunctions`
   "es7.comprehensions":                    require("./es7/comprehensions"),

src/babel/traversal/explode.js

@@ -0,0 +1,19 @@
+import * as t from "../types";
+
+export default function (obj) {
+  for (var type in obj) {
+    var fns = obj[type];
+    if (typeof fns === "function") {
+      obj[type] = fns = { enter: fns };
+    }
+
+    var aliases = t.FLIPPED_ALIAS_KEYS[type];
+    if (aliases) {
+      for (var i = 0; i < aliases.length; i++) {
+        var alias = aliases[i];
+        obj[alias] = obj[alias] || fns;
+      }
+    }
+  }
+  return obj;
+}

src/babel/traversal/index.js

@@ -1,4 +1,5 @@
 import TraversalContext from "./context";
+import explode from "./explode";
 import * as messages from "../messages";
 import includes from "lodash/collection/includes";
 import * as t from "../types";
@@ -112,23 +113,7 @@ traverse.removeProperties = function (tree) {
   return tree;
 };
 
-traverse.explode = function (obj) {
-  for (var type in obj) {
-    var fns = obj[type];
-    if (typeof fns === "function") {
-      obj[type] = fns = { enter: fns };
-    }
-
-    var aliases = t.FLIPPED_ALIAS_KEYS[type];
-    if (aliases) {
-      for (var i = 0; i < aliases.length; i++) {
-        var alias = aliases[i];
-        obj[alias] = obj[alias] || fns;
-      }
-    }
-  }
-  return obj;
-};
+traverse.explode = explode;
 
 function hasBlacklistedType(node, parent, scope, state) {
   if (node.type === state.type) {

src/babel/traversal/path/index.js

@@ -696,7 +696,11 @@ export default class TraversalPath {
       if (part === ".") {
         path = path.parentPath;
       } else {
-        path = path.get(parts[i]);
+        if (Array.isArray(path)) {
+          path = path[part];
+        } else {
+          path = path.get(part);
+        }
       }
     }
     return path;
@@ -707,7 +711,12 @@ export default class TraversalPath {
    */
 
   has(key): boolean {
-    return !!this.node[key];
+    var val = this.node[key];
+    if (val && Array.isArray(val)) {
+      return !!val.length;
+    } else {
+      return !!val;
+    }
   }
 
   /**

src/babel/traversal/scope.js

@@ -1,4 +1,5 @@
 import includes from "lodash/collection/includes";
+import explode from "./explode";
 import traverse from "./index";
 import defaults from "lodash/object/defaults";
 import * as messages from "../messages";
@@ -64,6 +65,55 @@ var blockVariableVisitor = {
   }
 };
 
+var renameVisitor = explode({
+  Identifier(node, parent, scope, state) {
+    if (this.isReferenced() && node.name === state.oldName) {
+      node.name = state.newName;
+    }
+  },
+
+  Declaration(node, parent, scope, state) {
+    var ids = {};
+
+    var matchesLocal = (node, key) => {
+      return node.local === node[key] && (node.local.name === state.oldName || node.local.name === state.newName);
+    };
+
+    if (this.isExportDeclaration() && this.has("specifiers")) {
+      var specifiers = this.get("specifiers");
+      for (var specifier of (specifiers: Array)) {
+        if (specifier.isExportSpecifier() && matchesLocal(specifier.node, "exported")) {
+          specifier.get("exported").replaceWith(t.identifier(state.oldName));
+        }
+      }
+    } else if (this.isImportDeclaration() && this.has("specifiers")) {
+      var specifiers = this.get("specifiers");
+      for (var specifier of (specifiers: Array)) {
+        if (specifier.isImportSpecifier() && matchesLocal(specifier.node, "imported")) {
+          state.binding = state.info.identifier = t.identifier(state.newName);
+          specifier.get("local").replaceWith(state.binding);
+        } else {
+          extend(ids, specifier.getBindingIdentifiers());
+        }
+      }
+    } else {
+      ids = this.getBindingIdentifiers();
+    }
+
+    for (var name in ids) {
+      if (name === state.oldName) ids[name].name = state.newName;
+    }
+  },
+
+  Scopable(node, parent, scope, state) {
+    if (this.isScope()) {
+      if (!scope.bindingIdentifierEquals(state.oldName, state.binding)) {
+        this.skip();
+      }
+    }
+  }
+});
+
 export default class Scope {
 
   /**
@@ -260,31 +310,33 @@ export default class Scope {
     var info = this.getBinding(oldName);
     if (!info) return;
 
-    var binding = info.identifier;
-    var scope   = info.scope;
-
-    scope.traverse(block || scope.block, {
-      enter(node, parent, scope) {
-        if (t.isReferencedIdentifier(node, parent) && node.name === oldName) {
-          node.name = newName;
-        } else if (t.isDeclaration(node)) {
-          var ids = this.getBindingIdentifiers();
-          for (var name in ids) {
-            if (name === oldName) ids[name].name = newName;
-          }
-        } else if (this.isScope()) {
-          if (!scope.bindingIdentifierEquals(oldName, binding)) {
-            this.skip();
-          }
-        }
-      }
-    });
+    var state = {
+      newName: newName,
+      oldName: oldName,
+      binding: info.identifier,
+      info:    info
+    };
+
+    var scope = info.scope;
+    scope.traverse(block || scope.block, renameVisitor, state);
 
     if (!block) {
       scope.removeOwnBinding(oldName);
       scope.bindings[newName] = info;
+      state.binding.name = newName;
+    }
+
+    var file = this.file;
+    if (file) {
+      this._renameFromMap(file.moduleFormatter.localImports, oldName, newName, state.binding);
+      //this._renameFromMap(file.moduleFormatter.localExports, oldName, newName);
+    }
+  }
 
-      binding.name = newName;
+  _renameFromMap(map, oldName, newName, value) {
+    if (map[oldName]) {
+      map[newName] = value;
+      map[oldName] = null;
     }
   }
 

test/core/fixtures/transformation/spec.function-name/assignment/expected.js

@@ -1,18 +1,8 @@
 "use strict";
 
-var i = (function (_i) {
-  function i() {
-    return _i.apply(this, arguments);
-  }
-
-  i.toString = function () {
-    return _i.toString();
-  };
-
-  return i;
-})(function () {
-  i = 5;
-});
+var _i = function i() {
+  _i = 5;
+};
 
 var j = function j() {
   var _ = 5;

test/core/fixtures/transformation/spec.function-name/export/actual.js

@@ -0,0 +1,7 @@
+export var foo = "yes";
+
+var bar = {
+  foo: function () {
+    foo;
+  }
+};

test/core/fixtures/transformation/spec.function-name/export/expected.js

@@ -0,0 +1,10 @@
+"use strict";
+
+var _foo = "yes";
+
+export { _foo as foo };
+var bar = {
+  foo: function foo() {
+    _foo;
+  }
+};

test/core/fixtures/transformation/spec.function-name/export/options.json

@@ -0,0 +1,4 @@
+{
+  "noCheckAst": true,
+  "blacklist": ["es6.modules"]
+}

test/core/fixtures/transformation/spec.function-name/modules-2/actual.js

@@ -2,7 +2,10 @@ import last from "lodash/array/last"
 
 export default class Container {
   last(key) {
-    if (!this.has(key)) return
+    if (!this.has(key)) {
+      return;
+    }
+
     return last(this.tokens.get(key))
   }
 }

test/core/fixtures/transformation/spec.function-name/modules-2/expected.js

@@ -15,20 +15,13 @@ var Container = (function () {
 
   babelHelpers.createClass(Container, [{
     key: "last",
-    value: (function (_last) {
-      function last(_x) {
-        return _last.apply(this, arguments);
+    value: function last(key) {
+      if (!this.has(key)) {
+        return;
       }
 
-      last.toString = function () {
-        return _last.toString();
-      };
-
-      return last;
-    })(function (key) {
-      if (!this.has(key)) return;
       return _last3["default"](this.tokens.get(key));
-    })
+    }
   }]);
   return Container;
 })();

test/core/fixtures/transformation/spec.function-name/modules-3/expected.js

@@ -18,19 +18,9 @@ var Login = (function (_React$Component) {
   babelHelpers.inherits(Login, _React$Component);
   babelHelpers.createClass(Login, [{
     key: "getForm",
-    value: (function (_getForm) {
-      function getForm() {
-        return _getForm.apply(this, arguments);
-      }
-
-      getForm.toString = function () {
-        return _getForm.toString();
-      };
-
-      return getForm;
-    })(function () {
+    value: function getForm() {
       return _getForm2.getForm().toJS();
-    })
+    }
   }]);
   return Login;
 })(React.Component);