Skip to content

Commit ea005f3

Browse files
CopilotDanielRosenwasserandrewbranch
authored
Fix assertion violation in auto-imports for JSX tag when React is type-imported (#3650)
Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: DanielRosenwasser <972891+DanielRosenwasser@users.noreply.github.com> Co-authored-by: andrewbranch <3277153+andrewbranch@users.noreply.github.com>
1 parent 51a1064 commit ea005f3

2 files changed

Lines changed: 141 additions & 19 deletions

File tree

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
package fourslash_test
2+
3+
import (
4+
"testing"
5+
6+
"github.com/microsoft/typescript-go/internal/fourslash"
7+
"github.com/microsoft/typescript-go/internal/testutil"
8+
)
9+
10+
// Test that auto-imports for JSX tags don't crash when React is type-imported.
11+
// When both the JSX namespace (React) and the component need to be imported,
12+
// getSymbolNamesToImport returns multiple names and the type-only promotion
13+
// path should handle this gracefully instead of panicking.
14+
func TestCodeFixPromoteTypeOnlyImportJsxTag(t *testing.T) {
15+
t.Parallel()
16+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
17+
const content = `// @module: preserve
18+
// @verbatimModuleSyntax: true
19+
// @jsx: react
20+
// @Filename: /react.ts
21+
const React: any = {};
22+
export default React;
23+
// @Filename: /bar.tsx
24+
import type React from "./react";
25+
26+
<Foo/**/ />;`
27+
f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
28+
defer done()
29+
f.GoToMarker(t, "")
30+
// The fix should promote the type-only import of React to a regular import.
31+
// The "Cannot find name 'Foo'" error does not produce an auto-import for
32+
// React since it's already imported (as type-only, handled by promotion).
33+
f.VerifyImportFixAtPosition(t, []string{
34+
`import React from "./react";
35+
36+
<Foo />;`,
37+
}, nil /*preferences*/)
38+
}
39+
40+
// Test edge case where both the component name (Foo) and the JSX namespace (React)
41+
// are type-only imported. Each diagnostic is matched to its symbol via the error
42+
// message, so each produces only its own promotion fix (no duplicates).
43+
func TestCodeFixPromoteTypeOnlyImportJsxTagBothTypeOnly(t *testing.T) {
44+
t.Parallel()
45+
defer testutil.RecoverAndFail(t, "Panic on fourslash test")
46+
const content = `// @module: preserve
47+
// @verbatimModuleSyntax: true
48+
// @jsx: react
49+
// @Filename: /react.ts
50+
const React: any = {};
51+
export default React;
52+
// @Filename: /foo.ts
53+
export function Foo() { return null; }
54+
// @Filename: /bar.tsx
55+
import type React from "./react";
56+
import type { Foo } from "./foo";
57+
58+
<Foo/**/ />;`
59+
f, done := fourslash.NewFourslash(t, nil /*capabilities*/, content)
60+
defer done()
61+
f.GoToMarker(t, "")
62+
// Both Foo and React are type-only imported. The error message string
63+
// matching disambiguates which diagnostic is about which symbol, so each
64+
// diagnostic produces only its own promotion fix (no duplicates).
65+
f.VerifyImportFixAtPosition(t, []string{
66+
`import type React from "./react";
67+
import { Foo } from "./foo";
68+
69+
<Foo />;`,
70+
`import React from "./react";
71+
import type { Foo } from "./foo";
72+
73+
<Foo />;`,
74+
}, nil /*preferences*/)
75+
}

internal/ls/codeactions_importfixes.go

Lines changed: 66 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package ls
33
import (
44
"context"
55
"slices"
6+
"strings"
67

78
"github.com/microsoft/typescript-go/internal/ast"
89
"github.com/microsoft/typescript-go/internal/astnav"
@@ -183,20 +184,42 @@ func getFixInfos(ctx context.Context, fixContext *CodeFixContext, errorCode int3
183184
} else if !ast.IsIdentifier(symbolToken) {
184185
return nil, nil
185186
} else if errorCode == diagnostics.X_0_cannot_be_used_as_a_value_because_it_was_imported_using_import_type.Code() {
186-
// Handle type-only import promotion
187187
ch, done := fixContext.Program.GetTypeChecker(ctx)
188188
defer done()
189189
compilerOptions := fixContext.Program.Options()
190190
symbolNames := getSymbolNamesToImport(fixContext.SourceFile, ch, symbolToken, compilerOptions)
191-
if len(symbolNames) != 1 {
192-
panic("Expected exactly one symbol name for type-only import promotion")
191+
192+
var allTypeOnlyFixes []*fixInfo
193+
for _, sn := range symbolNames {
194+
if !sn.isTypeOnly {
195+
continue
196+
}
197+
fix := getTypeOnlyPromotionFix(ctx, fixContext.SourceFile, symbolToken, sn.name, fixContext.Program)
198+
if fix != nil {
199+
allTypeOnlyFixes = append(allTypeOnlyFixes, &fixInfo{fix: fix, symbolName: sn.name, errorIdentifierText: symbolToken.Text()})
200+
}
193201
}
194-
symbolName := symbolNames[0]
195-
fix := getTypeOnlyPromotionFix(ctx, fixContext.SourceFile, symbolToken, symbolName, fixContext.Program)
196-
if fix != nil {
197-
return []*fixInfo{{fix: fix, symbolName: symbolName, errorIdentifierText: symbolToken.Text()}}, nil
202+
203+
// For JSX opening tags, there can be separate type-only errors for both the tag name
204+
// identifier and the JSX namespace identifier. When both produce valid fixes, we
205+
// disambiguate using the diagnostic message, which quotes the symbol name in single
206+
// quotes (e.g., "'React' cannot be used as a value..."). If filtering yields nothing
207+
// (e.g., due to localization), fall back to returning all candidates.
208+
diagnosticMessage := ""
209+
if fixContext.Diagnostic != nil {
210+
diagnosticMessage = fixContext.Diagnostic.Message
198211
}
199-
return nil, nil
212+
if len(allTypeOnlyFixes) > 1 && diagnosticMessage != "" {
213+
for _, fi := range allTypeOnlyFixes {
214+
if strings.Contains(diagnosticMessage, "'"+fi.symbolName+"'") {
215+
info = append(info, fi)
216+
}
217+
}
218+
}
219+
if len(info) == 0 {
220+
info = allTypeOnlyFixes
221+
}
222+
return info, nil
200223
} else {
201224
var err error
202225
view, err = fixContext.LS.getPreparedAutoImportView(fixContext.SourceFile)
@@ -289,7 +312,13 @@ func getFixesInfoForNonUMDImport(ctx context.Context, fixContext *CodeFixContext
289312
// Compute usage position for JSDoc import type fixes
290313
usagePosition := fixContext.LS.converters.PositionToLineAndCharacter(fixContext.SourceFile, core.TextPos(scanner.GetTokenPosOfNode(symbolToken, fixContext.SourceFile, false)))
291314

292-
for _, symbolName := range symbolNames {
315+
for _, sn := range symbolNames {
316+
// Type-only imports are handled by the promotion code path, not the auto-import path.
317+
if sn.isTypeOnly {
318+
continue
319+
}
320+
321+
symbolName := sn.name
293322
// "default" is a keyword and not a legal identifier for the import
294323
if symbolName == "default" {
295324
continue
@@ -310,8 +339,9 @@ func getFixesInfoForNonUMDImport(ctx context.Context, fixContext *CodeFixContext
310339
fixes := view.GetFixes(ctx, export, isJSXTagName, isValidTypeOnlyUseSite, &usagePosition)
311340
for _, fix := range fixes {
312341
allInfo = append(allInfo, &fixInfo{
313-
fix: fix,
314-
symbolName: symbolName,
342+
fix: fix,
343+
symbolName: symbolName,
344+
isJsxNamespaceFix: symbolName != symbolToken.Text(),
315345
})
316346
}
317347
}
@@ -344,22 +374,40 @@ func getTypeOnlyPromotionFix(ctx context.Context, sourceFile *ast.SourceFile, sy
344374
}
345375
}
346376

347-
func getSymbolNamesToImport(sourceFile *ast.SourceFile, ch *checker.Checker, symbolToken *ast.Node, compilerOptions *core.CompilerOptions) []string {
377+
type symbolNameInfo struct {
378+
name string
379+
isTypeOnly bool // whether the symbol currently resolves to a type-only import
380+
}
381+
382+
func getSymbolNamesToImport(sourceFile *ast.SourceFile, ch *checker.Checker, symbolToken *ast.Node, compilerOptions *core.CompilerOptions) []symbolNameInfo {
348383
parent := symbolToken.Parent
349384
if (ast.IsJsxOpeningLikeElement(parent) || ast.IsJsxClosingElement(parent)) &&
350385
parent.TagName() == symbolToken &&
351386
jsxModeNeedsExplicitImport(compilerOptions.Jsx) {
352387
jsxNamespace := ch.GetJsxNamespace(sourceFile.AsNode())
353388
if needsJsxNamespaceFix(jsxNamespace, symbolToken, ch) {
354-
needsComponentNameFix := !scanner.IsIntrinsicJsxName(symbolToken.Text()) &&
355-
ch.ResolveName(symbolToken.Text(), symbolToken, ast.SymbolFlagsValue, false /* excludeGlobals */) == nil
356-
if needsComponentNameFix {
357-
return []string{symbolToken.Text(), jsxNamespace}
389+
var result []symbolNameInfo
390+
if !scanner.IsIntrinsicJsxName(symbolToken.Text()) {
391+
compSymbol := ch.ResolveName(symbolToken.Text(), symbolToken, ast.SymbolFlagsValue, false /* excludeGlobals */)
392+
if compSymbol == nil {
393+
result = append(result, symbolNameInfo{name: symbolToken.Text()})
394+
} else if ch.GetTypeOnlyAliasDeclaration(compSymbol) != nil {
395+
result = append(result, symbolNameInfo{name: symbolToken.Text(), isTypeOnly: true})
396+
}
358397
}
359-
return []string{jsxNamespace}
398+
nsIsTypeOnly := false
399+
if nsSymbol := ch.ResolveName(jsxNamespace, symbolToken, ast.SymbolFlagsValue, true /* excludeGlobals */); nsSymbol != nil {
400+
nsIsTypeOnly = ch.GetTypeOnlyAliasDeclaration(nsSymbol) != nil
401+
}
402+
result = append(result, symbolNameInfo{name: jsxNamespace, isTypeOnly: nsIsTypeOnly})
403+
return result
360404
}
361405
}
362-
return []string{symbolToken.Text()}
406+
tokenIsTypeOnly := false
407+
if sym := ch.ResolveName(symbolToken.Text(), symbolToken, ast.SymbolFlagsValue, true /* excludeGlobals */); sym != nil {
408+
tokenIsTypeOnly = ch.GetTypeOnlyAliasDeclaration(sym) != nil
409+
}
410+
return []symbolNameInfo{{name: symbolToken.Text(), isTypeOnly: tokenIsTypeOnly}}
363411
}
364412

365413
func needsJsxNamespaceFix(jsxNamespace string, symbolToken *ast.Node, ch *checker.Checker) bool {
@@ -370,7 +418,6 @@ func needsJsxNamespaceFix(jsxNamespace string, symbolToken *ast.Node, ch *checke
370418
if namespaceSymbol == nil {
371419
return true
372420
}
373-
// Check if all declarations are type-only
374421
if slices.ContainsFunc(namespaceSymbol.Declarations, ast.IsTypeOnlyImportOrExportDeclaration) {
375422
return (namespaceSymbol.Flags & ast.SymbolFlagsValue) == 0
376423
}

0 commit comments

Comments
 (0)