Fix shadowed global detection to ignore declarations in type tokens (#…

…809)

Fixes #808

All identifier tokens, including type tokens, have a role assigned by the parser
that indicates whether they're a declaration, usage, etc, and even for type
tokens this is important for TS automatic export elision. When identifying
shadowed globals, though, only non-type declarations should be considered. This
fixes an issue where parameters within TS function types were inadvertently
being treated as declarations.

src/identifyShadowedGlobals.ts

@@ -31,6 +31,7 @@ export function hasShadowedGlobals(tokens: TokenProcessor, globalNames: Set<stri
   for (const token of tokens.tokens) {
     if (
       token.type === tt.name &&
+      !token.isType &&
       isNonTopLevelDeclaration(token) &&
       globalNames.has(tokens.identifierNameForToken(token))
     ) {
@@ -64,7 +65,7 @@ function markShadowedGlobals(
 
     const token = tokens.tokens[i];
     const name = tokens.identifierNameForToken(token);
-    if (scopeStack.length > 1 && token.type === tt.name && globalNames.has(name)) {
+    if (scopeStack.length > 1 && !token.isType && token.type === tt.name && globalNames.has(name)) {
       if (isBlockScopedDeclaration(token)) {
         markShadowedForScope(scopeStack[scopeStack.length - 1], tokens, name);
       } else if (isFunctionScopedDeclaration(token)) {

test/identifyShadowedGlobals-test.ts

@@ -8,7 +8,7 @@ import {parse} from "../src/parser";
 import TokenProcessor from "../src/TokenProcessor";
 
 function assertHasShadowedGlobals(code: string, expected: boolean): void {
-  const file = parse(code, false, false, false);
+  const file = parse(code, false, true, false);
   const nameManager = new NameManager(code, file.tokens);
   const helperManager = new HelperManager(nameManager);
   const tokenProcessor = new TokenProcessor(code, file.tokens, false, false, helperManager);
@@ -30,7 +30,7 @@ function assertHasShadowedGlobals(code: string, expected: boolean): void {
 }
 
 describe("identifyShadowedGlobals", () => {
-  it("properly does an up-front that there are any shadowed globals", () => {
+  it("properly does an up-front check that there are any shadowed globals", () => {
     assertHasShadowedGlobals(
       `
       import a from 'a';
@@ -53,4 +53,17 @@ describe("identifyShadowedGlobals", () => {
       false,
     );
   });
+
+  it("does not treat parameters within types as real declarations", () => {
+    assertHasShadowedGlobals(
+      `
+      import a from 'a';
+      
+      function foo(f: (a: number) => void) {
+        console.log(a);
+      }
+    `,
+      false,
+    );
+  });
 });

test/typescript-test.ts

@@ -1614,6 +1614,27 @@ describe("typescript transform", () => {
     );
   });
 
+  it("does not consider parameter lists in types to shadow names", () => {
+    assertTypeScriptESMResult(
+      `
+      import { someVariable } from './someFile';
+      import { otherVariable } from './otherFile';
+      
+      function Foo(arg: (someVariable: any) => void, otherVariable) {
+        console.log(arg, someVariable, otherVariable);
+      }
+    `,
+      `
+      import { someVariable } from './someFile';
+
+      
+      function Foo(arg, otherVariable) {
+        console.log(arg, someVariable, otherVariable);
+      }
+    `,
+    );
+  });
+
   it("produces proper import statements when eliding ESM imported names", () => {
     assertTypeScriptESMResult(
       `