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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix auto import crash due to difference in paths handling #50419

Merged
merged 1 commit into from Aug 25, 2022
Merged

Fix auto import crash due to difference in paths handling #50419

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
72 changes: 29 additions & 43 deletions src/services/codefixes/importFixes.ts
View file
Open in desktop
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;
}
Comment on lines -416 to -419
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the problematic code in particular; the rationale in the comment doesn’t hold if this file is the target of a tsconfig paths alias, which we don’t know until later in the process. But there is not much need to fix it here when this code is completely duplicated by the ExportMapCache.


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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
@@ -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
View file
Open in desktop
Expand Up @@ -108,6 +108,7 @@ declare module ts {
fileName?: string;
ambientModuleName?: string;
isPackageJsonImport?: true;
moduleSpecifier?: string;
exportName: string;
}

Expand Down