fix(52444): "... is a type and must be imported using a type-only import ..." should have a quick fix (#52565)

Author: a-tarasyuk

src/services/codefixes/convertToTypeOnlyImport.ts

@@ -1,67 +1,81 @@
 import {
-    CodeFixContextBase,
     Diagnostics,
     factory,
+    getSynthesizedDeepClone,
+    getSynthesizedDeepClones,
     getTokenAtPosition,
+    ImportClause,
     ImportDeclaration,
+    ImportSpecifier,
     isImportDeclaration,
+    isImportSpecifier,
     SourceFile,
     textChanges,
-    TextSpan,
-    tryCast,
 } from "../_namespaces/ts";
 import {
     codeFixAll,
     createCodeFixAction,
     registerCodeFix,
 } from "../_namespaces/ts.codefix";
 
-const errorCodes = [Diagnostics.This_import_is_never_used_as_a_value_and_must_use_import_type_because_importsNotUsedAsValues_is_set_to_error.code];
+const errorCodes = [
+    Diagnostics.This_import_is_never_used_as_a_value_and_must_use_import_type_because_importsNotUsedAsValues_is_set_to_error.code,
+    Diagnostics._0_is_a_type_and_must_be_imported_using_a_type_only_import_when_verbatimModuleSyntax_is_enabled.code,
+];
 const fixId = "convertToTypeOnlyImport";
+
 registerCodeFix({
     errorCodes,
     getCodeActions: function getCodeActionsToConvertToTypeOnlyImport(context) {
-        const changes = textChanges.ChangeTracker.with(context, t => {
-            const importDeclaration = getImportDeclarationForDiagnosticSpan(context.span, context.sourceFile);
-            fixSingleImportDeclaration(t, importDeclaration, context);
-        });
-        if (changes.length) {
+        const declaration = getDeclaration(context.sourceFile, context.span.start);
+        if (declaration) {
+            const changes = textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, declaration));
             return [createCodeFixAction(fixId, changes, Diagnostics.Convert_to_type_only_import, fixId, Diagnostics.Convert_all_imports_not_used_as_a_value_to_type_only_imports)];
         }
+        return undefined;
     },
     fixIds: [fixId],
     getAllCodeActions: function getAllCodeActionsToConvertToTypeOnlyImport(context) {
         return codeFixAll(context, errorCodes, (changes, diag) => {
-            const importDeclaration = getImportDeclarationForDiagnosticSpan(diag, context.sourceFile);
-            fixSingleImportDeclaration(changes, importDeclaration, context);
+            const declaration = getDeclaration(diag.file, diag.start);
+            if (declaration) {
+                doChange(changes, diag.file, declaration);
+            }
         });
     }
 });
 
