Skip to content

Use auto-imports for isolatedDeclarations fixes #4422

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Daniel Rosenwasser (DanielRosenwasser) wants to merge 4 commits into from
+181 −82
Closed

Use auto-imports for isolatedDeclarations fixes #4422

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
65aa7e6
Add failing test case.
DanielRosenwasser Jun 23, 2026
2e12857
Add extremely naive fix.
DanielRosenwasser Jun 23, 2026
acd3825
Bail when import type annotation symbol is out of scope
Copilot Aug 17, 2026
908dfcb
Merge branch 'main' into isolatedDeclarationsFixesWithNewImports
DanielRosenwasser Aug 19, 2026
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
121 changes: 121 additions & 0 deletions .../fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go
View file
Open in desktop

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Aug 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Technically one of the test names doesn't fit into this file name but it's not a huge deal.

Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
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"
)

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();`,
})
}

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();`,
})
}
142 changes: 60 additions & 82 deletions internal/ls/codeactions_fixmissingtypeannotation.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,36 +91,45 @@ 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,
// Full inline, Relative inline, Widened inline, Full extract
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
}
Expand All @@ -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,
Expand All @@ -149,12 +163,11 @@ func getAllIsolatedDeclarationsCodeActions(ctx context.Context, fixContext *Code
}
}

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
}
Expand All @@ -165,12 +178,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)

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Aug 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

One reason I think I didn't push this PR harder earlier is I didn't want us to create a distinct import adder for every fix. Might not be a big deal.

@jakebailey Jake Bailey (jakebailey) Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We already do worse in other places, just look at getCodeActionsToFixClassIncorrectlyImplementsInterface which creates one for every single type node in an implements, so surely it's not a big deal?

if err != nil {
return nil, err
}

fixer := &isolatedDeclarationsFixer{
sourceFile: fixContext.SourceFile,
Expand All @@ -184,12 +197,7 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker.

description := fn(fixer)
if description == "" {
return nil
}

// Add any symbols that need to be imported to existing import declarations
for _, sym := range fixer.symbolsToImport {
fixer.addSymbolToExistingImport(sym)
return nil, nil
}

changes := changeTracker.GetChanges()
Expand All @@ -201,29 +209,28 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker.
}

if len(fileChanges) == 0 {
return nil
return nil, nil
}

return &CodeAction{
Description: description,
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.
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 {
Expand Down Expand Up @@ -1196,12 +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 {
if f.importAdder != nil {
for _, symbol := range importableSymbols {
Comment thread
DanielRosenwasser marked this conversation as resolved.
f.importAdder.AddImportFromExportedSymbol(symbol, true /*isValidTypeOnlyUseSite*/)
}
} else if !f.allSymbolsAccessibleInScope(importableSymbols, enclosingDecl) {
return nil
}
typeNode = referenceTypeNode
f.symbolsToImport = append(f.symbolsToImport, importableSymbols...)
}
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.
Expand Down Expand Up @@ -1373,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
}
}
Loading