Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Fix auto import crash due to difference in paths handling (#50419)
  • Loading branch information
andrewbranch committed Aug 25, 2022
1 parent 12eb519 commit 38076df
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 48 deletions.
72 changes: 29 additions & 43 deletions src/services/codefixes/importFixes.ts
Expand Up @@ -32,8 +32,8 @@ namespace ts.codefix {
},
fixIds: [importFixId],
getAllCodeActions: context => {
const { sourceFile, program, preferences, host } = context;
const importAdder = createImportAdderWorker(sourceFile, program, /*useAutoImportProvider*/ true, preferences, host);
const { sourceFile, program, preferences, host, cancellationToken } = context;
const importAdder = createImportAdderWorker(sourceFile, program, /*useAutoImportProvider*/ true, preferences, host, cancellationToken);
eachDiagnostic(context, errorCodes, diag => importAdder.addImportFromDiagnostic(diag, context));
return createCombinedCodeActions(textChanges.ChangeTracker.with(context, importAdder.writeFixes));
},
Expand All @@ -49,8 +49,8 @@ namespace ts.codefix {
writeFixes: (changeTracker: textChanges.ChangeTracker) => void;
}

export function createImportAdder(sourceFile: SourceFile, program: Program, preferences: UserPreferences, host: LanguageServiceHost): ImportAdder {
return createImportAdderWorker(sourceFile, program, /*useAutoImportProvider*/ false, preferences, host);
export function createImportAdder(sourceFile: SourceFile, program: Program, preferences: UserPreferences, host: LanguageServiceHost, cancellationToken?: CancellationToken): ImportAdder {
return createImportAdderWorker(sourceFile, program, /*useAutoImportProvider*/ false, preferences, host, cancellationToken);
}

interface AddToExistingState {
Expand All @@ -59,7 +59,7 @@ namespace ts.codefix {
readonly namedImports: ESMap<string, AddAsTypeOnly>;
}

function createImportAdderWorker(sourceFile: SourceFile, program: Program, useAutoImportProvider: boolean, preferences: UserPreferences, host: LanguageServiceHost): ImportAdder {
function createImportAdderWorker(sourceFile: SourceFile, program: Program, useAutoImportProvider: boolean, preferences: UserPreferences, host: LanguageServiceHost, cancellationToken: CancellationToken | undefined): ImportAdder {
const compilerOptions = program.getCompilerOptions();
// Namespace fixes don't conflict, so just build a list.
const addToNamespace: FixUseNamespaceImport[] = [];
Expand All @@ -83,9 +83,9 @@ namespace ts.codefix {
const symbolName = getNameForExportedSymbol(exportedSymbol, getEmitScriptTarget(compilerOptions));
const checker = program.getTypeChecker();
const symbol = checker.getMergedSymbol(skipAlias(exportedSymbol, checker));
const exportInfo = getAllReExportingModules(sourceFile, symbol, moduleSymbol, symbolName, /*isJsxTagName*/ false, host, program, preferences, useAutoImportProvider);
const exportInfo = getAllExportInfoForSymbol(sourceFile, symbol, symbolName, /*isJsxTagName*/ false, program, host, preferences, cancellationToken);
const useRequire = shouldUseRequire(sourceFile, program);
const fix = getImportFixForSymbol(sourceFile, exportInfo, moduleSymbol, program, /*useNamespaceInfo*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
const fix = getImportFixForSymbol(sourceFile, Debug.checkDefined(exportInfo), moduleSymbol, program, /*useNamespaceInfo*/ undefined, !!isValidTypeOnlyUseSite, useRequire, host, preferences);
if (fix) {
addImport({ fix, symbolName, errorIdentifierText: undefined });
}
Expand Down Expand Up @@ -345,11 +345,15 @@ namespace ts.codefix {
formatContext: formatting.FormatContext,
position: number,
preferences: UserPreferences,
cancellationToken: CancellationToken,
): { readonly moduleSpecifier: string, readonly codeAction: CodeAction } {
const compilerOptions = program.getCompilerOptions();

const exportInfos = pathIsBareSpecifier(stripQuotes(moduleSymbol.name))
? [getSymbolExportInfoForSymbol(targetSymbol, moduleSymbol, program, host)]
: getAllReExportingModules(sourceFile, targetSymbol, moduleSymbol, symbolName, isJsxTagName, host, program, preferences, /*useAutoImportProvider*/ true);
? [getSingleExportInfoForSymbol(targetSymbol, moduleSymbol, program, host)]
: getAllExportInfoForSymbol(sourceFile, targetSymbol, symbolName, isJsxTagName, program, host, preferences, cancellationToken);

Debug.assertIsDefined(exportInfos);
const useRequire = shouldUseRequire(sourceFile, program);
const isValidTypeOnlyUseSite = isValidTypeOnlyAliasUseSite(getTokenAtPosition(sourceFile, position));
const fix = Debug.checkDefined(getImportFixForSymbol(sourceFile, exportInfos, moduleSymbol, program, { symbolName, position }, isValidTypeOnlyUseSite, useRequire, host, preferences));
Expand Down Expand Up @@ -383,7 +387,17 @@ namespace ts.codefix {
return { description, changes, commands };
}

function getSymbolExportInfoForSymbol(symbol: Symbol, moduleSymbol: Symbol, program: Program, host: LanguageServiceHost): SymbolExportInfo {
function getAllExportInfoForSymbol(importingFile: SourceFile, symbol: Symbol, symbolName: string, preferCapitalized: boolean, program: Program, host: LanguageServiceHost, preferences: UserPreferences, cancellationToken: CancellationToken | undefined): readonly SymbolExportInfo[] | undefined {
const getChecker = createGetChecker(program, host);
return getExportInfoMap(importingFile, host, program, preferences, cancellationToken)
.search(importingFile.path, preferCapitalized, name => name === symbolName, info => {
if (skipAlias(info[0].symbol, getChecker(info[0].isFromPackageJson)) === symbol) {
return info;
}
});
}

function getSingleExportInfoForSymbol(symbol: Symbol, moduleSymbol: Symbol, program: Program, host: LanguageServiceHost): SymbolExportInfo {
const compilerOptions = program.getCompilerOptions();
const mainProgramInfo = getInfoWithChecker(program.getTypeChecker(), /*isFromPackageJson*/ false);
if (mainProgramInfo) {
Expand All @@ -404,38 +418,6 @@ namespace ts.codefix {
}
}

function getAllReExportingModules(importingFile: SourceFile, targetSymbol: Symbol, exportingModuleSymbol: Symbol, symbolName: string, isJsxTagName: boolean, host: LanguageServiceHost, program: Program, preferences: UserPreferences, useAutoImportProvider: boolean): readonly SymbolExportInfo[] {
const result: SymbolExportInfo[] = [];
const compilerOptions = program.getCompilerOptions();
const getModuleSpecifierResolutionHost = memoizeOne((isFromPackageJson: boolean) => {
return createModuleSpecifierResolutionHost(isFromPackageJson ? host.getPackageJsonAutoImportProvider!()! : program, host);
});

forEachExternalModuleToImportFrom(program, host, preferences, useAutoImportProvider, (moduleSymbol, moduleFile, program, isFromPackageJson) => {
const checker = program.getTypeChecker();
// Don't import from a re-export when looking "up" like to `./index` or `../index`.
if (moduleFile && moduleSymbol !== exportingModuleSymbol && startsWith(importingFile.fileName, getDirectoryPath(moduleFile.fileName))) {
return;
}

const defaultInfo = getDefaultLikeExportInfo(moduleSymbol, checker, compilerOptions);
if (defaultInfo && (defaultInfo.name === symbolName || moduleSymbolToValidIdentifier(moduleSymbol, getEmitScriptTarget(compilerOptions), isJsxTagName) === symbolName) && skipAlias(defaultInfo.symbol, checker) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
result.push({ symbol: defaultInfo.symbol, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: defaultInfo.exportKind, targetFlags: skipAlias(defaultInfo.symbol, checker).flags, isFromPackageJson });
}

for (const exported of checker.getExportsAndPropertiesOfModule(moduleSymbol)) {
if (exported.name === symbolName && checker.getMergedSymbol(skipAlias(exported, checker)) === targetSymbol && isImportable(program, moduleFile, isFromPackageJson)) {
result.push({ symbol: exported, moduleSymbol, moduleFileName: moduleFile?.fileName, exportKind: ExportKind.Named, targetFlags: skipAlias(exported, checker).flags, isFromPackageJson });
}
}
});
return result;

function isImportable(program: Program, moduleFile: SourceFile | undefined, isFromPackageJson: boolean) {
return !moduleFile || isImportableFile(program, importingFile, moduleFile, preferences, /*packageJsonFilter*/ undefined, getModuleSpecifierResolutionHost(isFromPackageJson), host.getModuleSpecifierCache?.());
}
}

function getImportFixes(
exportInfos: readonly SymbolExportInfo[],
useNamespaceInfo: {
Expand Down Expand Up @@ -661,6 +643,10 @@ namespace ts.codefix {
return true;
}

function createGetChecker(program: Program, host: LanguageServiceHost) {
return memoizeOne((isFromPackageJson: boolean) => isFromPackageJson ? host.getPackageJsonAutoImportProvider!()!.getTypeChecker() : program.getTypeChecker());
}

function getNewImportFixes(
program: Program,
sourceFile: SourceFile,
Expand All @@ -675,7 +661,7 @@ namespace ts.codefix {
const isJs = isSourceFileJS(sourceFile);
const compilerOptions = program.getCompilerOptions();
const moduleSpecifierResolutionHost = createModuleSpecifierResolutionHost(program, host);
const getChecker = memoizeOne((isFromPackageJson: boolean) => isFromPackageJson ? host.getPackageJsonAutoImportProvider!()!.getTypeChecker() : program.getTypeChecker());
const getChecker = createGetChecker(program, host);
const rejectNodeModulesRelativePaths = moduleResolutionUsesNodeModules(getEmitModuleResolutionKind(compilerOptions));
const getModuleSpecifiers = fromCacheOnly
? (moduleSymbol: Symbol) => ({ moduleSpecifiers: moduleSpecifiers.tryGetModuleSpecifiersFromCache(moduleSymbol, sourceFile, moduleSpecifierResolutionHost, preferences), computedWithoutCache: false })
Expand Down
6 changes: 4 additions & 2 deletions src/services/completions.ts
Expand Up @@ -1671,7 +1671,7 @@ namespace ts.Completions {
}
case "symbol": {
const { symbol, location, contextToken, origin, previousToken } = symbolCompletion;
const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(name, location, contextToken, origin, symbol, program, host, compilerOptions, sourceFile, position, previousToken, formatContext, preferences, data, source);
const { codeActions, sourceDisplay } = getCompletionEntryCodeActionsAndSourceDisplay(name, location, contextToken, origin, symbol, program, host, compilerOptions, sourceFile, position, previousToken, formatContext, preferences, data, source, cancellationToken);
return createCompletionDetailsForSymbol(symbol, typeChecker, sourceFile, location, cancellationToken, codeActions, sourceDisplay); // TODO: GH#18217
}
case "literal": {
Expand Down Expand Up @@ -1722,6 +1722,7 @@ namespace ts.Completions {
preferences: UserPreferences,
data: CompletionEntryData | undefined,
source: string | undefined,
cancellationToken: CancellationToken,
): CodeActionsAndSourceDisplay {
if (data?.moduleSpecifier) {
if (previousToken && getImportStatementCompletionInfo(contextToken || previousToken).replacementNode) {
Expand Down Expand Up @@ -1786,7 +1787,8 @@ namespace ts.Completions {
program,
formatContext,
previousToken && isIdentifier(previousToken) ? previousToken.getStart(sourceFile) : position,
preferences);
preferences,
cancellationToken);
Debug.assert(!data?.moduleSpecifier || moduleSpecifier === data.moduleSpecifier);
return { sourceDisplay: [textPart(moduleSpecifier)], codeActions: [codeAction] };
}
Expand Down
7 changes: 4 additions & 3 deletions src/services/exportInfoMap.ts
Expand Up @@ -49,7 +49,7 @@ namespace ts {
clear(): void;
add(importingFile: Path, symbol: Symbol, key: __String, moduleSymbol: Symbol, moduleFile: SourceFile | undefined, exportKind: ExportKind, isFromPackageJson: boolean, checker: TypeChecker): void;
get(importingFile: Path, key: string): readonly SymbolExportInfo[] | undefined;
search(importingFile: Path, preferCapitalized: boolean, matches: (name: string, targetFlags: SymbolFlags) => boolean, action: (info: readonly SymbolExportInfo[], symbolName: string, isFromAmbientModule: boolean, key: string) => void): void;
search<T>(importingFile: Path, preferCapitalized: boolean, matches: (name: string, targetFlags: SymbolFlags) => boolean, action: (info: readonly SymbolExportInfo[], symbolName: string, isFromAmbientModule: boolean, key: string) => T | undefined): T | undefined;
releaseSymbols(): void;
isEmpty(): boolean;
/** @returns Whether the change resulted in the cache being cleared */
Expand Down Expand Up @@ -160,14 +160,15 @@ namespace ts {
},
search: (importingFile, preferCapitalized, matches, action) => {
if (importingFile !== usableByFileName) return;
exportInfo.forEach((info, key) => {
return forEachEntry(exportInfo, (info, key) => {
const { symbolName, ambientModuleName } = parseKey(key);
const name = preferCapitalized && info[0].capitalizedSymbolName || symbolName;
if (matches(name, info[0].targetFlags)) {
const rehydrated = info.map(rehydrateCachedInfo);
const filtered = rehydrated.filter((r, i) => isNotShadowedByDeeperNodeModulesPackage(r, info[i].packageName));
if (filtered.length) {
action(filtered, name, !!ambientModuleName, key);
const res = action(filtered, name, !!ambientModuleName, key);
if (res !== undefined) return res;
}
}
});
Expand Down
49 changes: 49 additions & 0 deletions tests/cases/fourslash/completionsImportPathsConflict.ts
@@ -0,0 +1,49 @@
/// <reference path="fourslash.ts" />

// @Filename: /tsconfig.json
//// {
//// "compilerOptions": {
//// "module": "esnext",
//// "paths": {
//// "@reduxjs/toolkit": ["src/index.ts"],
//// "@internal/*": ["src/*"]
//// }
//// }
//// }

// @Filename: /src/index.ts
//// export { configureStore } from "./configureStore";

// @Filename: /src/configureStore.ts
//// export function configureStore() {}

// @Filename: /src/tests/createAsyncThunk.typetest.ts
//// import {} from "@reduxjs/toolkit";
//// /**/

verify.completions({
marker: "",
includes: [{
name: "configureStore",
source: "@reduxjs/toolkit",
sourceDisplay: "@reduxjs/toolkit",
hasAction: true,
sortText: completion.SortText.AutoImportSuggestions,
}],
preferences: {
allowIncompleteCompletions: true,
includeCompletionsForModuleExports: true,
},
});

verify.applyCodeActionFromCompletion("", {
name: "configureStore",
source: "@reduxjs/toolkit",
data: {
exportName: "configureStore",
fileName: "/src/configureStore.ts",
moduleSpecifier: "@reduxjs/toolkit",
},
description: `Update import from "@reduxjs/toolkit"`,
newFileContent: `import { configureStore } from "@reduxjs/toolkit";\n`,
});
1 change: 1 addition & 0 deletions tests/cases/fourslash/fourslash.ts
Expand Up @@ -108,6 +108,7 @@ declare module ts {
fileName?: string;
ambientModuleName?: string;
isPackageJsonImport?: true;
moduleSpecifier?: string;
exportName: string;
}

Expand Down

0 comments on commit 38076df

Please sign in to comment.