Fix duplicate auto-import completions (#42850)
* Fix duplicate auto-import completions * Update src/services/completions.ts Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com> Co-authored-by: Daniel Rosenwasser <DanielRosenwasser@users.noreply.github.com>
This commit is contained in:
Andrew Branch
GitHub
parent
05ee94fba3
commit
1fd71478f9
|
|
@ -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<string, true>, key: string | number): boolean;
|
||||
export function addToSeen<T>(seen: ESMap<string, T>, key: string | number, value: T): boolean;
|
||||
export function addToSeen<T>(seen: ESMap<string, T>, key: string | number, value: T = true as any): boolean {
|
||||
key = String(key);
|
||||
export function addToSeen<K>(seen: ESMap<K, true>, key: K): boolean;
|
||||
export function addToSeen<K, T>(seen: ESMap<K, T>, key: K, value: T): boolean;
|
||||
export function addToSeen<K, T>(seen: ESMap<K, T>, key: K, value: T = true as any): boolean {
|
||||
if (seen.has(key)) {
|
||||
return false;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -12,7 +12,7 @@ namespace ts.codefix {
|
|||
},
|
||||
fixIds: [fixId],
|
||||
getAllCodeActions: context => {
|
||||
const fixedExportDeclarations = new Map<string, true>();
|
||||
const fixedExportDeclarations = new Map<number, true>();
|
||||
return codeFixAll(context, errorCodes, (changes, diag) => {
|
||||
const exportSpecifier = getExportSpecifierForDiagnosticSpan(diag, context.sourceFile);
|
||||
if (exportSpecifier && addToSeen(fixedExportDeclarations, getNodeId(exportSpecifier.parent.parent))) {
|
||||
|
|
|
|||
|
|
@ -16,7 +16,7 @@ namespace ts.codefix {
|
|||
},
|
||||
fixIds: [fixId],
|
||||
getAllCodeActions: context => {
|
||||
const seen = new Map<string, true>();
|
||||
const seen = new Map<number, true>();
|
||||
return codeFixAll(context, errorCodes, (changes, diag) => {
|
||||
const nodes = getNodes(diag.file, diag.start);
|
||||
if (!nodes || !addToSeen(seen, getNodeId(nodes.insertBefore))) return;
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ namespace ts.codefix {
|
|||
},
|
||||
fixIds: [fixId],
|
||||
getAllCodeActions: context => {
|
||||
const seenClassDeclarations = new Map<string, true>();
|
||||
const seenClassDeclarations = new Map<number, true>();
|
||||
return codeFixAll(context, errorCodes, (changes, diag) => {
|
||||
const classDeclaration = getClass(diag.file, diag.start);
|
||||
if (addToSeen(seenClassDeclarations, getNodeId(classDeclaration))) {
|
||||
|
|
|
|||
|
|
@ -17,7 +17,7 @@ namespace ts.codefix {
|
|||
},
|
||||
fixIds: [fixId],
|
||||
getAllCodeActions(context) {
|
||||
const seenClassDeclarations = new Map<string, true>();
|
||||
const seenClassDeclarations = new Map<number, true>();
|
||||
return codeFixAll(context, errorCodes, (changes, diag) => {
|
||||
const classDeclaration = getClass(diag.file, diag.start);
|
||||
if (addToSeen(seenClassDeclarations, getNodeId(classDeclaration))) {
|
||||
|
|
|
|||
|
|
@ -15,7 +15,7 @@ namespace ts.codefix {
|
|||
fixIds: [fixId],
|
||||
getAllCodeActions(context) {
|
||||
const { sourceFile } = context;
|
||||
const seenClasses = new Map<string, true>(); // Ensure we only do this once per class.
|
||||
const seenClasses = new Map<number, true>(); // Ensure we only do this once per class.
|
||||
return codeFixAll(context, errorCodes, (changes, diag) => {
|
||||
const nodes = getNodes(diag.file, diag.start);
|
||||
if (!nodes) return;
|
||||
|
|
|
|||
|
|
@ -1645,7 +1645,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<string, true>()): boolean {
|
||||
function symbolCanBeReferencedAtTypeLocation(symbol: Symbol, seenModules = new Map<SymbolId, true>()): boolean {
|
||||
const sym = skipAlias(symbol.exportSymbol || symbol, typeChecker);
|
||||
return !!(sym.flags & SymbolFlags.Type) ||
|
||||
!!(sym.flags & SymbolFlags.Module) &&
|
||||
|
|
@ -1655,50 +1655,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(
|
||||
|
|
@ -1713,16 +1670,8 @@ namespace ts.Completions {
|
|||
|
||||
const startTime = timestamp();
|
||||
log(`getSymbolsFromOtherSourceFileExports: Recomputing list${detailsEntryId ? " for details entry" : ""}`);
|
||||
const seenResolvedModules = new Map<string, true>();
|
||||
const seenExports = new Map<string, true>();
|
||||
/** Bucket B */
|
||||
const aliasesToAlreadyIncludedSymbols = new Map<string, true>();
|
||||
/** Bucket C */
|
||||
const aliasesToReturnIfOriginalsAreMissing = new Map<string, { alias: Symbol, moduleSymbol: Symbol, isFromPackageJson: boolean }>();
|
||||
/** Bucket A */
|
||||
const results: AutoImportSuggestion[] = [];
|
||||
/** Ids present in `results` for faster lookup */
|
||||
const resultSymbolIds = new Map<string, true>();
|
||||
const seenResolvedModules = new Map<SymbolId, true>();
|
||||
const results = createMultiMap<SymbolId, AutoImportSuggestion>();
|
||||
|
||||
codefix.forEachExternalModuleToImportFrom(program, host, sourceFile, !detailsEntryId, /*useAutoImportProvider*/ true, (moduleSymbol, _, program, isFromPackageJson) => {
|
||||
// Perf -- ignore other modules if this is a request for details
|
||||
|
|
@ -1744,73 +1693,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;
|
||||
}
|
||||
|
||||
/**
|
||||
|
|
@ -2897,15 +2825,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
|
||||
|
|
|
|||
|
|
@ -231,7 +231,7 @@ namespace ts.FindAllReferences {
|
|||
}
|
||||
else {
|
||||
const queue = entries && [...entries];
|
||||
const seenNodes = new Map<string, true>();
|
||||
const seenNodes = new Map<number, true>();
|
||||
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<T>(symbol: Symbol, propertyName: string, checker: TypeChecker, cb: (symbol: Symbol) => T | undefined): T | undefined {
|
||||
const seen = new Map<string, true>();
|
||||
const seen = new Map<SymbolId, true>();
|
||||
return recur(symbol);
|
||||
|
||||
function recur(symbol: Symbol): T | undefined {
|
||||
|
|
|
|||
|
|
@ -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<string, { readonly node: ClassDeclaration | InterfaceDeclaration | ObjectLiteralExpression, readonly sourceFile: SourceFile }>(); // Set<ClassDeclaration> implemented as Map<node id, ClassDeclaration>
|
||||
private readonly classesWithNodesInsertedAtStart = new Map<number, { readonly node: ClassDeclaration | InterfaceDeclaration | ObjectLiteralExpression, readonly sourceFile: SourceFile }>(); // Set<ClassDeclaration> implemented as Map<node id, ClassDeclaration>
|
||||
private readonly deletedNodes: { readonly sourceFile: SourceFile, readonly node: Node | NodeArray<TypeParameterDeclaration> }[] = [];
|
||||
|
||||
public static fromContext(context: TextChangesContext): ChangeTracker {
|
||||
|
|
|
|||
|
|
@ -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 },
|
||||
|
|
|
|||
Loading…
Reference in a new issue