From 65aa7e624d839e0dea629adcce5876811cb95f5f Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 23 Jun 2026 21:23:02 +0000 Subject: [PATCH 1/3] Add failing test case. --- ...ExportsUsingTypeFromUnimportedFile_test.go | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go diff --git a/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go b/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go new file mode 100644 index 00000000000..db463f2c4f9 --- /dev/null +++ b/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go @@ -0,0 +1,44 @@ +package fourslash_test + +import ( + "testing" + + "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/testutil" +) + +func TestCodeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile(t *testing.T) { + t.Parallel() + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = ` +// @module: preserve +// @verbatimModuleSyntax: true +// @declaration: true +// @isolatedDeclarations: true + +// @Filename: /types.ts +export type Thing = {}; + +// @Filename: /funcs.ts +import type { Thing } from "./types"; + +export function makeThing(): Thing { + throw {}; +} + +// @Filename: /exporter.ts +import { makeThing } from "./funcs"; + +export let thing/**/ = makeThing();` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + + f.GoToMarker(t, "") + f.VerifyCodeFix(t, fourslash.VerifyCodeFixOptions{ + Description: "Add annotation of type 'Thing'", + NewFileContent: `import { makeThing } from "./funcs"; +import type { Thing } from "./types"; + +export let thing: Thing = makeThing();`, + }) +} From 2e12857e47f1a97f6d69450c696dfac19ecc28a1 Mon Sep 17 00:00:00 2001 From: Daniel Rosenwasser Date: Tue, 23 Jun 2026 21:23:23 +0000 Subject: [PATCH 2/3] Add extremely naive fix. --- .../codeactions_fixmissingtypeannotation.go | 74 +++++++++++++------ 1 file changed, 50 insertions(+), 24 deletions(-) diff --git a/internal/ls/codeactions_fixmissingtypeannotation.go b/internal/ls/codeactions_fixmissingtypeannotation.go index b218d270771..c1b325627bd 100644 --- a/internal/ls/codeactions_fixmissingtypeannotation.go +++ b/internal/ls/codeactions_fixmissingtypeannotation.go @@ -91,11 +91,14 @@ func getIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *CodeFix var fixes []*CodeAction - addFix := func(action *CodeAction) { - if action == nil { - return + addFix := func(action *CodeAction, err error) error { + if err != nil { + return err } - fixes = append(fixes, action) + if action != nil { + fixes = append(fixes, action) + } + return nil } // Match TS ordering: Full annotation, Relative annotation, Widened annotation, @@ -103,24 +106,30 @@ func getIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *CodeFix modes := []typePrintMode{typePrintModeFull, typePrintModeRelative, typePrintModeWidened} for _, mode := range modes { - addFix(tryCodeAction(ctx, fixContext, ch, func(f *isolatedDeclarationsFixer) string { + if err := addFix(tryCodeAction(ctx, fixContext, ch, func(f *isolatedDeclarationsFixer) string { f.typePrintMode = mode return f.addTypeAnnotation(fixContext.Span) - })) + })); err != nil { + return nil, err + } } for _, mode := range modes { - addFix(tryCodeAction(ctx, fixContext, ch, func(f *isolatedDeclarationsFixer) string { + if err := addFix(tryCodeAction(ctx, fixContext, ch, func(f *isolatedDeclarationsFixer) string { f.typePrintMode = mode return f.addInlineAssertion(fixContext.Span) - })) + })); err != nil { + return nil, err + } } // extractAsVariable only in Full mode - addFix(tryCodeAction(ctx, fixContext, ch, func(f *isolatedDeclarationsFixer) string { + if err := addFix(tryCodeAction(ctx, fixContext, ch, func(f *isolatedDeclarationsFixer) string { f.typePrintMode = typePrintModeFull return f.extractAsVariable(fixContext.Span) - })) + })); err != nil { + return nil, err + } return fixes, nil } @@ -130,12 +139,17 @@ func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *Code defer done() changeTracker := change.NewTracker(ctx, fixContext.Program.Options(), fixContext.LS.FormatOptions(), fixContext.LS.converters) + importAdder, err := createImportAdder(ctx, fixContext, ch) + if err != nil { + return nil, err + } fixer := &isolatedDeclarationsFixer{ sourceFile: fixContext.SourceFile, program: fixContext.Program, checker: ch, changeTracker: changeTracker, + importAdder: importAdder, locale: locale.FromContext(ctx), fixedNodes: make(map[*ast.Node]bool), typePrintMode: typePrintModeFull, @@ -149,12 +163,17 @@ func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *Code } } - for _, sym := range fixer.symbolsToImport { - fixer.addSymbolToExistingImport(sym) + if importAdder == nil { + for _, sym := range fixer.symbolsToImport { + fixer.addSymbolToExistingImport(sym) + } } changes := changeTracker.GetChanges() fileChanges := changes[fixContext.SourceFile.FileName()] + if importAdder != nil && importAdder.HasFixes() { + fileChanges = append(fileChanges, importAdder.Edits()...) + } if len(fileChanges) == 0 { return nil, nil } @@ -165,12 +184,12 @@ func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *Code }, nil } -func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker.Checker, fn func(*isolatedDeclarationsFixer) string) *CodeAction { +func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker.Checker, fn func(*isolatedDeclarationsFixer) string) (*CodeAction, error) { changeTracker := change.NewTracker(ctx, fixContext.Program.Options(), fixContext.LS.FormatOptions(), fixContext.LS.converters) - - var importAdder autoimport.ImportAdder - // importAdder may be nil if the auto-import registry is not available; - // type node transformation still works without it, just without adding imports. + importAdder, err := createImportAdder(ctx, fixContext, ch) + if err != nil { + return nil, err + } fixer := &isolatedDeclarationsFixer{ sourceFile: fixContext.SourceFile, @@ -184,12 +203,13 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker. description := fn(fixer) if description == "" { - return nil + return nil, nil } - // Add any symbols that need to be imported to existing import declarations - for _, sym := range fixer.symbolsToImport { - fixer.addSymbolToExistingImport(sym) + if importAdder == nil { + for _, sym := range fixer.symbolsToImport { + fixer.addSymbolToExistingImport(sym) + } } changes := changeTracker.GetChanges() @@ -201,7 +221,7 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker. } if len(fileChanges) == 0 { - return nil + return nil, nil } return &CodeAction{ @@ -209,7 +229,7 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker. Changes: fileChanges, FixID: fixMissingTypeAnnotationOnExportsFixID, FixAllDescription: diagnostics.Add_all_missing_type_annotations.Localize(locale.FromContext(ctx)), - } + }, nil } // isolatedDeclarationsFixer encapsulates the state for fixing isolated declarations errors. @@ -1197,7 +1217,13 @@ func (f *isolatedDeclarationsFixer) typeToMinimizedReferenceType(t *checker.Type referenceTypeNode, importableSymbols := autoimport.TryGetAutoImportableReferenceFromTypeNode(typeNode, idToSymbol) if referenceTypeNode != nil { typeNode = referenceTypeNode - f.symbolsToImport = append(f.symbolsToImport, importableSymbols...) + if f.importAdder != nil { + for _, symbol := range importableSymbols { + f.importAdder.AddImportFromExportedSymbol(symbol, true /*isValidTypeOnlyUseSite*/) + } + } else { + f.symbolsToImport = append(f.symbolsToImport, importableSymbols...) + } } return typeNode } From acd3825d6d080e22b271c1dccd9b498a2968262d Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Mon, 17 Aug 2026 21:18:23 +0000 Subject: [PATCH 3/3] Bail when import type annotation symbol is out of scope Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com> --- ...ExportsUsingTypeFromUnimportedFile_test.go | 77 ++++++++++++++++ .../codeactions_fixmissingtypeannotation.go | 90 +++++-------------- 2 files changed, 98 insertions(+), 69 deletions(-) diff --git a/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go b/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go index db463f2c4f9..dba9cc9b6d1 100644 --- a/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go +++ b/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go @@ -3,7 +3,9 @@ package fourslash_test import ( "testing" + "github.com/microsoft/typescript-go/internal/core" "github.com/microsoft/typescript-go/internal/fourslash" + "github.com/microsoft/typescript-go/internal/ls/lsutil" "github.com/microsoft/typescript-go/internal/testutil" ) @@ -42,3 +44,78 @@ import type { Thing } from "./types"; export let thing: Thing = makeThing();`, }) } + +func TestCodeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFileNoAutoImport(t *testing.T) { + t.Parallel() + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = ` +// @module: preserve +// @verbatimModuleSyntax: true +// @declaration: true +// @isolatedDeclarations: true + +// @Filename: /types.ts +export type Thing = {}; + +// @Filename: /funcs.ts +import type { Thing } from "./types"; + +export function makeThing(): Thing { + throw {}; +} + +// @Filename: /exporter.ts +import { makeThing } from "./funcs"; + +export let thing/**/ = makeThing();` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + + prefs := lsutil.NewDefaultUserPreferences() + prefs.IncludeCompletionsForModuleExports = core.TSFalse + f.Configure(t, prefs) + + f.GoToMarker(t, "") + f.VerifyCodeFixNotAvailable(t, "Add annotation of type 'Thing'") +} + +func TestCodeFixMissingTypeAnnotationOnExportsUsingTypeFromImportedFileNoAutoImport(t *testing.T) { + t.Parallel() + defer testutil.RecoverAndFail(t, "Panic on fourslash test") + const content = ` +// @module: preserve +// @verbatimModuleSyntax: true +// @declaration: true +// @isolatedDeclarations: true + +// @Filename: /types.ts +export type Thing = {}; + +// @Filename: /funcs.ts +import type { Thing } from "./types"; + +export function makeThing(): Thing { + throw {}; +} + +// @Filename: /exporter.ts +import { makeThing } from "./funcs"; +import type { Thing } from "./types"; + +export let thing/**/ = makeThing();` + f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content) + defer done() + + prefs := lsutil.NewDefaultUserPreferences() + prefs.IncludeCompletionsForModuleExports = core.TSFalse + f.Configure(t, prefs) + + f.GoToMarker(t, "") + f.VerifyCodeFix(t, fourslash.VerifyCodeFixOptions{ + Description: "Add annotation of type 'Thing'", + NewFileContent: `import { makeThing } from "./funcs"; +import type { Thing } from "./types"; + +export let thing: Thing = makeThing();`, + }) +} diff --git a/internal/ls/codeactions_fixmissingtypeannotation.go b/internal/ls/codeactions_fixmissingtypeannotation.go index c1b325627bd..64915e542d4 100644 --- a/internal/ls/codeactions_fixmissingtypeannotation.go +++ b/internal/ls/codeactions_fixmissingtypeannotation.go @@ -163,12 +163,6 @@ func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *Code } } - if importAdder == nil { - for _, sym := range fixer.symbolsToImport { - fixer.addSymbolToExistingImport(sym) - } - } - changes := changeTracker.GetChanges() fileChanges := changes[fixContext.SourceFile.FileName()] if importAdder != nil && importAdder.HasFixes() { @@ -206,12 +200,6 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker. return nil, nil } - if importAdder == nil { - for _, sym := range fixer.symbolsToImport { - fixer.addSymbolToExistingImport(sym) - } - } - changes := changeTracker.GetChanges() fileChanges := changes[fixContext.SourceFile.FileName()] @@ -234,16 +222,15 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker. // isolatedDeclarationsFixer encapsulates the state for fixing isolated declarations errors. type isolatedDeclarationsFixer struct { - sourceFile *ast.SourceFile - program *compiler.Program - checker *checker.Checker - changeTracker *change.Tracker - importAdder autoimport.ImportAdder - locale locale.Locale - fixedNodes map[*ast.Node]bool - typePrintMode typePrintMode - symbolsToImport []*ast.Symbol - mutatedTarget bool // set by inferType/relativeType when the target was mutated (e.g., spread decomposition) + sourceFile *ast.SourceFile + program *compiler.Program + checker *checker.Checker + changeTracker *change.Tracker + importAdder autoimport.ImportAdder + locale locale.Locale + fixedNodes map[*ast.Node]bool + typePrintMode typePrintMode + mutatedTarget bool // set by inferType/relativeType when the target was mutated (e.g., spread decomposition) } func (f *isolatedDeclarationsFixer) addTypeAnnotation(span core.TextRange) string { @@ -1216,18 +1203,27 @@ func (f *isolatedDeclarationsFixer) typeToMinimizedReferenceType(t *checker.Type // and collect symbols that need to be imported referenceTypeNode, importableSymbols := autoimport.TryGetAutoImportableReferenceFromTypeNode(typeNode, idToSymbol) if referenceTypeNode != nil { - typeNode = referenceTypeNode if f.importAdder != nil { for _, symbol := range importableSymbols { f.importAdder.AddImportFromExportedSymbol(symbol, true /*isValidTypeOnlyUseSite*/) } - } else { - f.symbolsToImport = append(f.symbolsToImport, importableSymbols...) + } else if !f.allSymbolsAccessibleInScope(importableSymbols, enclosingDecl) { + return nil } + typeNode = referenceTypeNode } return typeNode } +func (f *isolatedDeclarationsFixer) allSymbolsAccessibleInScope(symbols []*ast.Symbol, enclosingDecl *ast.Node) bool { + for _, symbol := range symbols { + if symbol == nil || !f.checker.IsSymbolAccessibleByFlags(symbol, enclosingDecl, ast.SymbolFlagsType) { + return false + } + } + return true +} + // endOfRequiredTypeParameters finds the number of type arguments that are // actually required (i.e., differ from their defaults). Ported from TS's // services/codefixes/helpers.ts endOfRequiredTypeParameters. @@ -1399,47 +1395,3 @@ func getIdentifierNameForNode(node *ast.Node) string { } return "newLocal" } - -// addSymbolToExistingImport finds the existing import declaration for the symbol's module -// and adds the symbol name to the named imports. -func (f *isolatedDeclarationsFixer) addSymbolToExistingImport(sym *ast.Symbol) { - if sym == nil || sym.Parent == nil { - return - } - - // Find the module specifier for this symbol - moduleSymbol := sym.Parent - symbolName := sym.Name - - // Walk the source file's import declarations to find the one importing from the same module - for _, stmt := range f.sourceFile.Statements.Nodes { - if !ast.IsImportDeclaration(stmt) { - continue - } - importDecl := stmt.AsImportDeclaration() - if importDecl.ImportClause == nil { - continue - } - - // Check if this import is from the same module - importModuleSymbol := f.checker.GetSymbolAtLocation(importDecl.ModuleSpecifier) - if importModuleSymbol == nil || f.checker.GetMergedSymbol(importModuleSymbol) != f.checker.GetMergedSymbol(moduleSymbol) { - continue - } - - // Found the matching import - add the symbol to named imports - importClause := importDecl.ImportClause.AsImportClause() - if importClause.NamedBindings != nil && ast.IsNamedImports(importClause.NamedBindings) { - // Add to existing named imports - existingElements := importClause.NamedBindings.AsNamedImports().Elements.Nodes - factory := f.changeTracker.NodeFactory - newSpecifier := factory.NewImportSpecifier(false, nil, factory.NewIdentifier(symbolName)) - newElements := append(existingElements, newSpecifier.AsNode()) - newNamedImports := factory.NewNamedImports(factory.NewNodeList(newElements)) - newImportClause := factory.UpdateImportClause(importClause, importClause.PhaseModifier, importClause.Name(), newNamedImports) - newImportDecl := factory.UpdateImportDeclaration(importDecl, importDecl.Modifiers(), newImportClause, importDecl.ModuleSpecifier, importDecl.Attributes) - f.changeTracker.ReplaceNode(f.sourceFile, stmt, newImportDecl.AsNode(), nil) - } - return - } -}