Elide import = statements that are only used as a type (#441)

Fixes #422

In both CJS and ESM, we now look at the identifier in `import A =` and decide
whether to elide the entire statement in a similar way to normal TS import
elision. Since there are only two forms that these statements can have, I just
special cased the two when deleting statements.

Technically to implement this totally correctly, I'd need something like a graph
traversal: in an `import A = B.C;` statement, `B` is referenced as a value if
`A` is ever referenced as a value, so deciding if an import should be elided
requires following an arbitrary number of import statements. In practice, I
think this syntax is only ever used to pull in types (for values, you can just
use `const`, and the syntax is obscure anyway), so I treat it as a type
statement that gets elided.

src/CJSImportProcessor.ts

@@ -30,6 +30,7 @@ interface ImportInfo {
  * interopRequireWildcard, so we also allow that mode for compatibility.
  */
 export default class CJSImportProcessor {
+  private nonTypeIdentifiers: Set<string> = new Set();
   private importInfoByPath: Map<string, ImportInfo> = new Map();
   private importsToReplace: Map<string, string> = new Map();
   private identifierReplacements: Map<string, string> = new Map();
@@ -41,6 +42,7 @@ export default class CJSImportProcessor {
     readonly tokens: TokenProcessor,
     readonly enableLegacyTypeScriptModuleInterop: boolean,
     readonly options: Options,
+    readonly isTypeScriptTransformEnabled: boolean,
   ) {
     this.helpers = new HelperManager(nameManager);
   }
@@ -72,7 +74,7 @@ export default class CJSImportProcessor {
    * bare imports.
    */
   pruneTypeOnlyImports(): void {
-    const nonTypeIdentifiers = getNonTypeIdentifiers(this.tokens, this.options);
+    this.nonTypeIdentifiers = getNonTypeIdentifiers(this.tokens, this.options);
     for (const [path, importInfo] of this.importInfoByPath.entries()) {
       if (
         importInfo.hasBareImport ||
@@ -87,12 +89,16 @@ export default class CJSImportProcessor {
         ...importInfo.wildcardNames,
         ...importInfo.namedImports.map(({localName}) => localName),
       ];
-      if (names.every((name) => !nonTypeIdentifiers.has(name))) {
+      if (names.every((name) => this.isTypeName(name))) {
         this.importsToReplace.set(path, "");
       }
     }
   }
 
+  isTypeName(name: string): boolean {
+    return this.isTypeScriptTransformEnabled && !this.nonTypeIdentifiers.has(name);
+  }
+
   private generateImportReplacements(): void {
     for (const [path, importInfo] of this.importInfoByPath.entries()) {
       const {

src/index.ts

@@ -136,6 +136,7 @@ function getSucraseContext(code: string, options: Options): SucraseContext {
       tokenProcessor,
       enableLegacyTypeScriptModuleInterop,
       options,
+      options.transforms.includes("typescript"),
     );
     importProcessor.preprocessTokens();
     // We need to mark shadowed globals after processing imports so we know that the globals are,

src/transformers/CJSImportTransformer.ts

@@ -4,6 +4,7 @@ import {IdentifierRole, isDeclaration, isObjectShorthandDeclaration} from "../pa
 import {ContextualKeyword} from "../parser/tokenizer/keywords";
 import {TokenType as tt} from "../parser/tokenizer/types";
 import TokenProcessor from "../TokenProcessor";
+import elideImportEquals from "../util/elideImportEquals";
 import getDeclarationInfo, {
   DeclarationInfo,
   EMPTY_DECLARATION_INFO,
@@ -55,8 +56,7 @@ export default class CJSImportTransformer extends Transformer {
   process(): boolean {
     // TypeScript `import foo = require('foo');` should always just be translated to plain require.
     if (this.tokens.matches3(tt._import, tt.name, tt.eq)) {
-      this.tokens.replaceToken("const");
-      return true;
+      return this.processImportEquals();
     }
     if (this.tokens.matches1(tt._import)) {
       this.processImport();
@@ -91,6 +91,18 @@ export default class CJSImportTransformer extends Transformer {
     return false;
   }
 
+  private processImportEquals(): boolean {
+    const importName = this.tokens.identifierNameAtIndex(this.tokens.currentIndex() + 1);
+    if (this.importProcessor.isTypeName(importName)) {
+      // If this name is only used as a type, elide the whole import.
+      elideImportEquals(this.tokens);
+    } else {
+      // Otherwise, switch `import` to `const`.
+      this.tokens.replaceToken("const");
+    }
+    return true;
+  }
+
   /**
    * Transform this:
    * import foo, {bar} from 'baz';

src/transformers/ESMImportTransformer.ts

@@ -3,6 +3,7 @@ import NameManager from "../NameManager";
 import {ContextualKeyword} from "../parser/tokenizer/keywords";
 import {TokenType as tt} from "../parser/tokenizer/types";
 import TokenProcessor from "../TokenProcessor";
+import elideImportEquals from "../util/elideImportEquals";
 import getDeclarationInfo, {
   DeclarationInfo,
   EMPTY_DECLARATION_INFO,
@@ -39,8 +40,7 @@ export default class ESMImportTransformer extends Transformer {
   process(): boolean {
     // TypeScript `import foo = require('foo');` should always just be translated to plain require.
     if (this.tokens.matches3(tt._import, tt.name, tt.eq)) {
-      this.tokens.replaceToken("const");
-      return true;
+      return this.processImportEquals();
     }
     if (this.tokens.matches2(tt._export, tt.eq)) {
       this.tokens.replaceToken("module.exports");
@@ -58,6 +58,18 @@ export default class ESMImportTransformer extends Transformer {
     return false;
   }
 
+  private processImportEquals(): boolean {
+    const importName = this.tokens.identifierNameAtIndex(this.tokens.currentIndex() + 1);
+    if (this.isTypeName(importName)) {
+      // If this name is only used as a type, elide the whole import.
+      elideImportEquals(this.tokens);
+    } else {
+      // Otherwise, switch `import` to `const`.
+      this.tokens.replaceToken("const");
+    }
+    return true;
+  }
+
   private processImport(): boolean {
     if (this.tokens.matches2(tt._import, tt.parenL)) {
       // Dynamic imports don't need to be transformed.

src/util/elideImportEquals.ts

@@ -0,0 +1,29 @@
+import {TokenType as tt} from "../parser/tokenizer/types";
+import TokenProcessor from "../TokenProcessor";
+
+export default function elideImportEquals(tokens: TokenProcessor): void {
+  // import
+  tokens.removeInitialToken();
+  // name
+  tokens.removeToken();
+  // =
+  tokens.removeToken();
+  // name or require
+  tokens.removeToken();
+  // Handle either `import A = require('A')` or `import A = B.C.D`.
+  if (tokens.matches1(tt.parenL)) {
+    // (
+    tokens.removeToken();
+    // path string
+    tokens.removeToken();
+    // )
+    tokens.removeToken();
+  } else {
+    while (tokens.matches1(tt.dot)) {
+      // .
+      tokens.removeToken();
+      // name
+      tokens.removeToken();
+    }
+  }
+}

test/identifyShadowedGlobals-test.ts

@@ -10,9 +10,15 @@ function assertHasShadowedGlobals(code: string, expected: boolean): void {
   const tokenProcessor = new TokenProcessor(code, file.tokens, false);
   const nameManager = new NameManager(tokenProcessor);
   nameManager.preprocessNames();
-  const importProcessor = new CJSImportProcessor(nameManager, tokenProcessor, false, {
-    transforms: [],
-  });
+  const importProcessor = new CJSImportProcessor(
+    nameManager,
+    tokenProcessor,
+    false,
+    {
+      transforms: [],
+    },
+    false,
+  );
   importProcessor.preprocessTokens();
   assert.strictEqual(
     hasShadowedGlobals(tokenProcessor, importProcessor.getGlobalNames()),

test/typescript-test.ts

@@ -1654,6 +1654,58 @@ describe("typescript transform", () => {
     );
   });
 
+  it("elides unused import = statements", () => {
+    assertTypeScriptImportResult(
+      `
+      import A = require('A');
+      import B = require('B');
+      import C = A.C;
+      B();
+      const c: C = 2;
+    `,
+      {
+        expectedCJSResult: `"use strict";
+      ;
+      const B = require('B');
+      ;
+      B();
+      const c = 2;
+    `,
+        expectedESMResult: `
+      ;
+      const B = require('B');
+      ;
+      B();
+      const c = 2;
+    `,
+      },
+    );
+  });
+
+  // We don't support this for now since it needs a more complex implementation than we have
+  // anywhere else, and ideally you would just write `const B = A.B;`, which works.
+  it.skip("handles transitive references when eliding import = statements", () => {
+    assertTypeScriptImportResult(
+      `
+      import A from 'A';
+      import B = A.B;
+      B();
+    `,
+      {
+        expectedCJSResult: `"use strict";${ESMODULE_PREFIX}
+      const A_1 = require("A");
+      const B = A_1.default.B;
+      B();
+    `,
+        expectedESMResult: `
+      import A from 'A';
+      const B = A.B;
+      B();
+    `,
+      },
+    );
+  });
+
   it("handles newlines before class declarations", () => {
     assertTypeScriptResult(
       `