Fix shadowed import detection with simple arrow function params (#366)

We had code to mark parameters as function-scoped declaration identfiers, but this wasn't working for arrow functions and async arrow functions that omit the parens around their only parameter, so shadowed imports weren't being respected properly. In addition to fixing, I consolidated the identifier role assignment into one place, which will be useful for a future change.
alangpierce · Dec 24, 2018 · e476734 · e476734
1 parent 76f5b8d
commit e476734
Show file tree

Hide file tree

Showing 4 changed files with 58 additions and 11 deletions.
diff --git a/src/parser/traverser/expression.ts b/src/parser/traverser/expression.ts
@@ -57,7 +57,13 @@ import {ContextualKeyword} from "../tokenizer/keywords";
 import {Scope} from "../tokenizer/state";
 import {TokenType, TokenType as tt} from "../tokenizer/types";
 import {getNextContextId, isFlowEnabled, isJSXEnabled, isTypeScriptEnabled, state} from "./base";
-import {parseMaybeDefault, parseRest, parseSpread} from "./lval";
+import {
+  markPriorBindingIdentifier,
+  parseBindingIdentifier,
+  parseMaybeDefault,
+  parseRest,
+  parseSpread,
+} from "./lval";
 import {
   parseBlock,
   parseClass,
@@ -446,14 +452,16 @@ export function parseExprAtom(): boolean {
         contextualKeyword === ContextualKeyword._async &&
         match(tt.name)
       ) {
-        parseIdentifier();
+        parseBindingIdentifier(false);
         expect(tt.arrow);
-        // let foo = bar => {};
+        // let foo = async bar => {};
         parseArrowExpression(functionStart, startTokenIndex);
         return true;
       }
 
-      if (canBeArrow && !canInsertSemicolon() && eat(tt.arrow)) {
+      if (canBeArrow && !canInsertSemicolon() && match(tt.arrow)) {
+        markPriorBindingIdentifier(false);
+        expect(tt.arrow);
         parseArrowExpression(functionStart, startTokenIndex);
         return true;
       }

diff --git a/src/parser/traverser/lval.ts b/src/parser/traverser/lval.ts
@@ -28,8 +28,15 @@ export function parseRest(isBlockScope: boolean): void {
   parseBindingAtom(isBlockScope);
 }
 
-export function parseBindingIdentifier(): void {
+export function parseBindingIdentifier(isBlockScope: boolean): void {
   parseIdentifier();
+  markPriorBindingIdentifier(isBlockScope);
+}
+
+export function markPriorBindingIdentifier(isBlockScope: boolean): void {
+  state.tokens[state.tokens.length - 1].identifierRole = isBlockScope
+    ? IdentifierRole.BlockScopedDeclaration
+    : IdentifierRole.FunctionScopedDeclaration;
 }
 
 // Parses lvalue (assignable) atom.
@@ -46,10 +53,7 @@ export function parseBindingAtom(isBlockScope: boolean): void {
     case tt._yield:
     case tt.name: {
       state.type = tt.name;
-      parseBindingIdentifier();
-      state.tokens[state.tokens.length - 1].identifierRole = isBlockScope
-        ? IdentifierRole.BlockScopedDeclaration
-        : IdentifierRole.FunctionScopedDeclaration;
+      parseBindingIdentifier(isBlockScope);
       return;
     }
 

diff --git a/src/parser/traverser/statement.ts b/src/parser/traverser/statement.ts
@@ -568,8 +568,7 @@ export function parseFunction(
     if (!isStatement) {
       nameScopeStartTokenIndex = state.tokens.length;
     }
-    parseBindingIdentifier();
-    state.tokens[state.tokens.length - 1].identifierRole = IdentifierRole.FunctionScopedDeclaration;
+    parseBindingIdentifier(false);
   }
 
   const startTokenIndex = state.tokens.length;

diff --git a/test/imports-test.ts b/test/imports-test.ts
@@ -1125,4 +1125,40 @@ module.exports = exports.default;
       {transforms: ["imports", "typescript"]},
     );
   });
+
+  it("properly handles shadowing for simple arrow functions", () => {
+    assertResult(
+      `
+      import a from 'a';
+      const f = a => {
+        a();
+      };
+    `,
+      `"use strict";${IMPORT_DEFAULT_PREFIX}
+      
+      const f = a => {
+        a();
+      };
+    `,
+      {transforms: ["imports", "typescript"]},
+    );
+  });
+
+  it("properly handles shadowing for simple async arrow functions", () => {
+    assertResult(
+      `
+      import a from 'a';
+      const f = async a => {
+        a();
+      };
+    `,
+      `"use strict";${IMPORT_DEFAULT_PREFIX}
+      
+      const f = async a => {
+        a();
+      };
+    `,
+      {transforms: ["imports", "typescript"]},
+    );
+  });
 });