Skip to content

Commit

Permalink
Fix scope of function declaration binding in constant folding (#643)
Browse files Browse the repository at this point in the history
Summary:
**Summary**

This is a possibly incomplete fix for #291, it corrects at least one issue with name shadowing but there might be other problems present in the very large code payload presented in the original issue. The exact bug fixed here happens when a function declares an unused variable shadowing its own name:

```js
export function foo() {
    let foo;
}
```

The current code checks whether that the binding corresponding to the function is referenced in the scope of the FunctionDeclaration node, however in Babel that scope is situated inside the body of the function and not in the outer program. Since the binding is retrieved by name, Babel returns informations related to the inner, unreferenced variable and thus the entire function is removed. This fix changes the biding retrieval to read from the parent scope of the FunctionDefinition, which returns the correct binding.

**Test plan**

An additional test case has been added to verify the function remains in the final output.

Pull Request resolved: #643

Reviewed By: MichaReiser

Differential Revision: D27324443

Pulled By: motiz88

fbshipit-source-id: 30a9b7244846ad0ff4ba080c96e81dc6f582e5e5
  • Loading branch information
leops authored and facebook-github-bot committed Mar 25, 2021
1 parent 601f6cd commit 150d13e
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -346,4 +346,20 @@ describe('constant expressions', () => {

compare([constantFoldingPlugin], nonChanged, nonChanged);
});

it('does not confuse function identifiers with variables in inner scope', () => {
const code = `
export function foo() {
let foo;
}
`;

const expected = `
export function foo() {
let foo;
}
`;

compare([constantFoldingPlugin], code, expected);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function constantFoldingPlugin(context: {types: Types, ...}): Visitors {
const FunctionDeclaration = {
exit(path, state): void {
const binding =
path.node.id != null && path.scope.getBinding(path.node.id.name);
path.node.id != null && path.scope.parent.getBinding(path.node.id.name);

if (binding && !binding.referenced) {
state.stripped = true;
Expand Down

0 comments on commit 150d13e

Please sign in to comment.