Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

馃 Pick PR #49442 (Avoid repeating codefix work when r...) into release-4.7 #49447

Merged
merged 1 commit into from Jun 9, 2022
Merged

馃 Pick PR #49442 (Avoid repeating codefix work when r...) into release-4.7 #49447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
106 changes: 70 additions & 36 deletions src/services/codefixes/importFixes.ts
View file
Open in desktop
Expand Up @@ -40,6 +40,9 @@ namespace ts.codefix {
},
});

/**
* Computes multiple import additions to a file and writes them to a ChangeTracker.
*/
export interface ImportAdder {
hasFixes(): boolean;
addImportFromDiagnostic: (diagnostic: DiagnosticWithLocation, context: CodeFixContextBase) => void;
Expand Down Expand Up @@ -235,6 +238,47 @@ namespace ts.codefix {
}
}

/**
* Computes module specifiers for multiple import additions to a file.
*/
export interface ImportSpecifierResolver {
getModuleSpecifierForBestExportInfo(
exportInfo: readonly SymbolExportInfo[],
symbolName: string,
position: number,
isValidTypeOnlyUseSite: boolean,
fromCacheOnly?: boolean
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string, computedWithoutCacheCount: number } | undefined;
}

export function createImportSpecifierResolver(importingFile: SourceFile, program: Program, host: LanguageServiceHost, preferences: UserPreferences): ImportSpecifierResolver {
const packageJsonImportFilter = createPackageJsonImportFilter(importingFile, preferences, host);
const importMap = createExistingImportMap(program.getTypeChecker(), importingFile, program.getCompilerOptions());
return { getModuleSpecifierForBestExportInfo };

function getModuleSpecifierForBestExportInfo(
exportInfo: readonly SymbolExportInfo[],
symbolName: string,
position: number,
isValidTypeOnlyUseSite: boolean,
fromCacheOnly?: boolean,
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string, computedWithoutCacheCount: number } | undefined {
const { fixes, computedWithoutCacheCount } = getImportFixes(
exportInfo,
{ symbolName, position },
isValidTypeOnlyUseSite,
/*useRequire*/ false,
program,
importingFile,
host,
preferences,
importMap,
fromCacheOnly);
const result = getBestFix(fixes, importingFile, program, packageJsonImportFilter, host);
return result && { ...result, computedWithoutCacheCount };
}
}

// Sorted with the preferred fix coming first.
const enum ImportFixKind { UseNamespace, JsdocTypeImport, AddToExisting, AddNew, PromoteTypeOnly }
// These should not be combined as bitflags, but are given powers of 2 values to
Expand Down Expand Up @@ -394,32 +438,6 @@ namespace ts.codefix {
}
}

export function getModuleSpecifierForBestExportInfo(
exportInfo: readonly SymbolExportInfo[],
symbolName: string,
position: number,
isValidTypeOnlyUseSite: boolean,
importingFile: SourceFile,
program: Program,
host: LanguageServiceHost,
preferences: UserPreferences,
packageJsonImportFilter?: PackageJsonImportFilter,
fromCacheOnly?: boolean,
): { exportInfo?: SymbolExportInfo, moduleSpecifier: string, computedWithoutCacheCount: number } | undefined {
const { fixes, computedWithoutCacheCount } = getImportFixes(
exportInfo,
{ symbolName, position },
isValidTypeOnlyUseSite,
/*useRequire*/ false,
program,
importingFile,
host,
preferences,
fromCacheOnly);
const result = getBestFix(fixes, importingFile, program, packageJsonImportFilter || createPackageJsonImportFilter(importingFile, preferences, host), host);
return result && { ...result, computedWithoutCacheCount };
}

