diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts index d91bbac3b741a..015cc5ed93ebe 100644 --- a/src/compiler/utilities.ts +++ b/src/compiler/utilities.ts @@ -5511,10 +5511,9 @@ namespace ts { } /** Add a value to a set, and return true if it wasn't already present. */ - export function addToSeen(seen: ESMap, key: string | number): boolean; - export function addToSeen(seen: ESMap, key: string | number, value: T): boolean; - export function addToSeen(seen: ESMap, key: string | number, value: T = true as any): boolean { - key = String(key); + export function addToSeen(seen: ESMap, key: K): boolean; + export function addToSeen(seen: ESMap, key: K, value: T): boolean; + export function addToSeen(seen: ESMap, key: K, value: T = true as any): boolean { if (seen.has(key)) { return false; } diff --git a/src/harness/fourslashImpl.ts b/src/harness/fourslashImpl.ts index d86f2e904fe65..454240b7a726d 100644 --- a/src/harness/fourslashImpl.ts +++ b/src/harness/fourslashImpl.ts @@ -933,7 +933,7 @@ namespace FourSlash { assert.equal(actual.sortText, expected.sortText || ts.Completions.SortText.LocationPriority, this.messageAtLastKnownMarker(`Actual entry: ${JSON.stringify(actual)}`)); if (expected.text !== undefined) { - const actualDetails = this.getCompletionEntryDetails(actual.name, actual.source)!; + const actualDetails = ts.Debug.checkDefined(this.getCompletionEntryDetails(actual.name, actual.source), `No completion details available for name '${actual.name}' and source '${actual.source}'`); assert.equal(ts.displayPartsToString(actualDetails.displayParts), expected.text, "Expected 'text' property to match 'displayParts' string"); assert.equal(ts.displayPartsToString(actualDetails.documentation), expected.documentation || "", "Expected 'documentation' property to match 'documentation' display parts string"); // TODO: GH#23587 diff --git a/src/services/codefixes/convertToTypeOnlyExport.ts b/src/services/codefixes/convertToTypeOnlyExport.ts index 117aca7b983c8..3f7a0d0983db1 100644 --- a/src/services/codefixes/convertToTypeOnlyExport.ts +++ b/src/services/codefixes/convertToTypeOnlyExport.ts @@ -12,7 +12,7 @@ namespace ts.codefix { }, fixIds: [fixId], getAllCodeActions: context => { - const fixedExportDeclarations = new Map(); + const fixedExportDeclarations = new Map(); return codeFixAll(context, errorCodes, (changes, diag) => { const exportSpecifier = getExportSpecifierForDiagnosticSpan(diag, context.sourceFile); if (exportSpecifier && addToSeen(fixedExportDeclarations, getNodeId(exportSpecifier.parent.parent))) { diff --git a/src/services/codefixes/fixAwaitInSyncFunction.ts b/src/services/codefixes/fixAwaitInSyncFunction.ts index 84c054fdb887e..bfd68e05d0abf 100644 --- a/src/services/codefixes/fixAwaitInSyncFunction.ts +++ b/src/services/codefixes/fixAwaitInSyncFunction.ts @@ -16,7 +16,7 @@ namespace ts.codefix { }, fixIds: [fixId], getAllCodeActions: context => { - const seen = new Map(); + const seen = new Map(); return codeFixAll(context, errorCodes, (changes, diag) => { const nodes = getNodes(diag.file, diag.start); if (!nodes || !addToSeen(seen, getNodeId(nodes.insertBefore))) return; diff --git a/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts b/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts index ae1a2448560de..9e8313f357b14 100644 --- a/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts +++ b/src/services/codefixes/fixClassDoesntImplementInheritedAbstractMember.ts @@ -15,7 +15,7 @@ namespace ts.codefix { }, fixIds: [fixId], getAllCodeActions: context => { - const seenClassDeclarations = new Map(); + const seenClassDeclarations = new Map(); return codeFixAll(context, errorCodes, (changes, diag) => { const classDeclaration = getClass(diag.file, diag.start); if (addToSeen(seenClassDeclarations, getNodeId(classDeclaration))) { diff --git a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts index 2ae9f1b94a9b3..01410672b3f28 100644 --- a/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts +++ b/src/services/codefixes/fixClassIncorrectlyImplementsInterface.ts @@ -17,7 +17,7 @@ namespace ts.codefix { }, fixIds: [fixId], getAllCodeActions(context) { - const seenClassDeclarations = new Map(); + const seenClassDeclarations = new Map(); return codeFixAll(context, errorCodes, (changes, diag) => { const classDeclaration = getClass(diag.file, diag.start); if (addToSeen(seenClassDeclarations, getNodeId(classDeclaration))) { diff --git a/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts b/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts index dc8f261cc266b..224109f6d1237 100644 --- a/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts +++ b/src/services/codefixes/fixClassSuperMustPrecedeThisAccess.ts @@ -15,7 +15,7 @@ namespace ts.codefix { fixIds: [fixId], getAllCodeActions(context) { const { sourceFile } = context; - const seenClasses = new Map(); // Ensure we only do this once per class. + const seenClasses = new Map(); // Ensure we only do this once per class. return codeFixAll(context, errorCodes, (changes, diag) => { const nodes = getNodes(diag.file, diag.start); if (!nodes) return; diff --git a/src/services/completions.ts b/src/services/completions.ts index a3c957442cb0f..171624df345a2 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -1609,7 +1609,7 @@ namespace ts.Completions { } /** True if symbol is a type or a module containing at least one type. */ - function symbolCanBeReferencedAtTypeLocation(symbol: Symbol, seenModules = new Map()): boolean { + function symbolCanBeReferencedAtTypeLocation(symbol: Symbol, seenModules = new Map()): boolean { const sym = skipAlias(symbol.exportSymbol || symbol, typeChecker); return !!(sym.flags & SymbolFlags.Type) || !!(sym.flags & SymbolFlags.Module) && @@ -1619,50 +1619,7 @@ namespace ts.Completions { /** * Gathers symbols that can be imported from other files, de-duplicating along the way. Symbols can be "duplicates" - * if re-exported from another module, e.g. `export { foo } from "./a"`. That syntax creates a fresh symbol, but - * it’s just an alias to the first, and both have the same name, so we generally want to filter those aliases out, - * if and only if the the first can be imported (it may be excluded due to package.json filtering in - * `codefix.forEachExternalModuleToImportFrom`). - * - * Example. Imagine a chain of node_modules re-exporting one original symbol: - * - * ```js - * node_modules/x/index.js node_modules/y/index.js node_modules/z/index.js - * +-----------------------+ +--------------------------+ +--------------------------+ - * | | | | | | - * | export const foo = 0; | <--- | export { foo } from 'x'; | <--- | export { foo } from 'y'; | - * | | | | | | - * +-----------------------+ +--------------------------+ +--------------------------+ - * ``` - * - * Also imagine three buckets, which we’ll reference soon: - * - * ```md - * | | | | | | - * | **Bucket A** | | **Bucket B** | | **Bucket C** | - * | Symbols to | | Aliases to symbols | | Symbols to return | - * | definitely | | in Buckets A or C | | if nothing better | - * | return | | (don’t return these) | | comes along | - * |__________________| |______________________| |___________________| - * ``` - * - * We _probably_ want to show `foo` from 'x', but not from 'y' or 'z'. However, if 'x' is not in a package.json, it - * will not appear in a `forEachExternalModuleToImportFrom` iteration. Furthermore, the order of iterations is not - * guaranteed, as it is host-dependent. Therefore, when presented with the symbol `foo` from module 'y' alone, we - * may not be sure whether or not it should go in the list. So, we’ll take the following steps: - * - * 1. Resolve alias `foo` from 'y' to the export declaration in 'x', get the symbol there, and see if that symbol is - * already in Bucket A (symbols we already know will be returned). If it is, put `foo` from 'y' in Bucket B - * (symbols that are aliases to symbols in Bucket A). If it’s not, put it in Bucket C. - * 2. Next, imagine we see `foo` from module 'z'. Again, we resolve the alias to the nearest export, which is in 'y'. - * At this point, if that nearest export from 'y' is in _any_ of the three buckets, we know the symbol in 'z' - * should never be returned in the final list, so put it in Bucket B. - * 3. Next, imagine we see `foo` from module 'x', the original. Syntactically, it doesn’t look like a re-export, so - * we can just check Bucket C to see if we put any aliases to the original in there. If they exist, throw them out. - * Put this symbol in Bucket A. - * 4. After we’ve iterated through every symbol of every module, any symbol left in Bucket C means that step 3 didn’t - * occur for that symbol---that is, the original symbol is not in Bucket A, so we should include the alias. Move - * everything from Bucket C to Bucket A. + * if re-exported from another module by the same name, e.g. `export { foo } from "./a"`. */ function getSymbolsFromOtherSourceFileExports(target: ScriptTarget, host: LanguageServiceHost): readonly AutoImportSuggestion[] { const cached = importSuggestionsCache && importSuggestionsCache.get( @@ -1677,16 +1634,8 @@ namespace ts.Completions { const startTime = timestamp(); log(`getSymbolsFromOtherSourceFileExports: Recomputing list${detailsEntryId ? " for details entry" : ""}`); - const seenResolvedModules = new Map(); - const seenExports = new Map(); - /** Bucket B */ - const aliasesToAlreadyIncludedSymbols = new Map(); - /** Bucket C */ - const aliasesToReturnIfOriginalsAreMissing = new Map(); - /** Bucket A */ - const results: AutoImportSuggestion[] = []; - /** Ids present in `results` for faster lookup */ - const resultSymbolIds = new Map(); + const seenResolvedModules = new Map(); + const results = createMultiMap(); codefix.forEachExternalModuleToImportFrom(program, host, sourceFile, !detailsEntryId, /*useAutoImportProvider*/ true, (moduleSymbol, _, program, isFromPackageJson) => { // Perf -- ignore other modules if this is a request for details @@ -1708,73 +1657,52 @@ namespace ts.Completions { } for (const symbol of typeChecker.getExportsAndPropertiesOfModule(moduleSymbol)) { - const symbolId = getSymbolId(symbol).toString(); - // `getExportsAndPropertiesOfModule` can include duplicates - if (!addToSeen(seenExports, symbolId)) { - continue; - } // If this is `export { _break as break };` (a keyword) -- skip this and prefer the keyword completion. if (some(symbol.declarations, d => isExportSpecifier(d) && !!d.propertyName && isIdentifierANonContextualKeyword(d.name))) { continue; } - // If `symbol.parent !== moduleSymbol`, this is an `export * from "foo"` re-export. Those don't create new symbols. - const isExportStarFromReExport = typeChecker.getMergedSymbol(symbol.parent!) !== resolvedModuleSymbol; - // If `!!d.parent.parent.moduleSpecifier`, this is `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check). - if (isExportStarFromReExport || some(symbol.declarations, d => isExportSpecifier(d) && !d.propertyName && !!d.parent.parent.moduleSpecifier)) { - // Walk the export chain back one module (step 1 or 2 in diagrammed example). - // Or, in the case of `export * from "foo"`, `symbol` already points to the original export, so just use that. - const nearestExportSymbol = isExportStarFromReExport ? symbol : getNearestExportSymbol(symbol); - if (!nearestExportSymbol) continue; - const nearestExportSymbolId = getSymbolId(nearestExportSymbol).toString(); - const symbolHasBeenSeen = resultSymbolIds.has(nearestExportSymbolId) || aliasesToAlreadyIncludedSymbols.has(nearestExportSymbolId); - if (!symbolHasBeenSeen) { - aliasesToReturnIfOriginalsAreMissing.set(nearestExportSymbolId, { alias: symbol, moduleSymbol, isFromPackageJson }); - aliasesToAlreadyIncludedSymbols.set(symbolId, true); - } - else { - // Perf - we know this symbol is an alias to one that’s already covered in `symbols`, so store it here - // in case another symbol re-exports this one; that way we can short-circuit as soon as we see this symbol id. - addToSeen(aliasesToAlreadyIncludedSymbols, symbolId); - } - } - else { - // This is not a re-export, so see if we have any aliases pending and remove them (step 3 in diagrammed example) - aliasesToReturnIfOriginalsAreMissing.delete(symbolId); - pushSymbol(symbol, moduleSymbol, isFromPackageJson, /*skipFilter*/ false); - } + pushSymbol(symbol, moduleSymbol, isFromPackageJson, /*skipFilter*/ false); } }); - // By this point, any potential duplicates that were actually duplicates have been - // removed, so the rest need to be added. (Step 4 in diagrammed example) - aliasesToReturnIfOriginalsAreMissing.forEach(({ alias, moduleSymbol, isFromPackageJson }) => pushSymbol(alias, moduleSymbol, isFromPackageJson, /*skipFilter*/ false)); log(`getSymbolsFromOtherSourceFileExports: ${timestamp() - startTime}`); - return results; + return flatten(arrayFrom(results.values())); function pushSymbol(symbol: Symbol, moduleSymbol: Symbol, isFromPackageJson: boolean, skipFilter: boolean) { const isDefaultExport = symbol.escapedName === InternalSymbolName.Default; + const nonLocalSymbol = symbol; if (isDefaultExport) { symbol = getLocalSymbolForExportDefault(symbol) || symbol; } if (typeChecker.isUndefinedSymbol(symbol)) { return; } - addToSeen(resultSymbolIds, getSymbolId(symbol)); - const origin: SymbolOriginInfoExport = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport, isFromPackageJson }; - results.push({ - symbol, - symbolName: getNameForExportedSymbol(symbol, target), - origin, - skipFilter, - }); + const original = skipAlias(nonLocalSymbol, typeChecker); + const symbolName = getNameForExportedSymbol(symbol, target); + const existingSuggestions = results.get(getSymbolId(original)); + if (!some(existingSuggestions, s => s.symbolName === symbolName && moduleSymbolsAreDuplicateOrigins(moduleSymbol, s.origin.moduleSymbol))) { + const origin: SymbolOriginInfoExport = { kind: SymbolOriginInfoKind.Export, moduleSymbol, isDefaultExport, isFromPackageJson }; + results.add(getSymbolId(original), { + symbol, + symbolName, + origin, + skipFilter, + }); + } } } - function getNearestExportSymbol(fromSymbol: Symbol) { - return findAlias(typeChecker, fromSymbol, alias => { - return some(alias.declarations, d => isExportSpecifier(d) || !!d.localSymbol); - }); + /** + * Determines whether a module symbol is redundant with another for purposes of offering + * auto-import completions for exports of the same symbol. Exports of the same symbol + * will not be offered from different external modules, but they will be offered from + * different ambient modules. + */ + function moduleSymbolsAreDuplicateOrigins(a: Symbol, b: Symbol) { + const ambientNameA = pathIsBareSpecifier(stripQuotes(a.name)) ? a.name : undefined; + const ambientNameB = pathIsBareSpecifier(stripQuotes(b.name)) ? b.name : undefined; + return ambientNameA === ambientNameB; } /** @@ -2861,15 +2789,6 @@ namespace ts.Completions { return nodeIsMissing(left); } - function findAlias(typeChecker: TypeChecker, symbol: Symbol, predicate: (symbol: Symbol) => boolean): Symbol | undefined { - let currentAlias: Symbol | undefined = symbol; - while (currentAlias.flags & SymbolFlags.Alias && (currentAlias = typeChecker.getImmediateAliasedSymbol(currentAlias))) { - if (predicate(currentAlias)) { - return currentAlias; - } - } - } - /** Determines if a type is exactly the same type resolved by the global 'self', 'global', or 'globalThis'. */ function isProbablyGlobalType(type: Type, sourceFile: SourceFile, checker: TypeChecker) { // The type of `self` and `window` is the same in lib.dom.d.ts, but `window` does not exist in diff --git a/src/services/findAllReferences.ts b/src/services/findAllReferences.ts index 12a97433615ca..15e13674afcd8 100644 --- a/src/services/findAllReferences.ts +++ b/src/services/findAllReferences.ts @@ -231,7 +231,7 @@ namespace ts.FindAllReferences { } else { const queue = entries && [...entries]; - const seenNodes = new Map(); + const seenNodes = new Map(); while (queue && queue.length) { const entry = queue.shift() as NodeEntry; if (!addToSeen(seenNodes, getNodeId(entry.node))) { @@ -2154,7 +2154,7 @@ namespace ts.FindAllReferences { * The value of previousIterationSymbol is undefined when the function is first called. */ function getPropertySymbolsFromBaseTypes(symbol: Symbol, propertyName: string, checker: TypeChecker, cb: (symbol: Symbol) => T | undefined): T | undefined { - const seen = new Map(); + const seen = new Map(); return recur(symbol); function recur(symbol: Symbol): T | undefined { diff --git a/src/services/textChanges.ts b/src/services/textChanges.ts index 566c4c8c0f272..adbcfe68bef42 100644 --- a/src/services/textChanges.ts +++ b/src/services/textChanges.ts @@ -261,7 +261,7 @@ namespace ts.textChanges { export class ChangeTracker { private readonly changes: Change[] = []; private readonly newFiles: { readonly oldFile: SourceFile | undefined, readonly fileName: string, readonly statements: readonly (Statement | SyntaxKind.NewLineTrivia)[] }[] = []; - private readonly classesWithNodesInsertedAtStart = new Map(); // Set implemented as Map + private readonly classesWithNodesInsertedAtStart = new Map(); // Set implemented as Map private readonly deletedNodes: { readonly sourceFile: SourceFile, readonly node: Node | NodeArray }[] = []; public static fromContext(context: TextChangesContext): ChangeTracker { diff --git a/tests/cases/fourslash/completionsImport_reExportDefault.ts b/tests/cases/fourslash/completionsImport_reExportDefault.ts index 3ec987a7215a8..bcce25976f32f 100644 --- a/tests/cases/fourslash/completionsImport_reExportDefault.ts +++ b/tests/cases/fourslash/completionsImport_reExportDefault.ts @@ -28,16 +28,6 @@ verify.completions({ hasAction: true, sortText: completion.SortText.AutoImportSuggestions }, - { - name: "foo", - source: "/a/index", - sourceDisplay: "./a", - text: "(alias) function foo(): void\nexport foo", - kind: "alias", - kindModifiers: "export", - hasAction: true, - sortText: completion.SortText.AutoImportSuggestions - }, ...completion.globalKeywords, ], preferences: { includeCompletionsForModuleExports: true },