diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index a35d7a59d048c..fd5c6b5fc81d7 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -3692,9 +3692,12 @@ export function isInExpressionContext(node: Node): boolean { case SyntaxKind.ShorthandPropertyAssignment: // TODO(jakebailey): it's possible that node could be the name, too return (parent as ShorthandPropertyAssignment).objectAssignmentInitializer === node; - case SyntaxKind.SatisfiesExpression: - return node === (parent as SatisfiesExpression).expression; - default: + case SyntaxKind.SatisfiesExpression: + return node === (parent as SatisfiesExpression).expression; + case SyntaxKind.ElementAccessExpression: + const elementAccess = parent as ElementAccessExpression; + return node === elementAccess.expression || node === elementAccess.argumentExpression; + default: return isExpressionNode(parent); } } @@ -10502,14 +10505,14 @@ export function isValidBigIntString(s: string, roundTripOnly: boolean): boolean && (!roundTripOnly || s === pseudoBigIntToString({ negative, base10Value: parsePseudoBigInt(scanner.getTokenValue()) })); } -/** @internal */ -export function isValidTypeOnlyAliasUseSite(useSite: Node): boolean { - return !!(useSite.flags & NodeFlags.Ambient) - || isInJSDoc(useSite) - || isPartOfTypeQuery(useSite) - || isIdentifierInNonEmittingHeritageClause(useSite) - || isPartOfPossiblyValidTypeOrAbstractComputedPropertyName(useSite) - || !(isExpressionNode(useSite) || isShorthandPropertyNameUseSite(useSite)); +/** @internal */ +export function isValidTypeOnlyAliasUseSite(useSite: Node): boolean { + return !!(useSite.flags & NodeFlags.Ambient) + || isInJSDoc(useSite) + || isPartOfTypeQuery(useSite) + || isIdentifierInNonEmittingHeritageClause(useSite) + || isPartOfPossiblyValidTypeOrAbstractComputedPropertyName(useSite) + || !(isExpressionNode(useSite) || isShorthandPropertyNameUseSite(useSite) || isInExpressionContext(useSite)); } function isShorthandPropertyNameUseSite(useSite: Node) { diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 6034c9dfc3de1..9363f4281b4a1 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -1073,29 +1073,29 @@ function getNamespaceLikeImportText(declaration: AnyImportOrRequire) { } } -function getAddAsTypeOnly( - isValidTypeOnlyUseSite: boolean, - isForNewImportDeclaration: boolean, - symbol: Symbol | undefined, - targetFlags: SymbolFlags, - checker: TypeChecker, - compilerOptions: CompilerOptions, -) { - if (!isValidTypeOnlyUseSite) { - // Can't use a type-only import if the usage is an emitting position - return AddAsTypeOnly.NotAllowed; - } - if ( - symbol && - compilerOptions.verbatimModuleSyntax && - (!(targetFlags & SymbolFlags.Value) || !!checker.getTypeOnlyAliasDeclaration(symbol)) - ) { - // A type-only import is required for this symbol if under these settings if the symbol will - // be erased, which will happen if the target symbol is purely a type or if it was exported/imported - // as type-only already somewhere between this import and the target. - return AddAsTypeOnly.Required; - } - return AddAsTypeOnly.Allowed; +function getAddAsTypeOnly( + isValidTypeOnlyUseSite: boolean, + isForNewImportDeclaration: boolean, + symbol: Symbol | undefined, + targetFlags: SymbolFlags, + checker: TypeChecker, + compilerOptions: CompilerOptions, +) { + if (!isValidTypeOnlyUseSite) { + // Can't use a type-only import if the usage is an emitting position + return AddAsTypeOnly.NotAllowed; + } + if ( + symbol && + compilerOptions.verbatimModuleSyntax && + (!(targetFlags & SymbolFlags.Value) || !!checker.getTypeOnlyAliasDeclaration(symbol)) + ) { + // A type-only import is required for this symbol if under these settings if the symbol will + // be erased, which will happen if the target symbol is purely a type or if it was exported/imported + // as type-only already somewhere between this import and the target. + return AddAsTypeOnly.Required; + } + return AddAsTypeOnly.Allowed; } function tryAddToExistingImport(existingImports: readonly FixAddToExistingImportInfo[], isValidTypeOnlyUseSite: boolean, checker: TypeChecker, compilerOptions: CompilerOptions): FixAddToExistingImport | undefined { @@ -1104,14 +1104,18 @@ function tryAddToExistingImport(existingImports: readonly FixAddToExistingImport const fix = getAddToExistingImportFix(existingImport); if (!fix) continue; const isTypeOnly = isTypeOnlyImportDeclaration(fix.importClauseOrBindingPattern); - if ( - fix.addAsTypeOnly !== AddAsTypeOnly.NotAllowed && isTypeOnly || - fix.addAsTypeOnly === AddAsTypeOnly.NotAllowed && !isTypeOnly - ) { - // Give preference to putting types in existing type-only imports and avoiding conversions - // of import statements to/from type-only. - return fix; - } + if ( + fix.addAsTypeOnly !== AddAsTypeOnly.NotAllowed && isTypeOnly || + fix.addAsTypeOnly === AddAsTypeOnly.NotAllowed && !isTypeOnly + ) { + // Give preference to putting types in existing type-only imports and avoiding conversions + // of import statements to/from type-only. + return fix; + } + // Don't add to existing type-only imports if the symbol should be imported as a value + if (fix.addAsTypeOnly === AddAsTypeOnly.NotAllowed && isTypeOnly) { + continue; // Skip this import, don't even consider it as a fallback + } best ??= fix; } return best; diff --git a/tests/cases/fourslash/completionsSymbolAutoimportFromTypeOnlyImport.ts b/tests/cases/fourslash/completionsSymbolAutoimportFromTypeOnlyImport.ts new file mode 100644 index 0000000000000..694a7179a99ba --- /dev/null +++ b/tests/cases/fourslash/completionsSymbolAutoimportFromTypeOnlyImport.ts @@ -0,0 +1,52 @@ +/// + +// Test for issue #61894: Symbol auto-import bug with existing type-only imports +// Verifies that symbols used in element access expressions are imported as values, +// not added to existing type-only imports, when completing inside thing[...] + +// @noLib: true + +// @Filename: /globals.d.ts +////declare const Symbol: { for(key: string): symbol }; + +// @Filename: /exportsSymbol.ts +////export const SYM_FOO_BAR = Symbol.for("foo.bar"); +//// +////export interface ObjWithSym { +//// [SYM_FOO_BAR]: any; +////} + +// @Filename: /usesSymbol.ts +////import type { ObjWithSym } from "./exportsSymbol"; +//// +////export declare const thing: ObjWithSym; +//// +////function main() { +//// thing[|./**/|] +////} + +verify.completions({ + marker: "", + includes: [ + { name: "SYM_FOO_BAR", source: "/exportsSymbol", insertText: "[SYM_FOO_BAR]", replacementSpan: test.ranges()[0], hasAction: true }, + ], + preferences: { + includeInsertTextCompletions: true, + includeCompletionsForModuleExports: true, + }, +}); + +// Verify the fix - the symbol should be imported as a value, not as a type +verify.applyCodeActionFromCompletion("", { + name: "SYM_FOO_BAR", + source: "/exportsSymbol", + description: `Update import from "./exportsSymbol"`, + newFileContent: +`import { SYM_FOO_BAR, type ObjWithSym } from "./exportsSymbol"; + +export declare const thing: ObjWithSym; + +function main() { + thing. +}` +}); \ No newline at end of file diff --git a/tests/cases/fourslash/completionsSymbolAutoimportFromTypeOnlyImportControl.ts b/tests/cases/fourslash/completionsSymbolAutoimportFromTypeOnlyImportControl.ts new file mode 100644 index 0000000000000..7c28e3b97a891 --- /dev/null +++ b/tests/cases/fourslash/completionsSymbolAutoimportFromTypeOnlyImportControl.ts @@ -0,0 +1,48 @@ +/// + +// Control test for issue #61894: Symbol auto-import without existing type-only imports +// Verifies that symbols are correctly imported as values when there are no existing imports + +// @noLib: true + +// @Filename: /globals.d.ts +////declare const Symbol: { for(key: string): symbol }; + +// @Filename: /exportsSymbol.ts +////export const SYM_FOO_BAR = Symbol.for("foo.bar"); +//// +////export interface ObjWithSym { +//// [SYM_FOO_BAR]: any; +////} + +// @Filename: /usesSymbol.ts +////export declare const thing: import("./exportsSymbol").ObjWithSym; +//// +////function main() { +//// thing[|./**/|] +////} + +verify.completions({ + marker: "", + includes: [ + { name: "SYM_FOO_BAR", source: "/exportsSymbol", insertText: "[SYM_FOO_BAR]", replacementSpan: test.ranges()[0], hasAction: true }, + ], + preferences: { + includeInsertTextCompletions: true, + includeCompletionsForModuleExports: true, + }, +}); + +verify.applyCodeActionFromCompletion("", { + name: "SYM_FOO_BAR", + source: "/exportsSymbol", + description: `Add import from "./exportsSymbol"`, + newFileContent: +`import { SYM_FOO_BAR } from "./exportsSymbol"; + +export declare const thing: import("./exportsSymbol").ObjWithSym; + +function main() { + thing. +}` +}); \ No newline at end of file