-function getImportDeclarationForDiagnosticSpan(span: TextSpan, sourceFile: SourceFile) {
-    return tryCast(getTokenAtPosition(sourceFile, span.start).parent, isImportDeclaration);
+function getDeclaration(sourceFile: SourceFile, pos: number) {
+    const { parent } = getTokenAtPosition(sourceFile, pos);
+    return isImportSpecifier(parent) || isImportDeclaration(parent) && parent.importClause ? parent : undefined;
 }
 
-function fixSingleImportDeclaration(changes: textChanges.ChangeTracker, importDeclaration: ImportDeclaration | undefined, context: CodeFixContextBase) {
-    if (!importDeclaration?.importClause) {
-        return;
+function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, declaration: ImportDeclaration | ImportSpecifier) {
+    if (isImportSpecifier(declaration)) {
+        changes.replaceNode(sourceFile, declaration, factory.updateImportSpecifier(declaration, /*isTypeOnly*/ true, declaration.propertyName, declaration.name));
     }
-
-    const { importClause } = importDeclaration;
-    // `changes.insertModifierBefore` produces a range that might overlap further changes
-    changes.insertText(context.sourceFile, importDeclaration.getStart() + "import".length, " type");
-
-    // `import type foo, { Bar }` is not allowed, so move `foo` to new declaration
-    if (importClause.name && importClause.namedBindings) {
-        changes.deleteNodeRangeExcludingEnd(context.sourceFile, importClause.name, importDeclaration.importClause.namedBindings);
-        changes.insertNodeBefore(context.sourceFile, importDeclaration, factory.updateImportDeclaration(
-            importDeclaration,
-            /*modifiers*/ undefined,
-            factory.createImportClause(
-                /*isTypeOnly*/ true,
-                importClause.name,
-                /*namedBindings*/ undefined),
-            importDeclaration.moduleSpecifier,
-            /*assertClause*/ undefined));
+    else {
+        const importClause = declaration.importClause as ImportClause;
+        if (importClause.name && importClause.namedBindings) {
+            changes.replaceNodeWithNodes(sourceFile, declaration, [
+                factory.createImportDeclaration(
+                    getSynthesizedDeepClones(declaration.modifiers, /*includeTrivia*/ true),
+                    factory.createImportClause(/*isTypeOnly*/ true, getSynthesizedDeepClone(importClause.name, /*includeTrivia*/ true), /*namedBindings*/ undefined),
+                    getSynthesizedDeepClone(declaration.moduleSpecifier, /*includeTrivia*/ true),
+                    getSynthesizedDeepClone(declaration.assertClause, /*includeTrivia*/ true),
+                ),
+                factory.createImportDeclaration(
+                    getSynthesizedDeepClones(declaration.modifiers, /*includeTrivia*/ true),
+                    factory.createImportClause(/*isTypeOnly*/ true, /*name*/ undefined, getSynthesizedDeepClone(importClause.namedBindings, /*includeTrivia*/ true)),
+                    getSynthesizedDeepClone(declaration.moduleSpecifier, /*includeTrivia*/ true),
+                    getSynthesizedDeepClone(declaration.assertClause, /*includeTrivia*/ true),
+                ),
+            ]);
+        }
+        else {
+            const importDeclaration = factory.updateImportDeclaration(declaration, declaration.modifiers,
+                factory.updateImportClause(importClause, /*isTypeOnly*/ true, importClause.name, importClause.namedBindings), declaration.moduleSpecifier, declaration.assertClause);
+            changes.replaceNode(sourceFile, declaration, importDeclaration);
+        }
     }
 }

tests/baselines/reference/tsserver/plugins/getSupportedCodeFixes-can-be-proxied.js

@@ -309,6 +309,7 @@ Info 32   [00:01:13.000] response:
         "2713",
         "1205",
         "1371",
+        "1484",
         "80009",
         "2690",
         "2420",
@@ -721,7 +722,6 @@ Info 32   [00:01:13.000] response:
         "1477",
         "1478",
         "1479",
-        "1484",
         "1485",
         "1486",
         "2200",

tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts

@@ -9,8 +9,8 @@
 
 // @Filename: imports.ts
 ////import {
-////  B,
-////  C,
+////    B,
+////    C,
 ////} from './exports';
 ////
 ////declare const b: B;
@@ -19,11 +19,11 @@
 
 goTo.file("imports.ts");
 verify.codeFix({
-  index: 0,
-  description: ts.Diagnostics.Convert_to_type_only_import.message,
-  newFileContent: `import type {
-  B,
-  C,
+    index: 0,
+    description: ts.Diagnostics.Convert_to_type_only_import.message,
+    newFileContent: `import type {
+    B,
+    C,
 } from './exports';
 
 declare const b: B;

tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts

@@ -17,9 +17,9 @@
 
 goTo.file("imports.ts");
 verify.codeFix({
-  index: 0,
-  description: ts.Diagnostics.Convert_to_type_only_import.message,
-  newFileContent: `import type A from './exports';
+    index: 0,
+    description: ts.Diagnostics.Convert_to_type_only_import.message,
+    newFileContent: `import type A from './exports';
 import type { B, C } from './exports';
 
 declare const a: A;

tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts

@@ -25,9 +25,9 @@
 
 goTo.file("imports.ts");
 verify.codeFixAll({
-  fixId: "convertToTypeOnlyImport",
-  fixAllDescription: ts.Diagnostics.Convert_all_imports_not_used_as_a_value_to_type_only_imports.message,
-  newFileContent: `import type A from './exports1';
+    fixId: "convertToTypeOnlyImport",
+    fixAllDescription: ts.Diagnostics.Convert_all_imports_not_used_as_a_value_to_type_only_imports.message,
+    newFileContent: `import type A from './exports1';
 import type { B, C } from './exports1';
 import type D from "./exports2";
 import type * as others from "./exports2";

tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts

@@ -0,0 +1,17 @@
+/// <reference path="fourslash.ts" />
+
+// @module: esnext
+// @verbatimModuleSyntax: true
+// @filename: /b.ts
+////export interface I {}
+////export const foo = {};
+
+// @filename: /a.ts
+////import { I, foo } from "./b";
+
+goTo.file("/a.ts");
+verify.codeFix({
+    index: 0,
+    description: ts.Diagnostics.Convert_to_type_only_import.message,
+    newFileContent: `import { type I, foo } from "./b";`
+});