Skip to content

Commit

Permalink
Fix parameter expression get binding (#10912)
Browse files Browse the repository at this point in the history
* fix: parameter expression closure should not have access to the declaration inside function body

* fix: renameVisitor should skip when a pattern is a scope

* address review comments
  • Loading branch information
JLHwung committed Dec 24, 2019
1 parent ee5b79d commit fc5365f
Show file tree
Hide file tree
Showing 44 changed files with 289 additions and 2 deletions.
3 changes: 2 additions & 1 deletion packages/babel-traverse/src/path/lib/virtual-types.js
Expand Up @@ -64,7 +64,8 @@ export const Expression = {
};

export const Scope = {
types: ["Scopable"],
// When pattern is inside the function params, it is a scope
types: ["Scopable", "Pattern"],
checkPath(path) {
return t.isScope(path.node, path.parent);
},
Expand Down
19 changes: 18 additions & 1 deletion packages/babel-traverse/src/scope/index.js
Expand Up @@ -897,10 +897,27 @@ export default class Scope {

getBinding(name: string) {
let scope = this;
let previousPath;

do {
const binding = scope.getOwnBinding(name);
if (binding) return binding;
if (binding) {
// Check if a pattern is a part of parameter expressions.
// 9.2.10.28: The closure created by this expression should not have visibility of
// declarations in the function body. If the binding is not a `param`-kind,
// then it must be defined inside the function body, thus it should be skipped
if (
previousPath &&
previousPath.isPattern() &&
previousPath.parentPath.isFunction() &&
binding.kind !== "param"
) {
// do nothing
} else {
return binding;
}
}
previousPath = scope.path;
} while ((scope = scope.parent));
}

Expand Down
@@ -0,0 +1,7 @@
let a = "outside";

function f(g = () => a) {
let a = "inside";
return g();
}

@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,6 @@
let a = "outside";

function f(g = () => a) {
let z = "inside";
return g();
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,5 @@
let a = "outside";

function r({ a: b }, { [a]: { c } = a }) {
g(a);
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,11 @@
let z = "outside";

function r({
a: b
}, {
[z]: {
c
} = z
}) {
g(z);
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,5 @@
let a = "outside";

function h(a, g = () => a) {
return g();
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,5 @@
let a = "outside";

function h(z, g = () => z) {
return g();
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,6 @@
let a = "outside";

function j(g = a) {
let a = "inside";
return g;
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,6 @@
let a = "outside";

function j(g = a) {
let z = "inside";
return g;
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,8 @@
let a = "outside";

function k([{
g = a
}]) {
let a = "inside";
return g;
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,8 @@
let a = "outside";

function k([{
g = a
}]) {
let z = "inside";
return g;
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,8 @@
let a = "outside";

function f([{
[a]: g
}]) {
let a = "inside";
return g;
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,8 @@
let a = "outside";

function f([{
[a]: g
}]) {
let z = "inside";
return g;
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,5 @@
let a = "outside";

function n(g = (a = a) => {}) {
let a = "inside";
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,5 @@
let a = "outside";

function n(g = (a = a) => {}) {
let z = "inside";
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,5 @@
let a = "outside";

function n(a, g = (a = a) => {}) {
a = "inside";
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,5 @@
let a = "outside";

function n(z, g = (a = a) => {}) {
z = "inside";
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,5 @@
let a = "outside";

function q(a, g = (b = a) => b) {
g(a);
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,5 @@
let a = "outside";

function q(z, g = (b = z) => b) {
g(z);
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
@@ -0,0 +1,5 @@
let a = "outside";

function r(a, g = (a, b = a) => a) {
g(a);
}
@@ -0,0 +1,3 @@
{
"plugins": ["./plugin"]
}
@@ -0,0 +1,5 @@
let a = "outside";

function r(z, g = (a, b = a) => a) {
g(z);
}
@@ -0,0 +1,9 @@
module.exports = function() {
return {
visitor: {
FunctionDeclaration(path) {
path.scope.rename("a", "z");
}
}
};
};
19 changes: 19 additions & 0 deletions packages/babel-traverse/test/scope.js
Expand Up @@ -73,6 +73,25 @@ describe("scope", () => {
).toBe("Identifier");
});

describe("function parameter expression", function() {
it("should not have visibility of declarations inside function body", () => {
expect(
getPath(
`var a = "outside"; (function foo(b = a) { let a = "inside" })`,
)
.get("body.1.expression.params.0")
.scope.getBinding("a").path.node.init.value,
).toBe("outside");
});
it("should have visibility on parameter bindings", () => {
expect(
getPath(`var a = "outside"; (function foo(b = a, a = "inside") {})`)
.get("body.1.expression.params.0")
.scope.getBinding("a").path.node.right.value,
).toBe("inside");
});
});

it("variable declaration", function() {
expect(getPath("var foo = null;").scope.getBinding("foo").path.type).toBe(
"VariableDeclarator",
Expand Down
7 changes: 7 additions & 0 deletions packages/babel-types/src/validators/isScope.js
Expand Up @@ -4,6 +4,7 @@ import {
isCatchClause,
isBlockStatement,
isScopable,
isPattern,
} from "./generated";

/**
Expand All @@ -18,5 +19,11 @@ export default function isScope(node: Object, parent: Object): boolean {
return false;
}

// If a Pattern is an immediate descendent of a Function, it must be in the params.
// Hence we skipped the parentKey === "params" check
if (isPattern(node) && isFunction(parent)) {
return true;
}

return isScopable(node);
}

0 comments on commit fc5365f

Please sign in to comment.