function getImportFixes(
exportInfos: readonly SymbolExportInfo[],
useNamespaceInfo: {
Expand All @@ -433,10 +451,11 @@ namespace ts.codefix {
sourceFile: SourceFile,
host: LanguageServiceHost,
preferences: UserPreferences,
importMap = createExistingImportMap(program.getTypeChecker(), sourceFile, program.getCompilerOptions()),
fromCacheOnly?: boolean,
): { computedWithoutCacheCount: number, fixes: readonly ImportFixWithModuleSpecifier[] } {
const checker = program.getTypeChecker();
const existingImports = flatMap(exportInfos, info => getExistingImportDeclarations(info, checker, sourceFile, program.getCompilerOptions()));
const existingImports = flatMap(exportInfos, importMap.getImportsForExportInfo);
const useNamespace = useNamespaceInfo && tryUseExistingNamespaceImport(existingImports, useNamespaceInfo.symbolName, useNamespaceInfo.position, checker);
const addToExisting = tryAddToExistingImport(existingImports, isValidTypeOnlyUseSite, checker, program.getCompilerOptions());
if (addToExisting) {
Expand Down Expand Up @@ -587,19 +606,34 @@ namespace ts.codefix {
});
}

function getExistingImportDeclarations({ moduleSymbol, exportKind, targetFlags, symbol }: SymbolExportInfo, checker: TypeChecker, importingFile: SourceFile, compilerOptions: CompilerOptions): readonly FixAddToExistingImportInfo[] {
// Can't use an es6 import for a type in JS.
if (!(targetFlags & SymbolFlags.Value) && isSourceFileJS(importingFile)) return emptyArray;
const importKind = getImportKind(importingFile, exportKind, compilerOptions);
return mapDefined(importingFile.imports, (moduleSpecifier): FixAddToExistingImportInfo | undefined => {
function createExistingImportMap(checker: TypeChecker, importingFile: SourceFile, compilerOptions: CompilerOptions) {
let importMap: MultiMap<SymbolId, AnyImportOrRequire> | undefined;
for (const moduleSpecifier of importingFile.imports) {
const i = importFromModuleSpecifier(moduleSpecifier);
if (isVariableDeclarationInitializedToRequire(i.parent)) {
return checker.resolveExternalModuleName(moduleSpecifier) === moduleSymbol ? { declaration: i.parent, importKind, symbol, targetFlags } : undefined;
const moduleSymbol = checker.resolveExternalModuleName(moduleSpecifier);
if (moduleSymbol) {
(importMap ||= createMultiMap()).add(getSymbolId(moduleSymbol), i.parent);
}
}
if (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) {
return checker.getSymbolAtLocation(moduleSpecifier) === moduleSymbol ? { declaration: i, importKind, symbol, targetFlags } : undefined;
else if (i.kind === SyntaxKind.ImportDeclaration || i.kind === SyntaxKind.ImportEqualsDeclaration) {
const moduleSymbol = checker.getSymbolAtLocation(moduleSpecifier);
if (moduleSymbol) {
(importMap ||= createMultiMap()).add(getSymbolId(moduleSymbol), i);
}
}
});
}

return {
getImportsForExportInfo: ({ moduleSymbol, exportKind, targetFlags, symbol }: SymbolExportInfo): readonly FixAddToExistingImportInfo[] => {
// Can't use an es6 import for a type in JS.
if (!(targetFlags & SymbolFlags.Value) && isSourceFileJS(importingFile)) return emptyArray;
const matchingDeclarations = importMap?.get(getSymbolId(moduleSymbol));
if (!matchingDeclarations) return emptyArray;
const importKind = getImportKind(importingFile, exportKind, compilerOptions);
return matchingDeclarations.map(declaration => ({ declaration, importKind, symbol, targetFlags }));
}
};
}

function shouldUseRequire(sourceFile: SourceFile, program: Program): boolean {
Expand Down
16 changes: 8 additions & 8 deletions src/services/completions.ts
View file
Open in desktop
Expand Up @@ -184,16 +184,15 @@ namespace ts.Completions {
function resolvingModuleSpecifiers<TReturn>(
logPrefix: string,
host: LanguageServiceHost,
resolver: codefix.ImportSpecifierResolver,
program: Program,
sourceFile: SourceFile,
position: number,
preferences: UserPreferences,
isForImportStatementCompletion: boolean,
isValidTypeOnlyUseSite: boolean,
cb: (context: ModuleSpecifierResolutioContext) => TReturn,
): TReturn {
const start = timestamp();
const packageJsonImportFilter = createPackageJsonImportFilter(sourceFile, preferences, host);
// Under `--moduleResolution nodenext`, we have to resolve module specifiers up front, because
// package.json exports can mean we *can't* resolve a module specifier (that doesn't include a
// relative path into node_modules), and we want to filter those completions out entirely.
Expand Down Expand Up @@ -221,7 +220,7 @@ namespace ts.Completions {

function tryResolve(exportInfo: readonly SymbolExportInfo[], symbolName: string, isFromAmbientModule: boolean): ModuleSpecifierResolutionResult {
if (isFromAmbientModule) {
const result = codefix.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite, sourceFile, program, host, preferences);
const result = resolver.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite);
if (result) {
ambientCount++;
}
Expand All @@ -230,7 +229,7 @@ namespace ts.Completions {
const shouldResolveModuleSpecifier = needsFullResolution || preferences.allowIncompleteCompletions && resolvedCount < moduleSpecifierResolutionLimit;
const shouldGetModuleSpecifierFromCache = !shouldResolveModuleSpecifier && preferences.allowIncompleteCompletions && cacheAttemptCount < moduleSpecifierResolutionCacheAttemptLimit;
const result = (shouldResolveModuleSpecifier || shouldGetModuleSpecifierFromCache)
? codefix.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite, sourceFile, program, host, preferences, packageJsonImportFilter, shouldGetModuleSpecifierFromCache)
? resolver.getModuleSpecifierForBestExportInfo(exportInfo, symbolName, position, isValidTypeOnlyUseSite, shouldGetModuleSpecifierFromCache)
: undefined;

if (!shouldResolveModuleSpecifier && !shouldGetModuleSpecifierFromCache || shouldGetModuleSpecifierFromCache && !result) {
Expand Down Expand Up @@ -371,8 +370,8 @@ namespace ts.Completions {
const newEntries = resolvingModuleSpecifiers(
"continuePreviousIncompleteResponse",
host,
codefix.createImportSpecifierResolver(file, program, host, preferences),
program,
file,
location.getStart(),
preferences,
/*isForImportStatementCompletion*/ false,
Expand Down Expand Up @@ -2206,6 +2205,7 @@ namespace ts.Completions {
let hasUnresolvedAutoImports = false;
// This also gets mutated in nested-functions after the return
let symbols: Symbol[] = [];
let importSpecifierResolver: codefix.ImportSpecifierResolver | undefined;
const symbolToOriginInfoMap: SymbolOriginInfoMap = [];
const symbolToSortTextMap: SymbolSortTextMap = [];
const seenPropertySymbols = new Map<SymbolId, true>();
Expand Down Expand Up @@ -2448,14 +2448,14 @@ namespace ts.Completions {
}
else {
const fileName = isExternalModuleNameRelative(stripQuotes(moduleSymbol.name)) ? getSourceFileOfModule(moduleSymbol)?.fileName : undefined;
const { moduleSpecifier } = codefix.getModuleSpecifierForBestExportInfo([{
const { moduleSpecifier } = (importSpecifierResolver ||= codefix.createImportSpecifierResolver(sourceFile, program, host, preferences)).getModuleSpecifierForBestExportInfo([{
exportKind: ExportKind.Named,
moduleFileName: fileName,
isFromPackageJson: false,
moduleSymbol,
symbol: firstAccessibleSymbol,
targetFlags: skipAlias(firstAccessibleSymbol, typeChecker).flags,
}], firstAccessibleSymbol.name, position, isValidTypeOnlyAliasUseSite(location), sourceFile, program, host, preferences) || {};
}], firstAccessibleSymbol.name, position, isValidTypeOnlyAliasUseSite(location)) || {};

if (moduleSpecifier) {
const origin: SymbolOriginInfoResolvedExport = {
Expand Down Expand Up @@ -2733,8 +2733,8 @@ namespace ts.Completions {
resolvingModuleSpecifiers(
"collectAutoImports",
host,
importSpecifierResolver ||= codefix.createImportSpecifierResolver(sourceFile, program, host, preferences),
program,
sourceFile,
position,
preferences,
!!importCompletionNode,
Expand Down