diff --git a/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go b/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go new file mode 100644 index 00000000000..dba9cc9b6d1 --- /dev/null +++ b/internal/fourslash/tests/codeFixMissingTypeAnnotationOnExportsUsingTypeFromUnimportedFile_test.go @@ -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();`, + }) +} diff --git a/internal/ls/codeactions_fixmissingtypeannotation.go b/internal/ls/codeactions_fixmissingtypeannotation.go index 58fa3ebe215..edb396cd422 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,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 } @@ -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) + if err != nil { + return nil, err + } fixer := &isolatedDeclarationsFixer{ sourceFile: fixContext.SourceFile, @@ -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() @@ -201,7 +209,7 @@ func tryCodeAction(ctx context.Context, fixContext *CodeFixContext, ch *checker. } if len(fileChanges) == 0 { - return nil + return nil, nil } return &CodeAction{ @@ -209,21 +217,20 @@ 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. 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 { @@ -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 { + 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. @@ -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 - } -}