Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix parameter expression get binding #10912

Merged
merged 3 commits into from Dec 24, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a Pattern is a child of Function, it must be one of the params key because both body and typeAnnotation can not be a Pattern.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you need to add a comment to explain your PR, then why don't add it directly in the code? 😉

return true;
}

return isScopable(node);
}