Type-only auto imports (#36412)

* WIP

* Promote existing type-only imports to regular if needed

* Add completions test adding to type-only import

* Update tests, revert whole-import-clause replacement codefix strategy to preserve import specifier formatting

* Revert unnecessary changes

* Delete unused function

* }
This commit is contained in:
Andrew Branch
2020-01-27 12:53:32 -08:00
committed by GitHub
parent 757e67041e
commit e1bce187a8
13 changed files with 106 additions and 84 deletions
+1 -6
View File
@@ -1879,12 +1879,7 @@ namespace ts {
}
function checkSymbolUsageInExpressionContext(symbol: Symbol, name: __String, useSite: Node) {
if (
!(useSite.flags & NodeFlags.Ambient) &&
!isPartOfTypeQuery(useSite) &&
!isPartOfPossiblyValidTypeOrAbstractComputedPropertyName(useSite) &&
isExpressionNode(useSite)
) {
if (!isValidTypeOnlyAliasUseSite(useSite)) {
const typeOnlyDeclaration = getTypeOnlyAliasDeclaration(symbol);
if (typeOnlyDeclaration) {
const message = typeOnlyDeclaration.kind === SyntaxKind.ExportSpecifier
+7
View File
@@ -6134,4 +6134,11 @@ namespace ts {
export function pseudoBigIntToString({negative, base10Value}: PseudoBigInt): string {
return (negative && base10Value !== "0" ? "-" : "") + base10Value;
}
export function isValidTypeOnlyAliasUseSite(useSite: Node): boolean {
return !!(useSite.flags & NodeFlags.Ambient)
|| isPartOfTypeQuery(useSite)
|| isPartOfPossiblyValidTypeOrAbstractComputedPropertyName(useSite)
|| !isExpressionNode(useSite);
}
}
+38 -53
View File
@@ -30,8 +30,8 @@ namespace ts.codefix {
const addToNamespace: FixUseNamespaceImport[] = [];
const importType: FixUseImportType[] = [];
// Keys are import clause node IDs.
const addToExisting = createMap<{ readonly importClause: ImportClause, defaultImport: string | undefined; readonly namedImports: string[] }>();
const newImports = createNewImportMap();
const addToExisting = createMap<{ readonly importClause: ImportClause, defaultImport: string | undefined; readonly namedImports: string[], canUseTypeOnlyImport: boolean }>();
const newImports = createMap<Mutable<ImportsCollection>>();
eachDiagnostic(context, errorCodes, diag => {
const info = getFixesInfo(context, diag.code, diag.start);
@@ -47,11 +47,11 @@ namespace ts.codefix {
importType.push(fix);
break;
case ImportFixKind.AddToExisting: {
const { importClause, importKind } = fix;
const { importClause, importKind, canUseTypeOnlyImport } = fix;
const key = String(getNodeId(importClause));
let entry = addToExisting.get(key);
if (!entry) {
addToExisting.set(key, entry = { importClause, defaultImport: undefined, namedImports: [] });
addToExisting.set(key, entry = { importClause, defaultImport: undefined, namedImports: [], canUseTypeOnlyImport });
}
if (importKind === ImportKind.Named) {
pushIfUnique(entry.namedImports, symbolName);
@@ -63,10 +63,10 @@ namespace ts.codefix {
break;
}
case ImportFixKind.AddNew: {
const { moduleSpecifier, importKind, typeOnly } = fix;
let entry = newImports.get(moduleSpecifier, typeOnly);
const { moduleSpecifier, importKind } = fix;
let entry = newImports.get(moduleSpecifier);
if (!entry) {
newImports.set(moduleSpecifier, typeOnly, entry = { defaultImport: undefined, namedImports: [], namespaceLikeImport: undefined });
newImports.set(moduleSpecifier, entry = { defaultImport: undefined, namedImports: [], namespaceLikeImport: undefined });
}
switch (importKind) {
case ImportKind.Default:
@@ -97,41 +97,16 @@ namespace ts.codefix {
for (const fix of importType) {
addImportType(changes, sourceFile, fix, quotePreference);
}
addToExisting.forEach(({ importClause, defaultImport, namedImports }) => {
doAddExistingFix(changes, sourceFile, importClause, defaultImport, namedImports);
addToExisting.forEach(({ importClause, defaultImport, namedImports, canUseTypeOnlyImport }) => {
doAddExistingFix(changes, sourceFile, importClause, defaultImport, namedImports, canUseTypeOnlyImport);
});
newImports.forEach((imports, moduleSpecifier, typeOnly) => {
addNewImports(changes, sourceFile, typeOnly, moduleSpecifier, quotePreference, imports);
newImports.forEach((imports, moduleSpecifier) => {
addNewImports(changes, sourceFile, moduleSpecifier, quotePreference, imports);
});
}));
},
});
function createNewImportMap() {
// Keys are module specifiers.
const newImports = createMap<Mutable<ImportsCollection>>();
return {
get: (moduleSpecifier: string, typeOnly: boolean) => {
return newImports.get(key(moduleSpecifier, typeOnly));
},
set: (moduleSpecifier: string, typeOnly: boolean, value: ImportsCollection) => {
return newImports.set(key(moduleSpecifier, typeOnly), value);
},
forEach: (action: (value: ImportsCollection, moduleSpecifier: string, typeOnly: boolean) => void) => {
newImports.forEach((value, key) => {
const typeOnly = !!+key[0];
const moduleSpecifier = key.slice(1);
action(value, moduleSpecifier, typeOnly);
});
},
};
function key(moduleSpecifier: string, typeOnly: boolean) {
const prefix = typeOnly ? "1" : "0";
return prefix + moduleSpecifier;
}
}
// Sorted with the preferred fix coming first.
const enum ImportFixKind { UseNamespace, ImportType, AddToExisting, AddNew }
type ImportFix = FixUseNamespaceImport | FixUseImportType | FixAddToExistingImport | FixAddNewImport;
@@ -149,12 +124,12 @@ namespace ts.codefix {
readonly kind: ImportFixKind.AddToExisting;
readonly importClause: ImportClause;
readonly importKind: ImportKind.Default | ImportKind.Named;
readonly canUseTypeOnlyImport: boolean;
}
interface FixAddNewImport {
readonly kind: ImportFixKind.AddNew;
readonly moduleSpecifier: string;
readonly importKind: ImportKind;
readonly typeOnly: boolean;
}
const enum ImportKind {
@@ -177,7 +152,6 @@ namespace ts.codefix {
interface FixAddToExistingImportInfo {
readonly declaration: AnyImportSyntax;
readonly importKind: ImportKind;
readonly exportedSymbolIsTypeOnly: boolean;
}
export function getImportCompletionAction(
@@ -229,9 +203,14 @@ namespace ts.codefix {
return !(skipAlias(s, checker).flags & SymbolFlags.Value);
}
function isTypeOnlyPosition(sourceFile: SourceFile, position: number) {
return isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
}
function getFixForImport(
exportInfos: readonly SymbolExportInfo[],
symbolName: string,
/** undefined only for missing JSX namespace */
position: number | undefined,
program: Program,
sourceFile: SourceFile,
@@ -241,7 +220,7 @@ namespace ts.codefix {
const checker = program.getTypeChecker();
const existingImports = flatMap(exportInfos, info => getExistingImportDeclarations(info, checker, sourceFile));
const useNamespace = position === undefined ? undefined : tryUseExistingNamespaceImport(existingImports, symbolName, position, checker);
const addToExisting = tryAddToExistingImport(existingImports);
const addToExisting = tryAddToExistingImport(existingImports, position !== undefined && isTypeOnlyPosition(sourceFile, position));
// 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, position, host, preferences);
return [...(useNamespace ? [useNamespace] : emptyArray), ...addImport];
@@ -271,14 +250,14 @@ namespace ts.codefix {
});
}
function tryAddToExistingImport(existingImports: readonly FixAddToExistingImportInfo[]): FixAddToExistingImport | undefined {
function tryAddToExistingImport(existingImports: readonly FixAddToExistingImportInfo[], canUseTypeOnlyImport: boolean): FixAddToExistingImport | undefined {
return firstDefined(existingImports, ({ declaration, importKind }): FixAddToExistingImport | undefined => {
if (declaration.kind !== SyntaxKind.ImportDeclaration) return undefined;
const { importClause } = declaration;
if (!importClause) return undefined;
const { name, namedBindings } = importClause;
return importKind === ImportKind.Default && !name || importKind === ImportKind.Named && (!namedBindings || namedBindings.kind === SyntaxKind.NamedImports)
? { kind: ImportFixKind.AddToExisting, importClause, importKind }
? { kind: ImportFixKind.AddToExisting, importClause, importKind, canUseTypeOnlyImport }
: undefined;
});
}
@@ -312,13 +291,14 @@ namespace ts.codefix {
): readonly (FixAddNewImport | FixUseImportType)[] {
const isJs = isSourceFileJS(sourceFile);
const { allowsImportingSpecifier } = createAutoImportFilter(sourceFile, program, host);
const choicesForEachExportingModule = flatMap(moduleSymbols, ({ moduleSymbol, importKind, exportedSymbolIsTypeOnly }) =>
moduleSpecifiers.getModuleSpecifiers(moduleSymbol, program.getCompilerOptions(), sourceFile, host, program.getSourceFiles(), preferences, program.redirectTargetsMap)
.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, "position should be defined") }
: { kind: ImportFixKind.AddNew, moduleSpecifier, importKind, typeOnly: exportedSymbolIsTypeOnly }));
: { kind: ImportFixKind.AddNew, moduleSpecifier, importKind }));
// Sort by presence in package.json, then shortest paths first
return sort(choicesForEachExportingModule, (a, b) => {
@@ -347,14 +327,14 @@ namespace ts.codefix {
return existingDeclaration ? [existingDeclaration] : getNewImportInfos(program, sourceFile, position, exportInfos, host, preferences);
}
function newImportInfoFromExistingSpecifier({ declaration, importKind, exportedSymbolIsTypeOnly }: FixAddToExistingImportInfo): FixAddNewImport | undefined {
function newImportInfoFromExistingSpecifier({ declaration, importKind }: FixAddToExistingImportInfo): FixAddNewImport | undefined {
const expression = declaration.kind === SyntaxKind.ImportDeclaration
? declaration.moduleSpecifier
: declaration.moduleReference.kind === SyntaxKind.ExternalModuleReference
? declaration.moduleReference.expression
: undefined;
return expression && isStringLiteral(expression)
? { kind: ImportFixKind.AddNew, moduleSpecifier: expression.text, importKind, typeOnly: exportedSymbolIsTypeOnly }
? { kind: ImportFixKind.AddNew, moduleSpecifier: expression.text, importKind }
: undefined;
}
@@ -554,14 +534,14 @@ namespace ts.codefix {
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);
const { importClause, importKind, canUseTypeOnlyImport } = fix;
doAddExistingFix(changes, sourceFile, importClause, importKind === ImportKind.Default ? symbolName : undefined, importKind === ImportKind.Named ? [symbolName] : emptyArray, canUseTypeOnlyImport);
const moduleSpecifierWithoutQuotes = stripQuotes(importClause.parent.moduleSpecifier.getText());
return [importKind === ImportKind.Default ? Diagnostics.Add_default_import_0_to_existing_import_declaration_from_1 : Diagnostics.Add_0_to_existing_import_declaration_from_1, symbolName, moduleSpecifierWithoutQuotes]; // you too!
}
case ImportFixKind.AddNew: {
const { importKind, moduleSpecifier } = fix;
addNewImports(changes, sourceFile, fix.typeOnly, moduleSpecifier, quotePreference, importKind === ImportKind.Default ? { defaultImport: symbolName, namedImports: emptyArray, namespaceLikeImport: undefined }
addNewImports(changes, sourceFile, moduleSpecifier, quotePreference, importKind === ImportKind.Default ? { defaultImport: symbolName, namedImports: emptyArray, namespaceLikeImport: undefined }
: importKind === ImportKind.Named ? { defaultImport: undefined, namedImports: [symbolName], namespaceLikeImport: undefined }
: { defaultImport: undefined, namedImports: emptyArray, namespaceLikeImport: { importKind, name: symbolName } });
return [importKind === ImportKind.Default ? Diagnostics.Import_default_0_from_module_1 : Diagnostics.Import_0_from_module_1, symbolName, moduleSpecifier];
@@ -571,9 +551,10 @@ namespace ts.codefix {
}
}
function doAddExistingFix(changes: textChanges.ChangeTracker, sourceFile: SourceFile, clause: ImportClause, defaultImport: string | undefined, namedImports: readonly string[]): void {
function doAddExistingFix(changes: textChanges.ChangeTracker, sourceFile: SourceFile, clause: ImportClause, defaultImport: string | undefined, namedImports: readonly string[], canUseTypeOnlyImport: boolean): void {
const convertTypeOnlyToRegular = !canUseTypeOnlyImport && clause.isTypeOnly;
if (defaultImport) {
Debug.assert(!clause.name, "Default imports can't have names");
Debug.assert(!clause.name, "Cannot add a default import to an import clause that already has one");
changes.insertNodeAt(sourceFile, clause.getStart(sourceFile), createIdentifier(defaultImport), { suffix: ", " });
}
@@ -591,11 +572,15 @@ namespace ts.codefix {
changes.replaceNode(sourceFile, clause.namedBindings, namedImports);
}
else {
changes.insertNodeAfter(sourceFile, Debug.assertDefined(clause.name, "Named import specifiers must have names"), namedImports);
changes.insertNodeAfter(sourceFile, Debug.assertDefined(clause.name, "Import clause must have either named imports or a default import"), namedImports);
}
}
}
}
if (convertTypeOnlyToRegular) {
changes.delete(sourceFile, getTypeKeywordOfTypeOnlyImport(clause, sourceFile));
}
}
function addNamespaceQualifier(changes: textChanges.ChangeTracker, sourceFile: SourceFile, { namespacePrefix, position }: FixUseNamespaceImport): void {
@@ -619,13 +604,13 @@ namespace ts.codefix {
readonly name: string;
} | undefined;
}
function addNewImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, typeOnly: boolean, moduleSpecifier: string, quotePreference: QuotePreference, { defaultImport, namedImports, namespaceLikeImport }: ImportsCollection): void {
function addNewImports(changes: textChanges.ChangeTracker, sourceFile: SourceFile, moduleSpecifier: string, quotePreference: QuotePreference, { defaultImport, namedImports, namespaceLikeImport }: ImportsCollection): void {
const quotedModuleSpecifier = makeStringLiteral(moduleSpecifier, quotePreference);
if (defaultImport !== undefined || namedImports.length) {
insertImport(changes, sourceFile,
makeImport(
defaultImport === undefined ? undefined : createIdentifier(defaultImport),
namedImports.map(n => createImportSpecifier(/*propertyName*/ undefined, createIdentifier(n))), moduleSpecifier, quotePreference, typeOnly));
namedImports.map(n => createImportSpecifier(/*propertyName*/ undefined, createIdentifier(n))), moduleSpecifier, quotePreference));
}
if (namespaceLikeImport) {
insertImport(
@@ -633,7 +618,7 @@ namespace ts.codefix {
sourceFile,
namespaceLikeImport.importKind === ImportKind.Equals ? createImportEqualsDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createIdentifier(namespaceLikeImport.name), createExternalModuleReference(quotedModuleSpecifier)) :
namespaceLikeImport.importKind === ImportKind.ConstEquals ? createConstEqualsRequireDeclaration(namespaceLikeImport.name, quotedModuleSpecifier) :
createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(namespaceLikeImport.name)), typeOnly), quotedModuleSpecifier));
createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(namespaceLikeImport.name))), quotedModuleSpecifier));
}
}
+7 -4
View File
@@ -525,6 +525,7 @@ namespace ts.Completions {
symbolToOriginInfoMap: SymbolOriginInfoMap;
previousToken: Node | undefined;
readonly isJsxInitializer: IsJsxInitializer;
readonly isTypeOnlyLocation: boolean;
}
function getSymbolCompletionFromEntryId(
program: Program,
@@ -543,7 +544,7 @@ namespace ts.Completions {
return { type: "request", request: completionData };
}
const { symbols, literals, location, completionKind, symbolToOriginInfoMap, previousToken, isJsxInitializer } = completionData;
const { symbols, literals, location, completionKind, symbolToOriginInfoMap, previousToken, isJsxInitializer, isTypeOnlyLocation } = completionData;
const literal = find(literals, l => completionNameForLiteral(l) === entryId.name);
if (literal !== undefined) return { type: "literal", literal };
@@ -556,7 +557,7 @@ namespace ts.Completions {
const origin = symbolToOriginInfoMap[getSymbolId(symbol)];
const info = getCompletionEntryDisplayNameForSymbol(symbol, compilerOptions.target!, origin, completionKind);
return info && info.name === entryId.name && getSourceFromOrigin(origin) === entryId.source
? { type: "symbol" as const, symbol, location, symbolToOriginInfoMap, previousToken, isJsxInitializer }
? { type: "symbol" as const, symbol, location, symbolToOriginInfoMap, previousToken, isJsxInitializer, isTypeOnlyLocation }
: undefined;
}) || { type: "none" };
}
@@ -714,6 +715,7 @@ namespace ts.Completions {
readonly isJsxInitializer: IsJsxInitializer;
readonly insideJsDocTagTypeExpression: boolean;
readonly symbolToSortTextMap: SymbolSortTextMap;
readonly isTypeOnlyLocation: boolean;
}
type Request = { readonly kind: CompletionDataKind.JsDocTagName | CompletionDataKind.JsDocTag } | { readonly kind: CompletionDataKind.JsDocParameterName, tag: JSDocParameterTag };
@@ -1012,6 +1014,7 @@ namespace ts.Completions {
const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
const symbolToSortTextMap: SymbolSortTextMap = [];
const importSuggestionsCache = host.getImportSuggestionsCache && host.getImportSuggestionsCache();
const isTypeOnly = isTypeOnlyCompletion();
if (isRightOfDot || isRightOfQuestionDot) {
getTypeScriptMemberSymbols();
@@ -1061,7 +1064,8 @@ namespace ts.Completions {
previousToken,
isJsxInitializer,
insideJsDocTagTypeExpression,
symbolToSortTextMap
symbolToSortTextMap,
isTypeOnlyLocation: isTypeOnly
};
type JSDocTagWithTypeExpression = JSDocParameterTag | JSDocPropertyTag | JSDocReturnTag | JSDocTypeTag | JSDocTypedefTag;
@@ -1329,7 +1333,6 @@ namespace ts.Completions {
const scopeNode = getScopeNode(contextToken, adjustedPosition, sourceFile) || sourceFile;
isInSnippetScope = isSnippetScope(scopeNode);
const isTypeOnly = isTypeOnlyCompletion();
const symbolMeanings = (isTypeOnly ? SymbolFlags.None : SymbolFlags.Value) | SymbolFlags.Type | SymbolFlags.Namespace | SymbolFlags.Alias;
symbols = Debug.assertEachDefined(typeChecker.getSymbolsInScope(scopeNode, symbolMeanings), "getSymbolsInScope() should all be defined");
+9
View File
@@ -1306,6 +1306,10 @@ namespace ts {
return contains(typeKeywords, kind);
}
export function isTypeKeywordToken(node: Node): node is Token<SyntaxKind.TypeKeyword> {
return node.kind === SyntaxKind.TypeKeyword;
}
/** True if the symbol is for an external module, as opposed to a namespace. */
export function isExternalModuleSymbol(moduleSymbol: Symbol): boolean {
return !!(moduleSymbol.flags & SymbolFlags.Module) && moduleSymbol.name.charCodeAt(0) === CharacterCodes.doubleQuote;
@@ -1494,6 +1498,11 @@ namespace ts {
}
}
export function getTypeKeywordOfTypeOnlyImport(importClause: ImportClause, sourceFile: SourceFile): Token<SyntaxKind.TypeKeyword> {
Debug.assert(importClause.isTypeOnly);
return cast(importClause.getChildAt(0, sourceFile), isTypeKeywordToken);
}
export function textSpansEqual(a: TextSpan | undefined, b: TextSpan | undefined): boolean {
return !!a && !!b && a.start === b.start && a.length === b.length;
}
@@ -20,7 +20,7 @@ goTo.file("/b.ts");
verify.codeFix({
description: "Infer parameter types from usage",
newFileContent:
`import type { User } from "./a";
`import { User } from "./a";
export function f(user: User) {
getEmail(user);
@@ -43,7 +43,7 @@ verify.applyCodeActionFromCompletion("1", {
source: "/a",
description: `Import 'b' from module "./a"`,
newFileContent:
`import type { b } from "./a";
`import { b } from "./a";
a;
let x: b;`,
@@ -54,7 +54,7 @@ verify.applyCodeActionFromCompletion("0", {
source: "/a",
description: `Import 'a' from module "./a"`,
newFileContent:
`import type { b } from "./a";
`import { b } from "./a";
import a = require("./a");
a;
@@ -0,0 +1,24 @@
/// <reference path="fourslash.ts" />
// @target: esnext
// @Filename: /a.ts
//// export class A {}
//// export class B {}
// @Filename: /b.ts
//// import type { A } from './a';
//// const b: B/**/
goTo.file('/b.ts');
verify.applyCodeActionFromCompletion('', {
name: 'B',
source: '/a',
description: `Add 'B' to existing import declaration from "./a"`,
preferences: {
includeCompletionsForModuleExports: true,
includeInsertTextCompletions: true
},
newFileContent: `import type { A, B } from './a';
const b: B`
});
@@ -0,0 +1,13 @@
/// <reference path="fourslash.ts" />
// @Filename: /a.ts
//// export class A {}
//// export class B {}
// @Filename: /b.ts
//// import type { A } from './a';
//// new B
goTo.file('/b.ts');
verify.importFixAtPosition([`import { A, B } from './a';
new B`]);
@@ -9,7 +9,7 @@
//// }
verify.importFixAtPosition([
`import type { XXX } from "./module";
`import { XXX } from "./module";
let t: XXX.I;`
]);
@@ -10,7 +10,7 @@
//// }
verify.importFixAtPosition([
`import type { A } from "./module";
`import { A } from "./module";
let t: A.B.I;`
]);
@@ -1,14 +0,0 @@
/// <reference path="fourslash.ts" />
// @target: esnext
// @Filename: /a.ts
////export interface Car {}
// @Filename: /b.ts
////function drive(car: /**/Car) {}
goTo.file("/b.ts");
verify.importFixAtPosition([`import type { Car } from "./a";
function drive(car: Car) {}`]);
@@ -16,9 +16,9 @@ verify.codeFixAll({
fixId: "fixMissingImport",
fixAllDescription: "Add all missing imports",
newFileContent:
`import a = require("./a");
`import { b } from "./a";
import type { b } from "./a";
import a = require("./a");
a;
let x: b;`,