From 38076df3465af555051db06006bbf30323af0e2d Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 25 Aug 2022 13:02:48 -0700 Subject: [PATCH] Fix auto import crash due to difference in `paths` handling (#50419) --- src/services/codefixes/importFixes.ts | 72 ++++++++----------- src/services/completions.ts | 6 +- src/services/exportInfoMap.ts | 7 +- .../completionsImportPathsConflict.ts | 49 +++++++++++++ tests/cases/fourslash/fourslash.ts | 1 + 5 files changed, 87 insertions(+), 48 deletions(-) create mode 100644 tests/cases/fourslash/completionsImportPathsConflict.ts diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index b45a83f462f32..496a5d888a194 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -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)); }, @@ -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 { @@ -59,7 +59,7 @@ namespace ts.codefix { readonly namedImports: ESMap; } - 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[] = []; @@ -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 }); } @@ -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)); @@ -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) { @@ -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: { @@ -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, @@ -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 }) diff --git a/src/services/completions.ts b/src/services/completions.ts index c24bc8dcaf165..bcc6efca67577 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -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": { @@ -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) { @@ -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] }; } diff --git a/src/services/exportInfoMap.ts b/src/services/exportInfoMap.ts index 964079a8604fe..5e927b3ff8afb 100644 --- a/src/services/exportInfoMap.ts +++ b/src/services/exportInfoMap.ts @@ -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(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 */ @@ -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; } } }); diff --git a/tests/cases/fourslash/completionsImportPathsConflict.ts b/tests/cases/fourslash/completionsImportPathsConflict.ts new file mode 100644 index 0000000000000..3dbfbe949025a --- /dev/null +++ b/tests/cases/fourslash/completionsImportPathsConflict.ts @@ -0,0 +1,49 @@ +/// + +// @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`, +}); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index bc3f1aab46ea4..76ca5d33314d7 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -108,6 +108,7 @@ declare module ts { fileName?: string; ambientModuleName?: string; isPackageJsonImport?: true; + moduleSpecifier?: string; exportName: string; }