Microsoft/TypeScript
1
0
Fork 0
mirror of https://github.com/microsoft/TypeScript.git synced 2025-11-18 17:21:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Use import().T for import completions/fixes of pure types in JS files (#25852)

Browse Source 
* Use `import().T` for import completions/fixes of pure types in JS files

* Don't call tryUseExistingNamespaceImport if position undefined
This commit is contained in:
Andy
2018-07-26 16:16:57 -07:00
committed by GitHub
parent d590d5bd0c
commit 0227997fa5
7 changed files with 183 additions and 43 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/services/codefixes/importFixes.ts
+69 -37
View File
@@ -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<SourceFile>,
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<SourceFile>): ReadonlyArray<SymbolExportInfo> {
@@ -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<SymbolExportInfo>,
symbolName: string,
symbolToken: Node | undefined,
position: number | undefined,
program: Program,
sourceFile: SourceFile,
host: LanguageServiceHost,
@@ -187,14 +203,14 @@ namespace ts.codefix {
): ReadonlyArray<ImportFix> {
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<FixAddToExistingImportInfo>, symbolName: string, symbolToken: Node | undefined, checker: TypeChecker): FixUseNamespaceImport | undefined {
function tryUseExistingNamespaceImport(existingImports: ReadonlyArray<FixAddToExistingImportInfo>, 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<FixAddToExistingImportInfo> {
return mapDefined<StringLiteralLike, FixAddToExistingImportInfo>(imports, moduleSpecifier => {
function getExistingImportDeclarations({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }: SymbolExportInfo, checker: TypeChecker, sourceFile: SourceFile): ReadonlyArray<FixAddToExistingImportInfo> {
// Can't use an es6 import for a type in JS.
return exportedSymbolIsTypeOnly && isSourceFileJavaScript(sourceFile) ? emptyArray : mapDefined<StringLiteralLike, FixAddToExistingImportInfo>(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<SymbolExportInfo>,
host: LanguageServiceHost,
preferences: UserPreferences,
): ReadonlyArray<FixAddNewImport> {
const choicesForEachExportingModule = flatMap<SymbolExportInfo, ReadonlyArray<FixAddNewImport>>(moduleSymbols, ({ moduleSymbol, importKind }) => {
): ReadonlyArray<FixAddNewImport | FixUseImportType> {
const isJs = isSourceFileJavaScript(sourceFile);
const choicesForEachExportingModule = flatMap<SymbolExportInfo, ReadonlyArray<FixAddNewImport | FixUseImportType>>(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<FixAddNewImport>(choicesForEachExportingModule.sort((a, b) => first(a).moduleSpecifier.length - first(b).moduleSpecifier.length));
return flatten<FixAddNewImport | FixUseImportType>(choicesForEachExportingModule.sort((a, b) => first(a).moduleSpecifier.length - first(b).moduleSpecifier.length));
}
function getFixesForAddImport(
@@ -268,11 +289,12 @@ namespace ts.codefix {
existingImports: ReadonlyArray<FixAddToExistingImportInfo>,
program: Program,
sourceFile: SourceFile,
position: number | undefined,
host: LanguageServiceHost,
preferences: UserPreferences,
): ReadonlyArray<FixAddNewImport> {
): ReadonlyArray<FixAddNewImport | FixUseImportType> {
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<SymbolExportInfo> = [{ moduleSymbol: symbol, importKind: getUmdImportKind(program.getCompilerOptions()) }];
const fixes = getFixForImport(exportInfos, symbolName, token, program, sourceFile, host, preferences);
const exportInfos: ReadonlyArray<SymbolExportInfo> = [{ 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 `<Foo/>`, 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<SymbolExportInfo>();
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 {
src/services/completions.ts
+3 -5
View File
@@ -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<SourceFile>,
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] };
}
src/services/textChanges.ts
+1 -1
View File
@@ -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);
}
src/services/utilities.ts
+8
View File
@@ -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);
tests/cases/fourslash/completionsImport_importType.ts
+60
View File
@@ -0,0 +1,60 @@
/// <reference path="fourslash.ts" />
// @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").} */`,
});
tests/cases/fourslash/importNameCodeFix_all_js.ts
+25
View File
@@ -0,0 +1,25 @@
/// <reference path='fourslash.ts' />
// @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;`,
});
tests/cases/fourslash/importNameCodeFix_importType.ts
+17
View File
@@ -0,0 +1,17 @@
/// <reference path='fourslash.ts' />
// @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;`]);