Skip to content
This repository has been archived by the owner on Feb 1, 2025. It is now read-only.

Commit

Permalink
Be smarter about renaming catch parameters.
Browse files Browse the repository at this point in the history
Fixes #154.
  • Loading branch information
benjamn committed Dec 20, 2014
1 parent 049a955 commit 52a9dc8
Show file tree
Hide file tree
Showing 3 changed files with 113 additions and 4 deletions.
19 changes: 16 additions & 3 deletions lib/emit.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ var b = types.builders;
var n = types.namedTypes;
var leap = require("./leap");
var meta = require("./meta");
var runtimeProperty = require("./util").runtimeProperty;
var runtimeKeysMethod = runtimeProperty("keys");
var util = require("./util");
var runtimeKeysMethod = util.runtimeProperty("keys");
var hasOwn = Object.prototype.hasOwnProperty;

function Emitter(contextId) {
Expand Down Expand Up @@ -732,10 +732,23 @@ Ep.explodeStatement = function(path, labelId) {

types.visit(bodyPath, {
visitIdentifier: function(path) {
if (path.value.name === catchParamName &&
if (util.isReference(path, catchParamName) &&
path.scope.lookup(catchParamName) === catchScope) {
return safeParam;
}

this.traverse(path);
},

visitFunction: function(path) {
if (path.scope.declares(catchParamName)) {
// Don't descend into nested scopes that shadow the catch
// parameter with their own declarations. This isn't
// logically necessary because of the path.scope.lookup we
// do in visitIdentifier, but it saves time.
return false;
}

this.traverse(path);
}
});
Expand Down
65 changes: 64 additions & 1 deletion lib/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@
* the same directory.
*/

var b = require("recast").types.builders;
var assert = require("assert");
var types = require("recast").types;
var n = types.namedTypes;
var b = types.builders;
var hasOwn = Object.prototype.hasOwnProperty;

exports.defaults = function(obj) {
Expand All @@ -35,3 +38,63 @@ exports.runtimeProperty = function(name) {
false
);
};

// Inspired by the isReference function from ast-util:
// https://github.com/eventualbuddha/ast-util/blob/9bf91c5ce8/lib/index.js#L466-L506
exports.isReference = function(path, name) {
var node = path.value;

if (!n.Identifier.check(node)) {
return false;
}

if (name && node.name !== name) {
return false;
}

var parent = path.parent.value;

switch (parent.type) {
case "VariableDeclarator":
return path.name === "init";

case "MemberExpression":
return path.name === "object" || (
parent.computed && path.name === "property"
);

case "FunctionExpression":
case "FunctionDeclaration":
case "ArrowFunctionExpression":
if (path.name === "id") {
return false;
}

if (parent.params === path.parentPath &&
parent.params[path.name] === node) {
return false;
}

return true;

case "ClassDeclaration":
case "ClassExpression":
return path.name !== "id";

case "CatchClause":
return path.name !== "param";

case "Property":
case "MethodDefinition":
return path.name !== "key";

case "ImportSpecifier":
case "ImportDefaultSpecifier":
case "ImportNamespaceSpecifier":
case "LabeledStatement":
return false;

default:
return true;
}
};
33 changes: 33 additions & 0 deletions test/tests.es6.js
Original file line number Diff line number Diff line change
Expand Up @@ -1147,6 +1147,39 @@ describe("catch parameter shadowing", function() {

check(gen(), ["e1", "e2", "e1"]);
});

it("should not interfere with non-referential identifiers", function() {
function *gen() {
try {
yield 1;
raise(new Error("oyez"));
yield 2;
} catch (e) {
yield 3;
e.e = "e.e";
e[e.message] = "e.oyez";
return {
e: e,
identity: function(x) {
var e = x;
return e;
}
};
}
yield 4;
}

var g = gen();
assert.deepEqual(g.next(), { value: 1, done: false });
assert.deepEqual(g.next(), { value: 3, done: false });

var info = g.next();
assert.strictEqual(info.done, true);
assert.strictEqual(info.value.e.message, "oyez");
assert.strictEqual(info.value.e.e, "e.e");
assert.strictEqual(info.value.e.oyez, "e.oyez");
assert.strictEqual(info.value.identity("same"), "same");
});
});

describe("empty while loops", function() {
Expand Down

0 comments on commit 52a9dc8

Please # to comment.