Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions src/compiler/diagnosticMessages.json
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
77 changes: 72 additions & 5 deletions src/services/codefixes/convertToTypeOnlyImport.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import {
Diagnostics,
factory,
FindAllReferences,
getSynthesizedDeepClone,
getSynthesizedDeepClones,
getTokenAtPosition,
Expand All @@ -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";

Expand All @@ -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),
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m not sure if the two fix options should have different fixNames

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change looks good to me but i don't know about this either.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Asked at the editor sync and this is only used for telemetry, and the telemetry stakeholders don’t care if they have the same name.

mainAction
];
}
return [mainAction];
}
return undefined;
},
fixIds: [fixId],
getAllCodeActions: function getAllCodeActionsToConvertToTypeOnlyImport(context) {
const fixedImportDeclarations: Set<ImportDeclaration> = 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);
}
});
}
Expand All @@ -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));
Expand All @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
},
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down
7 changes: 6 additions & 1 deletion tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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";`
});
24 changes: 24 additions & 0 deletions tests/cases/fourslash/codeFixConvertToTypeOnlyImport5.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/// <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";
////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;`,
});
26 changes: 26 additions & 0 deletions tests/cases/fourslash/codeFixConvertToTypeOnlyImport6.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/// <reference path="fourslash.ts" />

// @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;`,
});
34 changes: 34 additions & 0 deletions tests/cases/fourslash/codeFixConvertToTypeOnlyImport7.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/// <reference path="fourslash.ts" />

// @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();`,
});
Original file line number Diff line number Diff line change
Expand Up @@ -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";`,
});