From 81e99fbc25cc647a3f4247eaa817b9475eaba196 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Mar 2026 18:24:30 +0000 Subject: [PATCH 1/3] Initial plan From ab78177746602e5bee66eb5174f0e5973256e491 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Mar 2026 18:39:41 +0000 Subject: [PATCH 2/3] Fix auto-import to prefer existing type-only imports over value imports When adding a type import, the tryAddToExistingImport function now gives preference to existing `import type` declarations instead of returning the first valid match. This matches the TypeScript reference implementation behavior. Also adds missing check for default imports when addAsTypeOnly is Required and namedBindings exist. Fixes #3029 Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/a6cc4bfe-4eb1-48f4-8a31-62a398c4fbca Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com> --- internal/fourslash/_scripts/failingTests.txt | 2 - ...ImportPreferExistingTypeOnlyImport_test.go | 37 +++++++++++++++++++ internal/ls/autoimport/fix.go | 32 +++++++++++++--- 3 files changed, 64 insertions(+), 7 deletions(-) create mode 100644 internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go diff --git a/internal/fourslash/_scripts/failingTests.txt b/internal/fourslash/_scripts/failingTests.txt index 91c5f85708..eb1fcd3f47 100644 --- a/internal/fourslash/_scripts/failingTests.txt +++ b/internal/fourslash/_scripts/failingTests.txt @@ -26,7 +26,6 @@ TestAutoImportProvider9 TestAutoImportRelativePathToMonorepoPackage TestAutoImportSortCaseSensitivity1 TestAutoImportTypeImport4 -TestAutoImportTypeOnlyPreferred2 TestAutoImportVerbatimTypeOnly1 TestBestCommonTypeObjectLiterals TestBestCommonTypeObjectLiterals1 @@ -232,7 +231,6 @@ TestImportFixesGlobalTypingsCache TestImportMetaCompletionDetails TestImportNameCodeFix_avoidRelativeNodeModules TestImportNameCodeFix_externalNonRelative1 -TestImportNameCodeFix_importType2 TestImportNameCodeFix_noDestructureNonObjectLiteral TestImportNameCodeFix_order2 TestImportNameCodeFix_preferBaseUrl diff --git a/internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go b/internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go new file mode 100644 index 0000000000..6e47974643 --- /dev/null +++ b/internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go @@ -0,0 +1,37 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/testutil" +) + +// Regression test: auto-import should add a type to an existing `import type` +// declaration instead of adding an inline `type` modifier to a value import. +// https://github.com/microsoft/typescript-go/issues/3029 +func TestAutoImportPreferExistingTypeOnlyImport(t *testing.T) { + t.Parallel() + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = `// @Filename: /node_modules/typebox/index.d.ts +export declare const Type: { Object: Function; String: Function }; +export type Static = T; +// @Filename: /main.ts +import { Type } from "typebox"; +import type { } from "typebox"; + +const object = Type.Object({}); + +type ObjectType = Static/**/;` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + f.GoToMarker(t, "") + f.VerifyImportFixAtPosition(t, []string{ + `import { Type } from "typebox"; +import type { Static } from "typebox"; + +const object = Type.Object({}); + +type ObjectType = Static;`, + }, nil /*preferences*/) +} diff --git a/internal/ls/autoimport/fix.go b/internal/ls/autoimport/fix.go index b2107443f3..800245f903 100644 --- a/internal/ls/autoimport/fix.go +++ b/internal/ls/autoimport/fix.go @@ -695,6 +695,7 @@ func (v *View) tryAddToExistingImport( addAsTypeOnly := getAddAsTypeOnly(isValidTypeOnlyUseSite, export, v.program.Options()) + var best *Fix for _, existingImport := range matchingDeclarations { if existingImport.node.Kind == ast.KindImportEqualsDeclaration { continue @@ -702,7 +703,7 @@ func (v *View) tryAddToExistingImport( if existingImport.node.Kind == ast.KindVariableDeclaration { if (importKind == lsproto.ImportKindNamed || importKind == lsproto.ImportKindDefault) && existingImport.node.Name().Kind == ast.KindObjectBindingPattern { - return &Fix{ + fix := &Fix{ AutoImportFix: &lsproto.AutoImportFix{ Kind: lsproto.AutoImportFixKindAddToExisting, Name: export.Name(), @@ -712,6 +713,15 @@ func (v *View) tryAddToExistingImport( AddAsTypeOnly: addAsTypeOnly, }, } + // Variable declarations are never type-only. + // Give preference to putting types in existing type-only imports and avoiding conversions + // of import statements to/from type-only. + if addAsTypeOnly == lsproto.AddAsTypeOnlyNotAllowed { + return fix + } + if best == nil { + best = fix + } } continue } @@ -730,8 +740,9 @@ func (v *View) tryAddToExistingImport( continue } - if importKind == lsproto.ImportKindDefault && importClause.Name() != nil { - // Cannot add a default import to a declaration that already has one + if importKind == lsproto.ImportKindDefault && (importClause.Name() != nil || + // Cannot add a default import as type-only if the import already has named bindings + addAsTypeOnly == lsproto.AddAsTypeOnlyRequired && namedBindings != nil) { continue } @@ -740,7 +751,7 @@ func (v *View) tryAddToExistingImport( continue } - return &Fix{ + fix := &Fix{ AutoImportFix: &lsproto.AutoImportFix{ Kind: lsproto.AutoImportFixKindAddToExisting, Name: export.Name(), @@ -750,9 +761,20 @@ func (v *View) tryAddToExistingImport( AddAsTypeOnly: addAsTypeOnly, }, } + + isTypeOnly := importClause.IsTypeOnly() + // Give preference to putting types in existing type-only imports and avoiding conversions + // of import statements to/from type-only. + if (addAsTypeOnly != lsproto.AddAsTypeOnlyNotAllowed && isTypeOnly) || + (addAsTypeOnly == lsproto.AddAsTypeOnlyNotAllowed && !isTypeOnly) { + return fix + } + if best == nil { + best = fix + } } - return nil + return best } func getImportKind(importingFile *ast.SourceFile, export *Export, program *compiler.Program) lsproto.ImportKind { From c1f878a9be43af7e16bba8f932b40f4793e1b108 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 26 Mar 2026 19:01:55 +0000 Subject: [PATCH 3/3] Delete redundant test; existing gen tests cover the scenario Agent-Logs-Url: https://github.com/microsoft/typescript-go/sessions/b33f7744-e985-4803-99ef-7d2ff91321aa Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com> --- ...ImportPreferExistingTypeOnlyImport_test.go | 37 ------------------- 1 file changed, 37 deletions(-) delete mode 100644 internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go diff --git a/internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go b/internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go deleted file mode 100644 index 6e47974643..0000000000 --- a/internal/fourslash/tests/autoImportPreferExistingTypeOnlyImport_test.go +++ /dev/null @@ -1,37 +0,0 @@ -package fourslash_test - -import ( - "testing" - - "github.com/microsoft/typescript-go/internal/fourslash" - "github.com/microsoft/typescript-go/internal/testutil" -) - -// Regression test: auto-import should add a type to an existing `import type` -// declaration instead of adding an inline `type` modifier to a value import. -// https://github.com/microsoft/typescript-go/issues/3029 -func TestAutoImportPreferExistingTypeOnlyImport(t *testing.T) { - t.Parallel() - defer testutil.RecoverAndFail(t, "Panic on fourslash test") - const content = `// @Filename: /node_modules/typebox/index.d.ts -export declare const Type: { Object: Function; String: Function }; -export type Static = T; -// @Filename: /main.ts -import { Type } from "typebox"; -import type { } from "typebox"; - -const object = Type.Object({}); - -type ObjectType = Static/**/;` - f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) - defer done() - f.GoToMarker(t, "") - f.VerifyImportFixAtPosition(t, []string{ - `import { Type } from "typebox"; -import type { Static } from "typebox"; - -const object = Type.Object({}); - -type ObjectType = Static;`, - }, nil /*preferences*/) -}