Proposal: Always allow type-only imports to reference .ts extensions (#54746)

andrewbranch · web-flow · commit 2170e6c6cc12 · 2023-07-24T16:12:38.000-07:00
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -4908,8 +4908,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {
                 }
             }
             else if (resolvedModule.resolvedUsingTsExtension && !shouldAllowImportingTsExtension(compilerOptions, currentSourceFile.fileName)) {
-                const tsExtension = Debug.checkDefined(tryExtractTSExtension(moduleReference));
-                error(errorNode, Diagnostics.An_import_path_can_only_end_with_a_0_extension_when_allowImportingTsExtensions_is_enabled, tsExtension);
+                const importOrExport =
+                    findAncestor(location, isImportDeclaration)?.importClause ||
+                    findAncestor(location, or(isImportEqualsDeclaration, isExportDeclaration));
+                if (!(importOrExport?.isTypeOnly || findAncestor(location, isImportTypeNode))) {
+                    const tsExtension = Debug.checkDefined(tryExtractTSExtension(moduleReference));
+                    error(errorNode, Diagnostics.An_import_path_can_only_end_with_a_0_extension_when_allowImportingTsExtensions_is_enabled, tsExtension);
+                }
             }
 
             if (sourceFile.symbol) {
diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts
@@ -5,6 +5,7 @@ import {
     arrayFrom,
     CancellationToken,
     cast,
+    changeAnyExtension,
     CodeAction,
     CodeFixAction,
     CodeFixContextBase,
@@ -42,10 +43,13 @@ import {
     getExportInfoMap,
     getMeaningFromDeclaration,
     getMeaningFromLocation,
+    getModeForUsageLocation,
     getNameForExportedSymbol,
     getNodeId,
+    getOutputExtension,
     getQuoteFromPreference,
     getQuotePreference,
+    getResolvedModule,
     getSourceFileOfNode,
     getSymbolId,
     getTokenAtPosition,
@@ -1356,6 +1360,15 @@ function promoteFromTypeOnly(changes: textChanges.ChangeTracker, aliasDeclaratio
 
     function promoteImportClause(importClause: ImportClause) {
         changes.delete(sourceFile, getTypeKeywordOfTypeOnlyImport(importClause, sourceFile));
+        // Change .ts extension to .js if necessary
+        if (!compilerOptions.allowImportingTsExtensions) {
+            const moduleSpecifier = tryGetModuleSpecifierFromDeclaration(importClause.parent);
+            const resolvedModule = moduleSpecifier && getResolvedModule(sourceFile, moduleSpecifier.text, getModeForUsageLocation(sourceFile, moduleSpecifier));
+            if (resolvedModule?.resolvedUsingTsExtension) {
+                const changedExtension = changeAnyExtension(moduleSpecifier!.text, getOutputExtension(moduleSpecifier!.text, compilerOptions));
+                changes.replaceNode(sourceFile, moduleSpecifier!, factory.createStringLiteral(changedExtension));
+            }
+        }
         if (convertExistingToTypeOnly) {
             const namedImports = tryCast(importClause.namedBindings, isNamedImports);
             if (namedImports && namedImports.elements.length > 1) {
diff --git a/tests/baselines/reference/allowsImportingTsExtension.errors.txt b/tests/baselines/reference/allowsImportingTsExtension.errors.txt
@@ -0,0 +1,40 @@
+b.ts(2,16): error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.
+b.ts(3,30): error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.
+b.ts(5,25): error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.
+c.ts(2,16): error TS2846: A declaration file cannot be imported without 'import type'. Did you mean to import an implementation file './a.js' instead?
+c.ts(3,30): error TS2846: A declaration file cannot be imported without 'import type'. Did you mean to import an implementation file './a.js' instead?
+c.ts(5,25): error TS2846: A declaration file cannot be imported without 'import type'. Did you mean to import an implementation file './a.js' instead?
+
+
+==== a.ts (0 errors) ====
+    export class A {}
+    
+==== a.d.ts (0 errors) ====
+    export class A {}
+    
+==== b.ts (3 errors) ====
+    import type { A } from "./a.ts"; // ok
+    import {} from "./a.ts"; // error
+                   ~~~~~~~~
+!!! error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.
+    import { type A as _A } from "./a.ts"; // error
+                                 ~~~~~~~~
+!!! error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.
+    type __A = import("./a.ts").A; // ok
+    const aPromise = import("./a.ts"); // error
+                            ~~~~~~~~
+!!! error TS5097: An import path can only end with a '.ts' extension when 'allowImportingTsExtensions' is enabled.
+    
+==== c.ts (3 errors) ====
+    import type { A } from "./a.d.ts"; // ok
+    import {} from "./a.d.ts"; // error
+                   ~~~~~~~~~~
+!!! error TS2846: A declaration file cannot be imported without 'import type'. Did you mean to import an implementation file './a.js' instead?
+    import { type A as _A } from "./a.d.ts"; // error
+                                 ~~~~~~~~~~
+!!! error TS2846: A declaration file cannot be imported without 'import type'. Did you mean to import an implementation file './a.js' instead?
+    type __A = import("./a.d.ts").A; // ok
+    const aPromise = import("./a.d.ts"); // error
+                            ~~~~~~~~~~
+!!! error TS2846: A declaration file cannot be imported without 'import type'. Did you mean to import an implementation file './a.js' instead?
+    
diff --git a/tests/baselines/reference/allowsImportingTsExtension.js b/tests/baselines/reference/allowsImportingTsExtension.js
@@ -0,0 +1,32 @@
+//// [tests/cases/conformance/externalModules/typeOnly/allowsImportingTsExtension.ts] ////
+
+//// [a.ts]
+export class A {}
+
+//// [a.d.ts]
+export class A {}
+
+//// [b.ts]
+import type { A } from "./a.ts"; // ok
+import {} from "./a.ts"; // error
+import { type A as _A } from "./a.ts"; // error
+type __A = import("./a.ts").A; // ok
+const aPromise = import("./a.ts"); // error
+
+//// [c.ts]
+import type { A } from "./a.d.ts"; // ok
+import {} from "./a.d.ts"; // error
+import { type A as _A } from "./a.d.ts"; // error
+type __A = import("./a.d.ts").A; // ok
+const aPromise = import("./a.d.ts"); // error
+
+
+//// [a.js]
+export class A {
+}
+//// [b.js]
+const aPromise = import("./a.ts"); // error
+export {};
+//// [c.js]
+const aPromise = import("./a.d.ts"); // error
+export {};
diff --git a/tests/baselines/reference/allowsImportingTsExtension.symbols b/tests/baselines/reference/allowsImportingTsExtension.symbols
@@ -0,0 +1,44 @@
+//// [tests/cases/conformance/externalModules/typeOnly/allowsImportingTsExtension.ts] ////
+
+=== a.ts ===
+export class A {}
+>A : Symbol(A, Decl(a.ts, 0, 0))
+
+=== a.d.ts ===
+export class A {}
+>A : Symbol(A, Decl(a.d.ts, 0, 0))
+
+=== b.ts ===
+import type { A } from "./a.ts"; // ok
+>A : Symbol(A, Decl(b.ts, 0, 13))
+
+import {} from "./a.ts"; // error
+import { type A as _A } from "./a.ts"; // error
+>A : Symbol(A, Decl(a.ts, 0, 0))
+>_A : Symbol(_A, Decl(b.ts, 2, 8))
+
+type __A = import("./a.ts").A; // ok
+>__A : Symbol(__A, Decl(b.ts, 2, 38))
+>A : Symbol(A, Decl(a.ts, 0, 0))
+
+const aPromise = import("./a.ts"); // error
+>aPromise : Symbol(aPromise, Decl(b.ts, 4, 5))
+>"./a.ts" : Symbol("a", Decl(a.ts, 0, 0))
+
+=== c.ts ===
+import type { A } from "./a.d.ts"; // ok
+>A : Symbol(A, Decl(c.ts, 0, 13))
+
+import {} from "./a.d.ts"; // error
+import { type A as _A } from "./a.d.ts"; // error
+>A : Symbol(A, Decl(a.ts, 0, 0))
+>_A : Symbol(_A, Decl(c.ts, 2, 8))
+
+type __A = import("./a.d.ts").A; // ok
+>__A : Symbol(__A, Decl(c.ts, 2, 40))
+>A : Symbol(A, Decl(a.ts, 0, 0))
+
+const aPromise = import("./a.d.ts"); // error
+>aPromise : Symbol(aPromise, Decl(c.ts, 4, 5))
+>"./a.d.ts" : Symbol("a", Decl(a.ts, 0, 0))
+
diff --git a/tests/baselines/reference/allowsImportingTsExtension.types b/tests/baselines/reference/allowsImportingTsExtension.types
@@ -0,0 +1,44 @@
+//// [tests/cases/conformance/externalModules/typeOnly/allowsImportingTsExtension.ts] ////
+
+=== a.ts ===
+export class A {}
+>A : A
+
+=== a.d.ts ===
+export class A {}
+>A : A
+
+=== b.ts ===
+import type { A } from "./a.ts"; // ok
+>A : A
+
+import {} from "./a.ts"; // error
+import { type A as _A } from "./a.ts"; // error
+>A : typeof A
+>_A : typeof A
+
+type __A = import("./a.ts").A; // ok
+>__A : A
+
+const aPromise = import("./a.ts"); // error
+>aPromise : Promise<typeof import("a")>
+>import("./a.ts") : Promise<typeof import("a")>
+>"./a.ts" : "./a.ts"
+
+=== c.ts ===
+import type { A } from "./a.d.ts"; // ok
+>A : A
+
+import {} from "./a.d.ts"; // error
+import { type A as _A } from "./a.d.ts"; // error
+>A : typeof A
+>_A : typeof A
+
+type __A = import("./a.d.ts").A; // ok
+>__A : A
+
+const aPromise = import("./a.d.ts"); // error
+>aPromise : Promise<typeof import("a")>
+>import("./a.d.ts") : Promise<typeof import("a")>
+>"./a.d.ts" : "./a.d.ts"
+
diff --git a/tests/cases/conformance/externalModules/typeOnly/allowsImportingTsExtension.ts b/tests/cases/conformance/externalModules/typeOnly/allowsImportingTsExtension.ts
@@ -0,0 +1,23 @@
+// @allowImportingTsExtensions: false
+// @target: esnext
+// @module: esnext
+
+// @Filename: a.ts
+export class A {}
+
+// @Filename: a.d.ts
+export class A {}
+
+// @Filename: b.ts
+import type { A } from "./a.ts"; // ok
+import {} from "./a.ts"; // error
+import { type A as _A } from "./a.ts"; // error
+type __A = import("./a.ts").A; // ok
+const aPromise = import("./a.ts"); // error
+
+// @Filename: c.ts
+import type { A } from "./a.d.ts"; // ok
+import {} from "./a.d.ts"; // error
+import { type A as _A } from "./a.d.ts"; // error
+type __A = import("./a.d.ts").A; // ok
+const aPromise = import("./a.d.ts"); // error
diff --git a/tests/cases/fourslash/completionsImport_promoteTypeOnly6.ts b/tests/cases/fourslash/completionsImport_promoteTypeOnly6.ts
@@ -0,0 +1,34 @@
+/// <reference path="fourslash.ts" />
+// @module: nodenext
+// @allowImportingTsExtensions: false
+
+// @Filename: /exports.ts
+//// export interface SomeInterface {}
+//// export class SomePig {}
+
+// @Filename: /a.ts
+//// import type { SomePig } from "./exports.ts";
+//// new SomePig/**/
+
+verify.completions({
+  marker: "",
+  includes: [{
+    name: "SomePig",
+    source: completion.CompletionSource.TypeOnlyAlias,
+    hasAction: true,
+  }]
+});
+
+verify.applyCodeActionFromCompletion("", {
+  name: "SomePig",
+  source: completion.CompletionSource.TypeOnlyAlias,
+  description: `Remove 'type' from import declaration from "./exports.ts"`,
+  newFileContent:
+`import { SomePig } from "./exports.js";
+new SomePig`,
+  preferences: {
+    includeCompletionsForModuleExports: true,
+    allowIncompleteCompletions: true,
+    includeInsertTextCompletions: true,
+  },
+});
diff --git a/tests/cases/fourslash/completionsImport_promoteTypeOnly7.ts b/tests/cases/fourslash/completionsImport_promoteTypeOnly7.ts
@@ -0,0 +1,34 @@
+/// <reference path="fourslash.ts" />
+// @module: nodenext
+// @allowImportingTsExtensions: true
+
+// @Filename: /exports.ts
+//// export interface SomeInterface {}
+//// export class SomePig {}
+
+// @Filename: /a.ts
+//// import type { SomePig } from "./exports.ts";
+//// new SomePig/**/
+
+verify.completions({
+  marker: "",
+  includes: [{
+    name: "SomePig",
+    source: completion.CompletionSource.TypeOnlyAlias,
+    hasAction: true,
+  }]
+});
+
+verify.applyCodeActionFromCompletion("", {
+  name: "SomePig",
+  source: completion.CompletionSource.TypeOnlyAlias,
+  description: `Remove 'type' from import declaration from "./exports.ts"`,
+  newFileContent:
+`import { SomePig } from "./exports.ts";
+new SomePig`,
+  preferences: {
+    includeCompletionsForModuleExports: true,
+    allowIncompleteCompletions: true,
+    includeInsertTextCompletions: true,
+  },
+});

Original file line number	Diff line number	Diff line change
`@@ -4908,8 +4908,13 @@ export function createTypeChecker(host: TypeCheckerHost): TypeChecker {`
`4908`	`4908`	`}`
`4909`	`4909`	`}`
`4910`	`4910`	`else if (resolvedModule.resolvedUsingTsExtension && !shouldAllowImportingTsExtension(compilerOptions, currentSourceFile.fileName)) {`
`4911`		`- const tsExtension = Debug.checkDefined(tryExtractTSExtension(moduleReference));`
`4912`		`- error(errorNode, Diagnostics.An_import_path_can_only_end_with_a_0_extension_when_allowImportingTsExtensions_is_enabled, tsExtension);`
	`4911`	`+ const importOrExport =`
	`4912`	`+ findAncestor(location, isImportDeclaration)?.importClause \|\|`
	`4913`	`+ findAncestor(location, or(isImportEqualsDeclaration, isExportDeclaration));`
	`4914`	`+ if (!(importOrExport?.isTypeOnly \|\| findAncestor(location, isImportTypeNode))) {`
	`4915`	`+ const tsExtension = Debug.checkDefined(tryExtractTSExtension(moduleReference));`
	`4916`	`+ error(errorNode, Diagnostics.An_import_path_can_only_end_with_a_0_extension_when_allowImportingTsExtensions_is_enabled, tsExtension);`
	`4917`	`+ }`
`4913`	`4918`	`}`
`4914`	`4919`
`4915`	`4920`	`if (sourceFile.symbol) {`