From 150d13eb9abdad45606a3ada0613947e751d9542 Mon Sep 17 00:00:00 2001 From: leops Date: Thu, 25 Mar 2021 04:11:10 -0700 Subject: [PATCH] Fix scope of function declaration binding in constant folding (#643) Summary: **Summary** This is a possibly incomplete fix for https://github.com/facebook/metro/issues/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: https://github.com/facebook/metro/pull/643 Reviewed By: MichaReiser Differential Revision: D27324443 Pulled By: motiz88 fbshipit-source-id: 30a9b7244846ad0ff4ba080c96e81dc6f582e5e5 --- .../__tests__/constant-folding-plugin-test.js | 16 ++++++++++++++++ .../src/constant-folding-plugin.js | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js b/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js index b38e9785d..26a235f9a 100644 --- a/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js +++ b/packages/metro-transform-plugins/src/__tests__/constant-folding-plugin-test.js @@ -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); + }); }); diff --git a/packages/metro-transform-plugins/src/constant-folding-plugin.js b/packages/metro-transform-plugins/src/constant-folding-plugin.js index 404829ded..510b1ba34 100644 --- a/packages/metro-transform-plugins/src/constant-folding-plugin.js +++ b/packages/metro-transform-plugins/src/constant-folding-plugin.js @@ -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;