diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 9b18fe3637a..bd1a50ac5f9 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -27,6 +27,7 @@ namespace ts.codefix { // Namespace fixes don't conflict, so just build a list. const addToNamespace: FixUseNamespaceImport[] = []; + const importType: FixUseImportType[] = []; // Keys are import clause node IDs. const addToExisting = createMap<{ readonly importClause: ImportClause, defaultImport: string | undefined; readonly namedImports: string[] }>(); // Keys are module specifiers. @@ -42,6 +43,9 @@ namespace ts.codefix { case ImportFixKind.UseNamespace: addToNamespace.push(fix); break; + case ImportFixKind.ImportType: + importType.push(fix); + break; case ImportFixKind.AddToExisting: { const { importClause, importKind } = fix; const key = String(getNodeId(importClause)); @@ -86,13 +90,16 @@ namespace ts.codefix { }); return createCombinedCodeActions(textChanges.ChangeTracker.with(context, changes => { + const quotePreference = getQuotePreference(sourceFile, preferences); for (const fix of addToNamespace) { addNamespaceQualifier(changes, sourceFile, fix); } + for (const fix of importType) { + addImportType(changes, sourceFile, fix, quotePreference); + } addToExisting.forEach(({ importClause, defaultImport, namedImports }) => { doAddExistingFix(changes, sourceFile, importClause, defaultImport, namedImports); }); - const quotePreference = getQuotePreference(sourceFile, preferences); newImports.forEach((imports, moduleSpecifier) => { addNewImports(changes, sourceFile, moduleSpecifier, quotePreference, imports); }); @@ -101,12 +108,17 @@ namespace ts.codefix { }); // Sorted with the preferred fix coming first. - const enum ImportFixKind { UseNamespace, AddToExisting, AddNew } - type ImportFix = FixUseNamespaceImport | FixAddToExistingImport | FixAddNewImport; + const enum ImportFixKind { UseNamespace, ImportType, AddToExisting, AddNew } + type ImportFix = FixUseNamespaceImport | FixUseImportType | FixAddToExistingImport | FixAddNewImport; interface FixUseNamespaceImport { readonly kind: ImportFixKind.UseNamespace; readonly namespacePrefix: string; - readonly symbolToken: Identifier; + readonly position: number; + } + interface FixUseImportType { + readonly kind: ImportFixKind.ImportType; + readonly moduleSpecifier: string; + readonly position: number; } interface FixAddToExistingImport { readonly kind: ImportFixKind.AddToExisting; @@ -130,6 +142,8 @@ namespace ts.codefix { interface SymbolExportInfo { readonly moduleSymbol: Symbol; readonly importKind: ImportKind; + /** If true, can't use an es6 import from a js file. */ + readonly exportedSymbolIsTypeOnly: boolean; } /** Information needed to augment an existing import declaration. */ @@ -145,17 +159,15 @@ namespace ts.codefix { symbolName: string, host: LanguageServiceHost, program: Program, - checker: TypeChecker, - allSourceFiles: ReadonlyArray, formatContext: formatting.FormatContext, - symbolToken: Node | undefined, + position: number, preferences: UserPreferences, ): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } { - const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, checker, allSourceFiles); + const exportInfos = getAllReExportingModules(exportedSymbol, moduleSymbol, symbolName, sourceFile, program.getTypeChecker(), program.getSourceFiles()); Debug.assert(exportInfos.some(info => info.moduleSymbol === moduleSymbol)); // We sort the best codefixes first, so taking `first` is best for completions. - const moduleSpecifier = first(getNewImportInfos(program, sourceFile, exportInfos, host, preferences)).moduleSpecifier; - const fix = first(getFixForImport(exportInfos, symbolName, symbolToken, program, sourceFile, host, preferences)); + const moduleSpecifier = first(getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences)).moduleSpecifier; + const fix = first(getFixForImport(exportInfos, symbolName, position, program, sourceFile, host, preferences)); return { moduleSpecifier, codeAction: codeActionForFix({ host, formatContext }, sourceFile, symbolName, fix, getQuotePreference(sourceFile, preferences)) }; } function getAllReExportingModules(exportedSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, sourceFile: SourceFile, checker: TypeChecker, allSourceFiles: ReadonlyArray): ReadonlyArray { @@ -169,17 +181,21 @@ namespace ts.codefix { for (const exported of checker.getExportsOfModule(moduleSymbol)) { if ((exported.escapedName === InternalSymbolName.Default || exported.name === symbolName) && skipAlias(exported, checker) === exportedSymbol) { const isDefaultExport = checker.tryGetMemberInModuleExports(InternalSymbolName.Default, moduleSymbol) === exported; - result.push({ moduleSymbol, importKind: isDefaultExport ? ImportKind.Default : ImportKind.Named }); + result.push({ moduleSymbol, importKind: isDefaultExport ? ImportKind.Default : ImportKind.Named, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exported) }); } } }); return result; } + function isTypeOnlySymbol(s: Symbol): boolean { + return !(s.flags & SymbolFlags.Value); + } + function getFixForImport( exportInfos: ReadonlyArray, symbolName: string, - symbolToken: Node | undefined, + position: number | undefined, program: Program, sourceFile: SourceFile, host: LanguageServiceHost, @@ -187,14 +203,14 @@ namespace ts.codefix { ): ReadonlyArray { const checker = program.getTypeChecker(); const existingImports = flatMap(exportInfos, info => getExistingImportDeclarations(info, checker, sourceFile)); - const useNamespace = tryUseExistingNamespaceImport(existingImports, symbolName, symbolToken, checker); + const useNamespace = position === undefined ? undefined : tryUseExistingNamespaceImport(existingImports, symbolName, position, checker); const addToExisting = tryAddToExistingImport(existingImports); // Don't bother providing an action to add a new import if we can add to an existing one. - const addImport = addToExisting ? [addToExisting] : getFixesForAddImport(exportInfos, existingImports, program, sourceFile, host, preferences); + const addImport = addToExisting ? [addToExisting] : getFixesForAddImport(exportInfos, existingImports, program, sourceFile, position, host, preferences); return [...(useNamespace ? [useNamespace] : emptyArray), ...addImport]; } - function tryUseExistingNamespaceImport(existingImports: ReadonlyArray, symbolName: string, symbolToken: Node | undefined, checker: TypeChecker): FixUseNamespaceImport | undefined { + function tryUseExistingNamespaceImport(existingImports: ReadonlyArray, symbolName: string, position: number, checker: TypeChecker): FixUseNamespaceImport | undefined { // It is possible that multiple import statements with the same specifier exist in the file. // e.g. // @@ -207,12 +223,12 @@ namespace ts.codefix { // 1. change "member3" to "ns.member3" // 2. add "member3" to the second import statement's import list // and it is up to the user to decide which one fits best. - return !symbolToken || !isIdentifier(symbolToken) ? undefined : firstDefined(existingImports, ({ declaration }): FixUseNamespaceImport | undefined => { + return firstDefined(existingImports, ({ declaration }): FixUseNamespaceImport | undefined => { const namespace = getNamespaceImportName(declaration); if (namespace) { const moduleSymbol = checker.getAliasedSymbol(checker.getSymbolAtLocation(namespace)!); if (moduleSymbol && moduleSymbol.exports!.has(escapeLeadingUnderscores(symbolName))) { - return { kind: ImportFixKind.UseNamespace, namespacePrefix: namespace.text, symbolToken }; + return { kind: ImportFixKind.UseNamespace, namespacePrefix: namespace.text, position }; } } }); @@ -240,8 +256,9 @@ namespace ts.codefix { } } - function getExistingImportDeclarations({ moduleSymbol, importKind }: SymbolExportInfo, checker: TypeChecker, { imports }: SourceFile): ReadonlyArray { - return mapDefined(imports, moduleSpecifier => { + function getExistingImportDeclarations({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }: SymbolExportInfo, checker: TypeChecker, sourceFile: SourceFile): ReadonlyArray { + // Can't use an es6 import for a type in JS. + return exportedSymbolIsTypeOnly && isSourceFileJavaScript(sourceFile) ? emptyArray : mapDefined(sourceFile.imports, moduleSpecifier => { const i = importFromModuleSpecifier(moduleSpecifier); return (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) && checker.getSymbolAtLocation(moduleSpecifier) === moduleSymbol ? { declaration: i, importKind } : undefined; @@ -251,16 +268,20 @@ namespace ts.codefix { function getNewImportInfos( program: Program, sourceFile: SourceFile, + position: number | undefined, moduleSymbols: ReadonlyArray, host: LanguageServiceHost, preferences: UserPreferences, - ): ReadonlyArray { - const choicesForEachExportingModule = flatMap>(moduleSymbols, ({ moduleSymbol, importKind }) => { + ): ReadonlyArray { + const isJs = isSourceFileJavaScript(sourceFile); + const choicesForEachExportingModule = flatMap>(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) => { const modulePathsGroups = moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences); - return modulePathsGroups.map(group => group.map((moduleSpecifier): FixAddNewImport => ({ kind: ImportFixKind.AddNew, moduleSpecifier, importKind }))); + return modulePathsGroups.map(group => group.map((moduleSpecifier): FixAddNewImport | FixUseImportType => + // `position` should only be undefined at a missing jsx namespace, in which case we shouldn't be looking for pure types. + exportedSymbolIsTypeOnly && isJs ? { kind: ImportFixKind.ImportType, moduleSpecifier, position: Debug.assertDefined(position) } : { kind: ImportFixKind.AddNew, moduleSpecifier, importKind })); }); // Sort to keep the shortest paths first, but keep [relativePath, importRelativeToBaseUrl] groups together - return flatten(choicesForEachExportingModule.sort((a, b) => first(a).moduleSpecifier.length - first(b).moduleSpecifier.length)); + return flatten(choicesForEachExportingModule.sort((a, b) => first(a).moduleSpecifier.length - first(b).moduleSpecifier.length)); } function getFixesForAddImport( @@ -268,11 +289,12 @@ namespace ts.codefix { existingImports: ReadonlyArray, program: Program, sourceFile: SourceFile, + position: number | undefined, host: LanguageServiceHost, preferences: UserPreferences, - ): ReadonlyArray { + ): ReadonlyArray { const existingDeclaration = firstDefined(existingImports, newImportInfoFromExistingSpecifier); - return existingDeclaration ? [existingDeclaration] : getNewImportInfos(program, sourceFile, exportInfos, host, preferences); + return existingDeclaration ? [existingDeclaration] : getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences); } function newImportInfoFromExistingSpecifier({ declaration, importKind }: FixAddToExistingImportInfo): FixAddNewImport | undefined { @@ -289,7 +311,7 @@ namespace ts.codefix { const symbolToken = getTokenAtPosition(context.sourceFile, pos); const info = errorCode === Diagnostics._0_refers_to_a_UMD_global_but_the_current_file_is_a_module_Consider_adding_an_import_instead.code ? getFixesInfoForUMDImport(context, symbolToken) - : getFixesInfoForNonUMDImport(context, symbolToken); + : isIdentifier(symbolToken) ? getFixesInfoForNonUMDImport(context, symbolToken) : undefined; return info && { ...info, fixes: sort(info.fixes, (a, b) => a.kind - b.kind) }; } @@ -299,8 +321,8 @@ namespace ts.codefix { if (!umdSymbol) return undefined; const symbol = checker.getAliasedSymbol(umdSymbol); const symbolName = umdSymbol.name; - const exportInfos: ReadonlyArray = [{ moduleSymbol: symbol, importKind: getUmdImportKind(program.getCompilerOptions()) }]; - const fixes = getFixForImport(exportInfos, symbolName, token, program, sourceFile, host, preferences); + const exportInfos: ReadonlyArray = [{ moduleSymbol: symbol, importKind: getUmdImportKind(program.getCompilerOptions()), exportedSymbolIsTypeOnly: false }]; + const fixes = getFixForImport(exportInfos, symbolName, isIdentifier(token) ? token.getStart(sourceFile) : undefined, program, sourceFile, host, preferences); return { fixes, symbolName }; } function getUmdSymbol(token: Node, checker: TypeChecker): Symbol | undefined { @@ -339,21 +361,19 @@ namespace ts.codefix { } } - function getFixesInfoForNonUMDImport({ sourceFile, program, cancellationToken, host, preferences }: CodeFixContextBase, symbolToken: Node): FixesInfo | undefined { - // This will always be an Identifier, since the diagnostics we fix only fail on identifiers. + function getFixesInfoForNonUMDImport({ sourceFile, program, cancellationToken, host, preferences }: CodeFixContextBase, symbolToken: Identifier): FixesInfo | undefined { const checker = program.getTypeChecker(); // If we're at ``, we must check if `Foo` is already in scope, and if so, get an import for `React` instead. const symbolName = isJsxOpeningLikeElement(symbolToken.parent) && symbolToken.parent.tagName === symbolToken - && (!isIdentifier(symbolToken) || isIntrinsicJsxName(symbolToken.text) || checker.resolveName(symbolToken.text, symbolToken, SymbolFlags.All, /*excludeGlobals*/ false)) + && (isIntrinsicJsxName(symbolToken.text) || checker.resolveName(symbolToken.text, symbolToken, SymbolFlags.All, /*excludeGlobals*/ false)) ? checker.getJsxNamespace() - : isIdentifier(symbolToken) ? symbolToken.text : undefined; - if (!symbolName) return undefined; + : symbolToken.text; // "default" is a keyword and not a legal identifier for the import, so we don't expect it here Debug.assert(symbolName !== InternalSymbolName.Default); const fixes = arrayFrom(flatMapIterator(getExportInfos(symbolName, getMeaningFromLocation(symbolToken), cancellationToken, sourceFile, checker, program).entries(), ([_, exportInfos]) => - getFixForImport(exportInfos, symbolName, symbolToken, program, sourceFile, host, preferences))); + getFixForImport(exportInfos, symbolName, symbolToken.getStart(sourceFile), program, sourceFile, host, preferences))); return { fixes, symbolName }; } @@ -370,7 +390,7 @@ namespace ts.codefix { // Maps symbol id to info for modules providing that symbol (original export + re-exports). const originalSymbolToExportInfos = createMultiMap(); function addSymbol(moduleSymbol: Symbol, exportedSymbol: Symbol, importKind: ImportKind): void { - originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { moduleSymbol, importKind }); + originalSymbolToExportInfos.add(getUniqueSymbolId(exportedSymbol, checker).toString(), { moduleSymbol, importKind, exportedSymbolIsTypeOnly: isTypeOnlySymbol(exportedSymbol) }); } forEachExternalModuleToImportFrom(checker, sourceFile, program.getSourceFiles(), moduleSymbol => { cancellationToken.throwIfCancellationRequested(); @@ -438,6 +458,9 @@ namespace ts.codefix { case ImportFixKind.UseNamespace: addNamespaceQualifier(changes, sourceFile, fix); return [Diagnostics.Change_0_to_1, symbolName, `${fix.namespacePrefix}.${symbolName}`]; + case ImportFixKind.ImportType: + addImportType(changes, sourceFile, fix, quotePreference); + return [Diagnostics.Change_0_to_1, symbolName, getImportTypePrefix(fix.moduleSpecifier, quotePreference) + symbolName]; case ImportFixKind.AddToExisting: { const { importClause, importKind } = fix; doAddExistingFix(changes, sourceFile, importClause, importKind === ImportKind.Default ? symbolName : undefined, importKind === ImportKind.Named ? [symbolName] : emptyArray); @@ -483,8 +506,17 @@ namespace ts.codefix { } } - function addNamespaceQualifier(changes: textChanges.ChangeTracker, sourceFile: SourceFile, { namespacePrefix, symbolToken }: FixUseNamespaceImport): void { - changes.replaceNode(sourceFile, symbolToken, createPropertyAccess(createIdentifier(namespacePrefix), createIdentifier(symbolToken.text))); + function addNamespaceQualifier(changes: textChanges.ChangeTracker, sourceFile: SourceFile, { namespacePrefix, position }: FixUseNamespaceImport): void { + changes.insertText(sourceFile, position, namespacePrefix + "."); + } + + function addImportType(changes: textChanges.ChangeTracker, sourceFile: SourceFile, { moduleSpecifier, position }: FixUseImportType, quotePreference: QuotePreference): void { + changes.insertText(sourceFile, position, getImportTypePrefix(moduleSpecifier, quotePreference)); + } + + function getImportTypePrefix(moduleSpecifier: string, quotePreference: QuotePreference): string { + const quote = getQuoteFromPreference(quotePreference); + return `import(${quote}${moduleSpecifier}${quote}).`; } interface ImportsCollection { diff --git a/src/services/completions.ts b/src/services/completions.ts index 2c0bb0a8775..232514722cb 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -592,7 +592,7 @@ namespace ts.Completions { } case "symbol": { const { symbol, location, symbolToOriginInfoMap, previousToken } = symbolCompletion; - const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, previousToken, formatContext, program.getSourceFiles(), preferences); + const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(symbolToOriginInfoMap, symbol, program, typeChecker, host, compilerOptions, sourceFile, position, previousToken, formatContext, preferences); return createCompletionDetailsForSymbol(symbol, typeChecker, sourceFile, location!, cancellationToken, codeActions, sourceDisplay); // TODO: GH#18217 } case "literal": { @@ -652,9 +652,9 @@ namespace ts.Completions { host: LanguageServiceHost, compilerOptions: CompilerOptions, sourceFile: SourceFile, + position: number, previousToken: Node | undefined, formatContext: formatting.FormatContext, - allSourceFiles: ReadonlyArray, preferences: UserPreferences, ): CodeActionsAndSourceDisplay { const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; @@ -671,10 +671,8 @@ namespace ts.Completions { getSymbolName(symbol, symbolOriginInfo, compilerOptions.target!), host, program, - checker, - allSourceFiles, formatContext, - previousToken, + previousToken && isIdentifier(previousToken) ? previousToken.getStart(sourceFile) : position, preferences); return { sourceDisplay: [textPart(moduleSpecifier)], codeActions: [codeAction] }; } diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 959f190d474..670e1dc99a9 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -333,7 +333,7 @@ namespace ts.textChanges { this.changes.push({ kind: ChangeKind.Text, sourceFile, range, text }); } - private insertText(sourceFile: SourceFile, pos: number, text: string): void { + public insertText(sourceFile: SourceFile, pos: number, text: string): void { this.replaceRangeWithText(sourceFile, createTextRange(pos), text); } diff --git a/src/services/utilities.ts b/src/services/utilities.ts index 6f9ffe25b12..283f8e18e0b 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -1291,6 +1291,14 @@ namespace ts { } } + export function getQuoteFromPreference(qp: QuotePreference): string { + switch (qp) { + case QuotePreference.Single: return "'"; + case QuotePreference.Double: return '"'; + default: return Debug.assertNever(qp); + } + } + export function symbolNameNoDefault(symbol: Symbol): string | undefined { const escaped = symbolEscapedNameNoDefault(symbol); return escaped === undefined ? undefined : unescapeLeadingUnderscores(escaped); diff --git a/tests/cases/fourslash/completionsImport_importType.ts b/tests/cases/fourslash/completionsImport_importType.ts new file mode 100644 index 00000000000..b1b69a9a8d0 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_importType.ts @@ -0,0 +1,60 @@ +/// + +// @allowJs: true + +// @Filename: /a.js +////export const x = 0; +////export class C {} +/////** @typedef {number} T */ + +// @Filename: /b.js +/////** @type {/*0*/} */ +/////** @type {/*1*/} */ + +verify.completions({ + marker: ["0", "1"], + includes: [ + { + name: "C", + source: "/a", + sourceDisplay: "./a", + text: "class C", + hasAction: true, + }, + { + name: "T", + source: "/a", + sourceDisplay: "./a", + text: "type T = number", + hasAction: true, + }, + ], + excludes: "x", + preferences: { + includeCompletionsForModuleExports: true, + }, +}); + +// Something with a value-side will get a normal import. +verify.applyCodeActionFromCompletion("0", { + name: "C", + source: "/a", + description: `Import 'C' from module "./a"`, + newFileContent: +`import { C } from "./a"; + +/** @type {} */ +/** @type {} */`, +}); + +// A pure type will get `import().T` +verify.applyCodeActionFromCompletion("1", { + name: "T", + source: "/a", + description: `Change 'T' to 'import("./a").T'`, + newFileContent: +`import { C } from "./a"; + +/** @type {} */ +/** @type {import("./a").} */`, +}); diff --git a/tests/cases/fourslash/importNameCodeFix_all_js.ts b/tests/cases/fourslash/importNameCodeFix_all_js.ts new file mode 100644 index 00000000000..cf4921c6911 --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_all_js.ts @@ -0,0 +1,25 @@ +/// + +// @allowJs: true +// @checkJs: true + +// @Filename: /a.js +////export class C {} +/////** @typedef {number} T */ + +// @Filename: /b.js +////C; +/////** @type {T} */ +////const x = 0; + +goTo.file("/b.js"); +verify.codeFixAll({ + fixId: "fixMissingImport", + fixAllDescription: "Add all missing imports", + newFileContent: +`import { C } from "./a"; + +C; +/** @type {import("./a").T} */ +const x = 0;`, +}); diff --git a/tests/cases/fourslash/importNameCodeFix_importType.ts b/tests/cases/fourslash/importNameCodeFix_importType.ts new file mode 100644 index 00000000000..4e2f869dd0d --- /dev/null +++ b/tests/cases/fourslash/importNameCodeFix_importType.ts @@ -0,0 +1,17 @@ +/// + +// @allowJs: true +// @checkJs: true + +// @Filename: /a.js +////export {}; +/////** @typedef {number} T */ + +// @Filename: /b.js +/////** @type {T} */ +////const x = 0; + +goTo.file("/b.js"); +verify.importFixAtPosition([ +`/** @type {import("./a").T} */ +const x = 0;`]);