diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json index a9dd5ddf857f2..e7eedbe3b5fb1 100644 --- a/src/compiler/diagnosticMessages.json +++ b/src/compiler/diagnosticMessages.json @@ -1188,14 +1188,6 @@ "category": "Error", "code": 1371 }, - "Convert to type-only import": { - "category": "Message", - "code": 1373 - }, - "Convert all imports not used as a value to type-only imports": { - "category": "Message", - "code": 1374 - }, "'await' expressions are only allowed at the top level of a file when that file is a module, but this file has no imports or exports. Consider adding an empty 'export {}' to make this file a module.": { "category": "Error", "code": 1375 @@ -7616,6 +7608,18 @@ "category": "Message", "code": 95179 }, + "Use 'import type'": { + "category": "Message", + "code": 95180 + }, + "Use 'type {0}'": { + "category": "Message", + "code": 95181 + }, + "Fix all with type-only imports": { + "category": "Message", + "code": 95182 + }, "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": { "category": "Error", diff --git a/src/services/codefixes/convertToTypeOnlyImport.ts b/src/services/codefixes/convertToTypeOnlyImport.ts index 5b553bb33d0e5..6e167fb7313bb 100644 --- a/src/services/codefixes/convertToTypeOnlyImport.ts +++ b/src/services/codefixes/convertToTypeOnlyImport.ts @@ -1,6 +1,7 @@ import { Diagnostics, factory, + FindAllReferences, getSynthesizedDeepClone, getSynthesizedDeepClones, getTokenAtPosition, @@ -9,12 +10,18 @@ import { ImportSpecifier, isImportDeclaration, isImportSpecifier, + isValidTypeOnlyAliasUseSite, + Program, + sameMap, + some, SourceFile, + SyntaxKind, textChanges, } from "../_namespaces/ts"; import { codeFixAll, createCodeFixAction, + createCodeFixActionWithoutFixAll, registerCodeFix, } from "../_namespaces/ts.codefix"; @@ -30,16 +37,46 @@ registerCodeFix({ 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)]; + const importDeclarationChanges = declaration.kind === SyntaxKind.ImportSpecifier && canConvertImportDeclarationForSpecifier(declaration, context.sourceFile, context.program) + ? textChanges.ChangeTracker.with(context, t => doChange(t, context.sourceFile, declaration.parent.parent.parent)) + : undefined; + const mainAction = createCodeFixAction( + fixId, + changes, + declaration.kind === SyntaxKind.ImportSpecifier + ? [Diagnostics.Use_type_0, declaration.propertyName?.text ?? declaration.name.text] + : Diagnostics.Use_import_type, + fixId, + Diagnostics.Fix_all_with_type_only_imports); + + if (some(importDeclarationChanges)) { + return [ + createCodeFixActionWithoutFixAll(fixId, importDeclarationChanges, Diagnostics.Use_import_type), + mainAction + ]; + } + return [mainAction]; } return undefined; }, fixIds: [fixId], getAllCodeActions: function getAllCodeActionsToConvertToTypeOnlyImport(context) { + const fixedImportDeclarations: Set = new Set(); return codeFixAll(context, errorCodes, (changes, diag) => { - const declaration = getDeclaration(diag.file, diag.start); - if (declaration) { - doChange(changes, diag.file, declaration); + const errorDeclaration = getDeclaration(diag.file, diag.start); + if (errorDeclaration?.kind === SyntaxKind.ImportDeclaration && !fixedImportDeclarations.has(errorDeclaration)) { + doChange(changes, diag.file, errorDeclaration); + fixedImportDeclarations.add(errorDeclaration); + } + else if (errorDeclaration?.kind === SyntaxKind.ImportSpecifier + && !fixedImportDeclarations.has(errorDeclaration.parent.parent.parent) + && canConvertImportDeclarationForSpecifier(errorDeclaration, diag.file, context.program) + ) { + doChange(changes, diag.file, errorDeclaration.parent.parent.parent); + fixedImportDeclarations.add(errorDeclaration.parent.parent.parent); + } + else if (errorDeclaration?.kind === SyntaxKind.ImportSpecifier) { + doChange(changes, diag.file, errorDeclaration); } }); } @@ -50,6 +87,31 @@ function getDeclaration(sourceFile: SourceFile, pos: number) { return isImportSpecifier(parent) || isImportDeclaration(parent) && parent.importClause ? parent : undefined; } + +function canConvertImportDeclarationForSpecifier(specifier: ImportSpecifier, sourceFile: SourceFile, program: Program): boolean { + if (specifier.parent.parent.name) { + // An import declaration with a default import and named bindings can't be type-only + return false; + } + const nonTypeOnlySpecifiers = specifier.parent.elements.filter(e => !e.isTypeOnly); + if (nonTypeOnlySpecifiers.length === 1) { + // If the error specifier is on the only non-type-only specifier, we can convert the whole import + return true; + } + // Otherwise, we need to check the usage of the other specifiers + const checker = program.getTypeChecker(); + for (const specifier of nonTypeOnlySpecifiers) { + const isUsedAsValue = FindAllReferences.Core.eachSymbolReferenceInFile(specifier.name, checker, sourceFile, usage => { + return !isValidTypeOnlyAliasUseSite(usage); + }); + if (isUsedAsValue) { + return false; + } + } + // No other specifiers are used as values, so we can convert the whole import + return true; +} + 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)); @@ -73,8 +135,13 @@ function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, de ]); } else { + const newNamedBindings = importClause.namedBindings?.kind === SyntaxKind.NamedImports + ? factory.updateNamedImports( + importClause.namedBindings, + sameMap(importClause.namedBindings.elements, e => factory.updateImportSpecifier(e, /*isTypeOnly*/ false, e.propertyName, e.name))) + : importClause.namedBindings; const importDeclaration = factory.updateImportDeclaration(declaration, declaration.modifiers, - factory.updateImportClause(importClause, /*isTypeOnly*/ true, importClause.name, importClause.namedBindings), declaration.moduleSpecifier, declaration.assertClause); + factory.updateImportClause(importClause, /*isTypeOnly*/ true, importClause.name, newNamedBindings), declaration.moduleSpecifier, declaration.assertClause); changes.replaceNode(sourceFile, declaration, importDeclaration); } } diff --git a/src/services/codefixes/fixUnreferenceableDecoratorMetadata.ts b/src/services/codefixes/fixUnreferenceableDecoratorMetadata.ts index ec5b5e6e3ca90..4c35ee6b9629a 100644 --- a/src/services/codefixes/fixUnreferenceableDecoratorMetadata.ts +++ b/src/services/codefixes/fixUnreferenceableDecoratorMetadata.ts @@ -44,7 +44,7 @@ registerCodeFix({ actions = append(actions, createCodeFixActionWithoutFixAll(fixId, namespaceChanges, Diagnostics.Convert_named_imports_to_namespace_import)); } if (typeOnlyChanges.length) { - actions = append(actions, createCodeFixActionWithoutFixAll(fixId, typeOnlyChanges, Diagnostics.Convert_to_type_only_import)); + actions = append(actions, createCodeFixActionWithoutFixAll(fixId, typeOnlyChanges, Diagnostics.Use_import_type)); } return actions; }, diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts index 2453338f71b34..d8556c97e59c3 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts @@ -20,7 +20,7 @@ goTo.file("imports.ts"); verify.codeFix({ index: 0, - description: ts.Diagnostics.Convert_to_type_only_import.message, + description: ts.Diagnostics.Use_import_type.message, newFileContent: `import type { B, C, diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts index fae95f35dfea7..145998d985482 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts @@ -18,7 +18,7 @@ goTo.file("imports.ts"); verify.codeFix({ index: 0, - description: ts.Diagnostics.Convert_to_type_only_import.message, + description: ts.Diagnostics.Use_import_type.message, newFileContent: `import type A from './exports'; import type { B, C } from './exports'; diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts index 3f9aba15cf0fb..e021992ddd9db 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts @@ -26,7 +26,7 @@ goTo.file("imports.ts"); verify.codeFixAll({ fixId: "convertToTypeOnlyImport", - fixAllDescription: ts.Diagnostics.Convert_all_imports_not_used_as_a_value_to_type_only_imports.message, + fixAllDescription: ts.Diagnostics.Fix_all_with_type_only_imports.message, newFileContent: `import type A from './exports1'; import type { B, C } from './exports1'; import type D from "./exports2"; diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts index 126d0371f4674..32240684df779 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts @@ -12,6 +12,11 @@ goTo.file("/a.ts"); verify.codeFix({ index: 0, - description: ts.Diagnostics.Convert_to_type_only_import.message, + description: ts.Diagnostics.Use_import_type.message, + newFileContent: `import type { I, foo } from "./b";` +}); +verify.codeFix({ + index: 1, + description: `Use 'type I'`, newFileContent: `import { type I, foo } from "./b";` }); diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport5.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport5.ts new file mode 100644 index 0000000000000..1593f55e04920 --- /dev/null +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport5.ts @@ -0,0 +1,24 @@ +/// + +// @module: esnext +// @verbatimModuleSyntax: true +// @filename: /b.ts +////export interface I {} +////export const foo = {}; + +// @filename: /a.ts +////import { I, foo } from "./b"; +////foo; +////export declare const i: I; + +// ^ usage prevents 'Remove unused declaration' fix, +// so that lack of `index` option asserts only one fix is available. +// Specifically, we ensure no `Use 'import type'` fix is offered. + +goTo.file("/a.ts"); +verify.codeFix({ + description: `Use 'type I'`, + newFileContent: `import { type I, foo } from "./b"; +foo; +export declare const i: I;`, +}); diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport6.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport6.ts new file mode 100644 index 0000000000000..66b1315d8ffd5 --- /dev/null +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport6.ts @@ -0,0 +1,26 @@ +/// + +// @module: esnext +// @verbatimModuleSyntax: true +// @filename: /b.ts +////export interface I {} +////export const foo = {}; + +// @filename: /a.ts +//// import { I, type foo } from "./b"; +//// export declare const x: typeof foo; + +goTo.file("/a.ts"); +verify.codeFix({ + index: 0, + description: ts.Diagnostics.Use_import_type.message, + newFileContent: `import type { I, foo } from "./b"; +export declare const x: typeof foo;`, +}); + +verify.codeFix({ + index: 1, + description: `Use 'type I'`, + newFileContent: `import { type I, type foo } from "./b"; +export declare const x: typeof foo;`, +}); diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport7.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport7.ts new file mode 100644 index 0000000000000..d534fb26e84b3 --- /dev/null +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport7.ts @@ -0,0 +1,34 @@ +/// + +// @module: esnext +// @verbatimModuleSyntax: true + +// @Filename: /b.ts +////export interface I {} +////export const foo = {}; + +// @Filename: /c.ts +//// export interface C {} + +// @Filename: /d.ts +//// export interface D {} +//// export function d() {} + +// @Filename: /a.ts +//// import { I, type foo } from "./b"; +//// import { C } from "./c"; +//// import { D, d } from "./d"; +//// export declare const x: typeof foo; +//// d(); + +goTo.file("/a.ts"); +verify.codeFixAll({ + fixId: "convertToTypeOnlyImport", + fixAllDescription: ts.Diagnostics.Fix_all_with_type_only_imports.message, + newFileContent: +`import type { I, foo } from "./b"; +import type { C } from "./c"; +import { type D, d } from "./d"; +export declare const x: typeof foo; +d();`, +}); diff --git a/tests/cases/fourslash/codefixUnreferenceableDecoratorMetadata1.ts b/tests/cases/fourslash/codefixUnreferenceableDecoratorMetadata1.ts index 40f9a7239a762..eee196efa901d 100644 --- a/tests/cases/fourslash/codefixUnreferenceableDecoratorMetadata1.ts +++ b/tests/cases/fourslash/codefixUnreferenceableDecoratorMetadata1.ts @@ -40,7 +40,7 @@ class HelloWorld { verify.codeFix({ index: 1, - description: ts.Diagnostics.Convert_to_type_only_import.message, + description: ts.Diagnostics.Use_import_type.message, errorCode: diag.code, newRangeContent: `import type { I2 } from "./mod";`, });