From 40b3db5d1c1a391180312bc7de05a8e28d1692e6 Mon Sep 17 00:00:00 2001 From: Noamaan Mulla Date: Tue, 15 Sep 2026 21:40:55 +0000 Subject: [PATCH] Fix import type code fix comment duplication --- ...FixConvertToTypeOnlyImportComments_test.go | 31 ++++++++ tsc/internal/ls/codeactions.go | 1 + .../ls/codeactions_converttotypeonlyimport.go | 76 +++++++++++++++++++ 3 files changed, 108 insertions(+) create mode 100644 tsc/internal/fourslash/tests/codeFixConvertToTypeOnlyImportComments_test.go create mode 100644 tsc/internal/ls/codeactions_converttotypeonlyimport.go diff --git a/tsc/internal/fourslash/tests/codeFixConvertToTypeOnlyImportComments_test.go b/tsc/internal/fourslash/tests/codeFixConvertToTypeOnlyImportComments_test.go new file mode 100644 index 0000000000000..ff2c4f985e0e9 --- /dev/null +++ b/tsc/internal/fourslash/tests/codeFixConvertToTypeOnlyImportComments_test.go @@ -0,0 +1,31 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/TypeScript/tsc/internal/fourslash" + "github.com/microsoft/TypeScript/tsc/internal/testutil" +) + +func TestCodeFixConvertToTypeOnlyImportPreservesComments(t *testing.T) { + t.Parallel() + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = `// @module: esnext +// @verbatimModuleSyntax: true +// @Filename: /foo.ts +export type foo = "foo"; +// @Filename: /index.ts +// upper comment +/*left comment*/ import { fo/**/o } from "./foo"; // right comment +// lower comment` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + f.GoToMarker(t, "") + f.VerifyCodeFix(t, fourslash.VerifyCodeFixOptions{ + Description: "Use 'import type'", + NewFileContent: `// upper comment +/*left comment*/ import type { foo } from "./foo"; // right comment +// lower comment`, + Index: 0, + }) +} diff --git a/tsc/internal/ls/codeactions.go b/tsc/internal/ls/codeactions.go index 6874d861e9a83..0da0aaf0eeaae 100644 --- a/tsc/internal/ls/codeactions.go +++ b/tsc/internal/ls/codeactions.go @@ -70,6 +70,7 @@ type CombinedCodeActions struct { // codeFixProviders is the list of all registered code fix providers var codeFixProviders = []*CodeFixProvider{ ImportFixProvider, + ConvertToTypeOnlyImportProvider, IsolatedDeclarationsFixProvider, FixClassIncorrectlyImplementsInterfaceProvider, // Add more code fix providers here as they are implemented diff --git a/tsc/internal/ls/codeactions_converttotypeonlyimport.go b/tsc/internal/ls/codeactions_converttotypeonlyimport.go new file mode 100644 index 0000000000000..1b0ada3d42a2e --- /dev/null +++ b/tsc/internal/ls/codeactions_converttotypeonlyimport.go @@ -0,0 +1,76 @@ +package ls + +import ( + "context" + + "github.com/microsoft/TypeScript/tsc/internal/ast" + "github.com/microsoft/TypeScript/tsc/internal/astnav" + "github.com/microsoft/TypeScript/tsc/internal/core" + "github.com/microsoft/TypeScript/tsc/internal/diagnostics" + "github.com/microsoft/TypeScript/tsc/internal/locale" + "github.com/microsoft/TypeScript/tsc/internal/ls/change" + "github.com/microsoft/TypeScript/tsc/internal/scanner" +) + +const convertToTypeOnlyImportFixID = "convertToTypeOnlyImport" + +var convertToTypeOnlyImportErrorCodes = []int32{ + diagnostics.X_0_is_a_type_and_must_be_imported_using_a_type_only_import_when_verbatimModuleSyntax_is_enabled.Code(), + diagnostics.X_0_resolves_to_a_type_only_declaration_and_must_be_imported_using_a_type_only_import_when_verbatimModuleSyntax_is_enabled.Code(), +} + +var ConvertToTypeOnlyImportProvider = &CodeFixProvider{ + ErrorCodes: convertToTypeOnlyImportErrorCodes, + GetCodeActions: getConvertToTypeOnlyImportCodeActions, + FixIds: []string{convertToTypeOnlyImportFixID}, +} + +func getConvertToTypeOnlyImportCodeActions(ctx context.Context, fixContext *CodeFixContext) ([]*CodeAction, error) { + importDeclaration := getImportDeclarationForSoleSpecifier(fixContext.SourceFile, fixContext.Span.Pos()) + if importDeclaration == nil { + return nil, nil + } + + tracker := change.NewTracker(ctx, fixContext.Program.Options(), fixContext.LS.FormatOptions(), fixContext.LS.converters) + // Do not reprint the import declaration: its attached trivia would be emitted + // again while the original comments remain outside the replacement range. + scan := scanner.GetScannerForSourceFile(fixContext.SourceFile, importDeclaration.Pos()) + if scan.Token() != ast.KindImportKeyword { + return nil, nil + } + position := scan.TokenEnd() + tracker.ReplaceTextRangeWithText(fixContext.SourceFile, core.NewTextRange(position, position), " type") + + changes, unmappable := tracker.GetChanges() + if len(unmappable) != 0 { + return nil, nil + } + return []*CodeAction{{ + Description: diagnostics.Use_import_type.Localize(locale.FromContext(ctx)), + Changes: changes[fixContext.SourceFile.OriginalFileName()], + FixID: convertToTypeOnlyImportFixID, + }}, nil +} + +// getImportDeclarationForSoleSpecifier limits this text-only edit to the +// `import { Foo } from "..."` shape, where adding `type` changes no other import. +func getImportDeclarationForSoleSpecifier(sourceFile *ast.SourceFile, pos int) *ast.Node { + token := astnav.GetTokenAtPosition(sourceFile, pos) + if token == nil || token.Parent == nil || token.Parent.Kind != ast.KindImportSpecifier { + return nil + } + specifier := token.Parent + if specifier.Parent == nil || specifier.Parent.Kind != ast.KindNamedImports || specifier.Parent.Parent == nil || specifier.Parent.Parent.Kind != ast.KindImportClause || specifier.Parent.Parent.Parent == nil || specifier.Parent.Parent.Parent.Kind != ast.KindImportDeclaration { + return nil + } + + namedImports := specifier.Parent.AsNamedImports() + if len(namedImports.Elements.Nodes) != 1 || namedImports.Elements.Nodes[0] != specifier { + return nil + } + importClause := specifier.Parent.Parent.AsImportClause() + if importClause.IsTypeOnly() || importClause.Name() != nil { + return nil + } + return specifier.Parent.Parent.Parent +}