From 3b1c4901627012dd1998ff81d0eca805a1e90697 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 2 Feb 2023 12:07:34 +0200 Subject: [PATCH 1/4] fix(52444): add QF to handle verbatimModuleSyntax errors --- .../codefixes/convertToTypeOnlyImport.ts | 68 ++++++++++--------- .../codeFixConvertToTypeOnlyImport1.ts | 14 ++-- .../codeFixConvertToTypeOnlyImport2.ts | 6 +- .../codeFixConvertToTypeOnlyImport3.ts | 6 +- .../codeFixConvertToTypeOnlyImport4.ts | 17 +++++ 5 files changed, 65 insertions(+), 46 deletions(-) create mode 100644 tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts diff --git a/src/services/codefixes/convertToTypeOnlyImport.ts b/src/services/codefixes/convertToTypeOnlyImport.ts index b4f5bda36cafa..73215885c3c2e 100644 --- a/src/services/codefixes/convertToTypeOnlyImport.ts +++ b/src/services/codefixes/convertToTypeOnlyImport.ts @@ -1,14 +1,14 @@ import { - CodeFixContextBase, Diagnostics, factory, getTokenAtPosition, + ImportClause, ImportDeclaration, + ImportSpecifier, isImportDeclaration, + isImportSpecifier, SourceFile, textChanges, - TextSpan, - tryCast, } from "../_namespaces/ts"; import { codeFixAll, @@ -16,52 +16,54 @@ import { 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(declaration.modifiers, factory.createImportClause(/*isTypeOnly*/ true, importClause.name, /*namedBindings*/ undefined), declaration.moduleSpecifier, declaration.assertClause), + factory.createImportDeclaration(declaration.modifiers, factory.createImportClause(/*isTypeOnly*/ true, /*name*/ undefined, importClause.namedBindings), declaration.moduleSpecifier, declaration.assertClause), + ]); + } + 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); + } } } diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts index cd7fb8b05c399..2453338f71b34 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport1.ts +++ b/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; diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts index 9cd7111d05cc7..fae95f35dfea7 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport2.ts +++ 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; diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts index 0544e3035cdf4..3f9aba15cf0fb 100644 --- a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport3.ts +++ b/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"; diff --git a/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts new file mode 100644 index 0000000000000..126d0371f4674 --- /dev/null +++ b/tests/cases/fourslash/codeFixConvertToTypeOnlyImport4.ts @@ -0,0 +1,17 @@ +/// + +// @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";` +}); From e14e3ad8149abef1226b0bdc4cf47c7ab9d46d73 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Thu, 2 Feb 2023 12:21:24 +0200 Subject: [PATCH 2/4] update baseline --- .../plugins/getSupportedCodeFixes-can-be-proxied.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/baselines/reference/tsserver/plugins/getSupportedCodeFixes-can-be-proxied.js b/tests/baselines/reference/tsserver/plugins/getSupportedCodeFixes-can-be-proxied.js index 189d354018da9..17609c6ca1981 100644 --- a/tests/baselines/reference/tsserver/plugins/getSupportedCodeFixes-can-be-proxied.js +++ b/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", "2690", "2420", "2720", @@ -720,7 +721,6 @@ Info 32 [00:01:13.000] response: "1477", "1478", "1479", - "1484", "1485", "2200", "2201", @@ -1657,6 +1657,7 @@ Info 38 [00:01:19.000] response: "2713", "1205", "1371", + "1484", "2690", "2420", "2720", @@ -2068,7 +2069,6 @@ Info 38 [00:01:19.000] response: "1477", "1478", "1479", - "1484", "1485", "2200", "2201", @@ -2917,6 +2917,7 @@ Info 40 [00:01:21.000] response: "2713", "1205", "1371", + "1484", "2690", "2420", "2720", @@ -3328,7 +3329,6 @@ Info 40 [00:01:21.000] response: "1477", "1478", "1479", - "1484", "1485", "2200", "2201", From 4bb141d27ef45a20dfcfcba5fdc763c7ed06c3d3 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Fri, 3 Feb 2023 14:36:08 +0200 Subject: [PATCH 3/4] clone nodes --- src/services/codefixes/convertToTypeOnlyImport.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/services/codefixes/convertToTypeOnlyImport.ts b/src/services/codefixes/convertToTypeOnlyImport.ts index 73215885c3c2e..90a63fce94bcb 100644 --- a/src/services/codefixes/convertToTypeOnlyImport.ts +++ b/src/services/codefixes/convertToTypeOnlyImport.ts @@ -1,6 +1,8 @@ import { Diagnostics, factory, + getSynthesizedDeepClone, + getSynthesizedDeepClones, getTokenAtPosition, ImportClause, ImportDeclaration, @@ -55,9 +57,12 @@ function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, de else { const importClause = declaration.importClause as ImportClause; if (importClause.name && importClause.namedBindings) { + const modifiers = getSynthesizedDeepClones(declaration.modifiers, /*includeTrivia*/ true); + const moduleSpecifier = getSynthesizedDeepClone(declaration.moduleSpecifier, /*includeTrivia*/ true); + const assertClause = getSynthesizedDeepClone(declaration.assertClause, /*includeTrivia*/ true); changes.replaceNodeWithNodes(sourceFile, declaration, [ - factory.createImportDeclaration(declaration.modifiers, factory.createImportClause(/*isTypeOnly*/ true, importClause.name, /*namedBindings*/ undefined), declaration.moduleSpecifier, declaration.assertClause), - factory.createImportDeclaration(declaration.modifiers, factory.createImportClause(/*isTypeOnly*/ true, /*name*/ undefined, importClause.namedBindings), declaration.moduleSpecifier, declaration.assertClause), + factory.createImportDeclaration(modifiers, factory.createImportClause(/*isTypeOnly*/ true, getSynthesizedDeepClone(importClause.name, /*includeTrivia*/ true), /*namedBindings*/ undefined), moduleSpecifier, assertClause), + factory.createImportDeclaration(modifiers, factory.createImportClause(/*isTypeOnly*/ true, /*name*/ undefined, getSynthesizedDeepClone(importClause.namedBindings, /*includeTrivia*/ true)), moduleSpecifier, assertClause), ]); } else { From a2572f21a9f78943d237c2f22fbd203a1b40c733 Mon Sep 17 00:00:00 2001 From: Oleksandr T Date: Wed, 8 Feb 2023 08:41:16 +0200 Subject: [PATCH 4/4] create own copies of the import arguments --- .../codefixes/convertToTypeOnlyImport.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/services/codefixes/convertToTypeOnlyImport.ts b/src/services/codefixes/convertToTypeOnlyImport.ts index 90a63fce94bcb..5b553bb33d0e5 100644 --- a/src/services/codefixes/convertToTypeOnlyImport.ts +++ b/src/services/codefixes/convertToTypeOnlyImport.ts @@ -57,12 +57,19 @@ function doChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, de else { const importClause = declaration.importClause as ImportClause; if (importClause.name && importClause.namedBindings) { - const modifiers = getSynthesizedDeepClones(declaration.modifiers, /*includeTrivia*/ true); - const moduleSpecifier = getSynthesizedDeepClone(declaration.moduleSpecifier, /*includeTrivia*/ true); - const assertClause = getSynthesizedDeepClone(declaration.assertClause, /*includeTrivia*/ true); changes.replaceNodeWithNodes(sourceFile, declaration, [ - factory.createImportDeclaration(modifiers, factory.createImportClause(/*isTypeOnly*/ true, getSynthesizedDeepClone(importClause.name, /*includeTrivia*/ true), /*namedBindings*/ undefined), moduleSpecifier, assertClause), - factory.createImportDeclaration(modifiers, factory.createImportClause(/*isTypeOnly*/ true, /*name*/ undefined, getSynthesizedDeepClone(importClause.namedBindings, /*includeTrivia*/ true)), moduleSpecifier, assertClause), + 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 {