diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index d88f0e0854e0f..7ba4e94902db8 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -783,13 +783,13 @@ namespace FourSlash { }); } - public verifyCompletionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) { + public verifyCompletionListContains(entryId: ts.Completions.CompletionEntryIdentifier, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) { const completions = this.getCompletionListAtCaret(); if (completions) { - this.assertItemInCompletionList(completions.entries, symbol, text, documentation, kind, spanIndex, hasAction); + this.assertItemInCompletionList(completions.entries, entryId, text, documentation, kind, spanIndex, hasAction); } else { - this.raiseError(`No completions at position '${this.currentCaretPosition}' when looking for '${symbol}'.`); + this.raiseError(`No completions at position '${this.currentCaretPosition}' when looking for '${JSON.stringify(entryId)}'.`); } } @@ -804,7 +804,7 @@ namespace FourSlash { * @param expectedKind the kind of symbol (see ScriptElementKind) * @param spanIndex the index of the range that the completion item's replacement text span should match */ - public verifyCompletionListDoesNotContain(symbol: string, expectedText?: string, expectedDocumentation?: string, expectedKind?: string, spanIndex?: number) { + public verifyCompletionListDoesNotContain(entryId: ts.Completions.CompletionEntryIdentifier, expectedText?: string, expectedDocumentation?: string, expectedKind?: string, spanIndex?: number) { const that = this; let replacementSpan: ts.TextSpan; if (spanIndex !== undefined) { @@ -833,14 +833,14 @@ namespace FourSlash { const completions = this.getCompletionListAtCaret(); if (completions) { - let filterCompletions = completions.entries.filter(e => e.name === symbol); + let filterCompletions = completions.entries.filter(e => e.name === entryId.name && e.source === entryId.source); filterCompletions = expectedKind ? filterCompletions.filter(e => e.kind === expectedKind) : filterCompletions; filterCompletions = filterCompletions.filter(filterByTextOrDocumentation); if (filterCompletions.length !== 0) { // After filtered using all present criterion, if there are still symbol left in the list // then these symbols must meet the criterion for Not supposed to be in the list. So we // raise an error - let error = "Completion list did contain \'" + symbol + "\'."; + let error = `Completion list did contain '${JSON.stringify(entryId)}\'.`; const details = this.getCompletionEntryDetails(filterCompletions[0].name); if (expectedText) { error += "Expected text: " + expectedText + " to equal: " + ts.displayPartsToString(details.displayParts) + "."; @@ -1130,8 +1130,8 @@ Actual: ${stringify(fullActual)}`); return this.languageService.getCompletionsAtPosition(this.activeFile.fileName, this.currentCaretPosition); } - private getCompletionEntryDetails(entryName: string) { - return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName, this.formatCodeSettings); + private getCompletionEntryDetails(entryName: string, source?: string) { + return this.languageService.getCompletionEntryDetails(this.activeFile.fileName, this.currentCaretPosition, entryName, this.formatCodeSettings, source); } private getReferencesAtCaret() { @@ -1640,7 +1640,7 @@ Actual: ${stringify(fullActual)}`); const longestNameLength = max(entries, m => m.name.length); const longestKindLength = max(entries, m => m.kind.length); entries.sort((m, n) => m.sortText > n.sortText ? 1 : m.sortText < n.sortText ? -1 : m.name > n.name ? 1 : m.name < n.name ? -1 : 0); - const membersString = entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers}`).join("\n"); + const membersString = entries.map(m => `${pad(m.name, longestNameLength)} ${pad(m.kind, longestKindLength)} ${m.kindModifiers} ${m.source === undefined ? "" : m.source}`).join("\n"); Harness.IO.log(membersString); } @@ -2296,13 +2296,13 @@ Actual: ${stringify(fullActual)}`); public applyCodeActionFromCompletion(markerName: string, options: FourSlashInterface.VerifyCompletionActionOptions) { this.goToMarker(markerName); - const actualCompletion = this.getCompletionListAtCaret().entries.find(e => e.name === options.name); + const actualCompletion = this.getCompletionListAtCaret().entries.find(e => e.name === options.name && e.source === options.source); if (!actualCompletion.hasAction) { this.raiseError(`Completion for ${options.name} does not have an associated action.`); } - const details = this.getCompletionEntryDetails(options.name); + const details = this.getCompletionEntryDetails(options.name, actualCompletion.source); if (details.codeActions.length !== 1) { this.raiseError(`Expected one code action, got ${details.codeActions.length}`); } @@ -2984,7 +2984,7 @@ Actual: ${stringify(fullActual)}`); private assertItemInCompletionList( items: ts.CompletionEntry[], - name: string, + entryId: ts.Completions.CompletionEntryIdentifier, text: string | undefined, documentation: string | undefined, kind: string | undefined, @@ -2992,25 +2992,27 @@ Actual: ${stringify(fullActual)}`); hasAction: boolean | undefined, ) { for (const item of items) { - if (item.name === name) { - if (documentation !== undefined || text !== undefined) { - const details = this.getCompletionEntryDetails(item.name); + if (item.name === entryId.name && item.source === entryId.source) { + if (documentation !== undefined || text !== undefined || entryId.source !== undefined) { + const details = this.getCompletionEntryDetails(item.name, item.source); if (documentation !== undefined) { - assert.equal(ts.displayPartsToString(details.documentation), documentation, this.assertionMessageAtLastKnownMarker("completion item documentation for " + name)); + assert.equal(ts.displayPartsToString(details.documentation), documentation, this.assertionMessageAtLastKnownMarker("completion item documentation for " + entryId)); } if (text !== undefined) { - assert.equal(ts.displayPartsToString(details.displayParts), text, this.assertionMessageAtLastKnownMarker("completion item detail text for " + name)); + assert.equal(ts.displayPartsToString(details.displayParts), text, this.assertionMessageAtLastKnownMarker("completion item detail text for " + entryId)); } + + assert.deepEqual(details.source, entryId.source === undefined ? undefined : [ts.textPart(entryId.source)]); } if (kind !== undefined) { - assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + name)); + assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId)); } if (spanIndex !== undefined) { const span = this.getTextSpanForRangeAtIndex(spanIndex); - assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + name)); + assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId)); } assert.equal(item.hasAction, hasAction); @@ -3021,7 +3023,7 @@ Actual: ${stringify(fullActual)}`); const itemsString = items.map(item => stringify({ name: item.name, kind: item.kind })).join(",\n"); - this.raiseError(`Expected "${stringify({ name, text, documentation, kind })}" to be in list [${itemsString}]`); + this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`); } private findFile(indexOrName: any) { @@ -3732,12 +3734,15 @@ namespace FourSlashInterface { // Verifies the completion list contains the specified symbol. The // completion list is brought up if necessary - public completionListContains(symbol: string, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) { + public completionListContains(entryId: string | ts.Completions.CompletionEntryIdentifier, text?: string, documentation?: string, kind?: string, spanIndex?: number, hasAction?: boolean) { + if (typeof entryId === "string") { + entryId = { name: entryId, source: undefined }; + } if (this.negative) { - this.state.verifyCompletionListDoesNotContain(symbol, text, documentation, kind, spanIndex); + this.state.verifyCompletionListDoesNotContain(entryId, text, documentation, kind, spanIndex); } else { - this.state.verifyCompletionListContains(symbol, text, documentation, kind, spanIndex, hasAction); + this.state.verifyCompletionListContains(entryId, text, documentation, kind, spanIndex, hasAction); } } @@ -4492,6 +4497,7 @@ namespace FourSlashInterface { export interface VerifyCompletionActionOptions extends NewContentOptions { name: string; + source?: string; description: string; } } diff --git a/src/server/protocol.ts b/src/server/protocol.ts index 35e9f20d05825..6092f235942ed 100644 --- a/src/server/protocol.ts +++ b/src/server/protocol.ts @@ -1625,7 +1625,12 @@ namespace ts.server.protocol { /** * Names of one or more entries for which to obtain details. */ - entryNames: string[]; + entryNames: (string | CompletionEntryIdentifier)[]; + } + + export interface CompletionEntryIdentifier { + name: string; + source: string; } /** @@ -1685,6 +1690,10 @@ namespace ts.server.protocol { * made to avoid errors. The CompletionEntryDetails will have these actions. */ hasAction?: true; + /** + * Identifier (not necessarily human-readable) identifying where this completion came from. + */ + source?: string; } /** @@ -1722,6 +1731,11 @@ namespace ts.server.protocol { * The associated code actions for this entry */ codeActions?: CodeAction[]; + + /** + * Human-readable description of the `source` from the CompletionEntry. + */ + source?: SymbolDisplayPart[]; } export interface CompletionsResponse extends Response { diff --git a/src/server/session.ts b/src/server/session.ts index 9f249790005eb..7ada8d2ff9c82 100644 --- a/src/server/session.ts +++ b/src/server/session.ts @@ -1188,10 +1188,10 @@ namespace ts.server { if (simplifiedResult) { return mapDefined(completions && completions.entries, entry => { if (completions.isMemberCompletion || (entry.name.toLowerCase().indexOf(prefix.toLowerCase()) === 0)) { - const { name, kind, kindModifiers, sortText, replacementSpan, hasAction } = entry; + const { name, kind, kindModifiers, sortText, replacementSpan, hasAction, source } = entry; const convertedSpan = replacementSpan ? this.decorateSpan(replacementSpan, scriptInfo) : undefined; // Use `hasAction || undefined` to avoid serializing `false`. - return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined }; + return { name, kind, kindModifiers, sortText, replacementSpan: convertedSpan, hasAction: hasAction || undefined, source }; } }).sort((a, b) => compareStrings(a.name, b.name)); } @@ -1206,8 +1206,9 @@ namespace ts.server { const position = this.getPosition(args, scriptInfo); const formattingOptions = project.projectService.getFormatCodeOptions(file); - return mapDefined(args.entryNames, entryName => { - const details = project.getLanguageService().getCompletionEntryDetails(file, position, entryName, formattingOptions); + return mapDefined(args.entryNames, entryName => { + const { name, source } = typeof entryName === "string" ? { name: entryName, source: undefined } : entryName; + const details = project.getLanguageService().getCompletionEntryDetails(file, position, name, formattingOptions, source); if (details) { const mappedCodeActions = map(details.codeActions, action => this.mapCodeAction(action, scriptInfo)); return { ...details, codeActions: mappedCodeActions }; diff --git a/src/services/codefixes/importFixes.ts b/src/services/codefixes/importFixes.ts index 1167f117c330e..3f0d27e5b070f 100644 --- a/src/services/codefixes/importFixes.ts +++ b/src/services/codefixes/importFixes.ts @@ -249,7 +249,11 @@ namespace ts.codefix { const lastImportDeclaration = findLast(sourceFile.statements, isAnyImportSyntax); const moduleSpecifierWithoutQuotes = stripQuotes(moduleSpecifier); - const importDecl = createImportDeclaration(/*decorators*/ undefined, /*modifiers*/ undefined, createImportClauseOfKind(kind, symbolName), createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes)); + const importDecl = createImportDeclaration( + /*decorators*/ undefined, + /*modifiers*/ undefined, + createImportClauseOfKind(kind, symbolName), + createStringLiteralWithQuoteStyle(sourceFile, moduleSpecifierWithoutQuotes)); const changes = ChangeTracker.with(context, changeTracker => { if (lastImportDeclaration) { changeTracker.insertNodeAfter(sourceFile, lastImportDeclaration, importDecl, { suffix: newLineCharacter }); @@ -279,13 +283,14 @@ namespace ts.codefix { } function createImportClauseOfKind(kind: ImportKind, symbolName: string) { + const id = createIdentifier(symbolName); switch (kind) { case ImportKind.Default: - return createImportClause(createIdentifier(symbolName), /*namedBindings*/ undefined); + return createImportClause(id, /*namedBindings*/ undefined); case ImportKind.Namespace: - return createImportClause(/*name*/ undefined, createNamespaceImport(createIdentifier(symbolName))); + return createImportClause(/*name*/ undefined, createNamespaceImport(id)); case ImportKind.Named: - return createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, createIdentifier(symbolName))])); + return createImportClause(/*name*/ undefined, createNamedImports([createImportSpecifier(/*propertyName*/ undefined, id)])); default: Debug.assertNever(kind); } @@ -529,7 +534,7 @@ namespace ts.codefix { declarations: ReadonlyArray): ImportCodeAction { const fromExistingImport = firstDefined(declarations, declaration => { if (declaration.kind === SyntaxKind.ImportDeclaration && declaration.importClause) { - const changes = tryUpdateExistingImport(ctx, ctx.kind, declaration.importClause); + const changes = tryUpdateExistingImport(ctx, declaration.importClause); if (changes) { const moduleSpecifierWithoutQuotes = stripQuotes(declaration.moduleSpecifier.getText()); return createCodeAction( @@ -559,8 +564,8 @@ namespace ts.codefix { return expression && isStringLiteral(expression) ? expression.text : undefined; } - function tryUpdateExistingImport(context: SymbolContext, kind: ImportKind, importClause: ImportClause): FileTextChanges[] | undefined { - const { symbolName, sourceFile } = context; + function tryUpdateExistingImport(context: SymbolContext & { kind: ImportKind }, importClause: ImportClause): FileTextChanges[] | undefined { + const { symbolName, sourceFile, kind } = context; const { name, namedBindings } = importClause; switch (kind) { case ImportKind.Default: diff --git a/src/services/completions.ts b/src/services/completions.ts index 7afa85ce26df4..29b6335a61fbf 100644 --- a/src/services/completions.ts +++ b/src/services/completions.ts @@ -12,7 +12,7 @@ namespace ts.Completions { * Map from symbol id -> SymbolOriginInfo. * Only populated for symbols that come from other modules. */ - type SymbolOriginInfoMap = SymbolOriginInfo[]; + type SymbolOriginInfoMap = (SymbolOriginInfo | undefined)[]; const enum KeywordCompletionFilters { None, @@ -100,7 +100,7 @@ namespace ts.Completions { function getJavaScriptCompletionEntries( sourceFile: SourceFile, position: number, - uniqueNames: Map, + uniqueNames: Map<{}>, target: ScriptTarget, entries: Push): void { getNameTable(sourceFile).forEach((pos, name) => { @@ -127,7 +127,15 @@ namespace ts.Completions { }); } - function createCompletionEntry(symbol: Symbol, location: Node, performCharacterChecks: boolean, typeChecker: TypeChecker, target: ScriptTarget, allowStringLiteral: boolean): CompletionEntry { + function createCompletionEntry( + symbol: Symbol, + location: Node, + performCharacterChecks: boolean, + typeChecker: TypeChecker, + target: ScriptTarget, + allowStringLiteral: boolean, + origin: SymbolOriginInfo, + ): CompletionEntry | undefined { // Try to get a valid display name for this symbol, if we could not find one, then ignore it. // We would like to only show things that can be added after a dot, so for instance numeric properties can // not be accessed with a dot (a.1 <- invalid) @@ -149,9 +157,15 @@ namespace ts.Completions { kind: SymbolDisplay.getSymbolKind(typeChecker, symbol, location), kindModifiers: SymbolDisplay.getSymbolModifiers(symbol), sortText: "0", + source: getSourceFromOrigin(origin), + hasAction: origin === undefined ? undefined : true, }; } + function getSourceFromOrigin(origin: SymbolOriginInfo | undefined): string | undefined { + return origin && stripQuotes(origin.moduleSymbol.name); + } + function getCompletionEntriesFromSymbols( symbols: ReadonlyArray, entries: Push, @@ -164,25 +178,35 @@ namespace ts.Completions { symbolToOriginInfoMap?: SymbolOriginInfoMap, ): Map { const start = timestamp(); - const uniqueNames = createMap(); + // Tracks unique names. + // We don't set this for global variables or completions from external module exports, because we can have multiple of those. + // Based on the order we add things we will always see locals first, then globals, then module exports. + // So adding a completion for a local will prevent us from adding completions for external module exports sharing the same name. + const uniques = createMap(); if (symbols) { for (const symbol of symbols) { - const entry = createCompletionEntry(symbol, location, performCharacterChecks, typeChecker, target, allowStringLiteral); - if (entry) { - const id = entry.name; - if (!uniqueNames.has(id)) { - if (symbolToOriginInfoMap && symbolToOriginInfoMap[getUniqueSymbolId(symbol, typeChecker)]) { - entry.hasAction = true; - } - entries.push(entry); - uniqueNames.set(id, true); - } + const origin = symbolToOriginInfoMap ? symbolToOriginInfoMap[getSymbolId(symbol)] : undefined; + const entry = createCompletionEntry(symbol, location, performCharacterChecks, typeChecker, target, allowStringLiteral, origin); + if (!entry) { + continue; + } + + const { name } = entry; + if (uniques.has(name)) { + continue; } + + // Latter case tests whether this is a global variable. + if (!origin && !(symbol.parent === undefined && !some(symbol.declarations, d => d.getSourceFile() === location.getSourceFile()))) { + uniques.set(name, true); + } + + entries.push(entry); } } log("getCompletionsAtPosition: getCompletionEntriesFromSymbols: " + (timestamp() - start)); - return uniqueNames; + return uniques; } function getStringLiteralCompletionEntries(sourceFile: SourceFile, position: number, typeChecker: TypeChecker, compilerOptions: CompilerOptions, host: LanguageServiceHost, log: Log): CompletionInfo | undefined { @@ -329,43 +353,60 @@ namespace ts.Completions { } } + function getSymbolCompletionFromEntryId( + typeChecker: TypeChecker, + log: (message: string) => void, + compilerOptions: CompilerOptions, + sourceFile: SourceFile, + position: number, + { name, source }: CompletionEntryIdentifier, + allSourceFiles: ReadonlyArray, + ): { symbol: Symbol, location: Node, symbolToOriginInfoMap: SymbolOriginInfoMap } | undefined { + const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); + if (!completionData) { + return undefined; + } + + const { symbols, location, allowStringLiteral, symbolToOriginInfoMap } = completionData; + // Find the symbol with the matching entry name. + // We don't need to perform character checks here because we're only comparing the + // name against 'entryName' (which is known to be good), not building a new + // completion entry. + const symbol = find(symbols, s => + getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === name + && getSourceFromOrigin(symbolToOriginInfoMap[getSymbolId(s)]) === source); + return symbol && { symbol, location, symbolToOriginInfoMap }; + } + + export interface CompletionEntryIdentifier { + name: string; + source?: string; + } + export function getCompletionEntryDetails( typeChecker: TypeChecker, log: (message: string) => void, compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, - name: string, + entryId: CompletionEntryIdentifier, allSourceFiles: ReadonlyArray, host: LanguageServiceHost, rulesProvider: formatting.RulesProvider, ): CompletionEntryDetails { - + const { name, source } = entryId; // Compute all the completion symbols again. - const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); - if (completionData) { - const { symbols, location, allowStringLiteral, symbolToOriginInfoMap } = completionData; - - // Find the symbol with the matching entry name. - // We don't need to perform character checks here because we're only comparing the - // name against 'entryName' (which is known to be good), not building a new - // completion entry. - const symbol = find(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === name); - - if (symbol) { - const codeActions = getCompletionEntryCodeActions(symbolToOriginInfoMap, symbol, typeChecker, host, compilerOptions, sourceFile, rulesProvider); - const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); - const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); - return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions }; - } + const symbolCompletion = getSymbolCompletionFromEntryId(typeChecker, log, compilerOptions, sourceFile, position, entryId, allSourceFiles); + if (symbolCompletion) { + const { symbol, location, symbolToOriginInfoMap } = symbolCompletion; + const codeActions = getCompletionEntryCodeActions(symbolToOriginInfoMap, symbol, typeChecker, host, compilerOptions, sourceFile, rulesProvider); + const kindModifiers = SymbolDisplay.getSymbolModifiers(symbol); + const { displayParts, documentation, symbolKind, tags } = SymbolDisplay.getSymbolDisplayPartsDocumentationAndSymbolKind(typeChecker, symbol, sourceFile, location, location, SemanticMeaning.All); + return { name, kindModifiers, kind: symbolKind, displayParts, documentation, tags, codeActions, source: source === undefined ? undefined : [textPart(source)] }; } // Didn't find a symbol with this name. See if we can find a keyword instead. - const keywordCompletion = forEach( - getKeywordCompletions(KeywordCompletionFilters.None), - c => c.name === name - ); - if (keywordCompletion) { + if (source === undefined && some(getKeywordCompletions(KeywordCompletionFilters.None), c => c.name === name)) { return { name, kind: ScriptElementKind.keyword, @@ -374,14 +415,23 @@ namespace ts.Completions { documentation: undefined, tags: undefined, codeActions: undefined, + source: undefined, }; } return undefined; } - function getCompletionEntryCodeActions(symbolToOriginInfoMap: SymbolOriginInfoMap, symbol: Symbol, checker: TypeChecker, host: LanguageServiceHost, compilerOptions: CompilerOptions, sourceFile: SourceFile, rulesProvider: formatting.RulesProvider): CodeAction[] | undefined { - const symbolOriginInfo = symbolToOriginInfoMap[getUniqueSymbolId(symbol, checker)]; + function getCompletionEntryCodeActions( + symbolToOriginInfoMap: SymbolOriginInfoMap, + symbol: Symbol, + checker: TypeChecker, + host: LanguageServiceHost, + compilerOptions: CompilerOptions, + sourceFile: SourceFile, + rulesProvider: formatting.RulesProvider, + ): CodeAction[] | undefined { + const symbolOriginInfo = symbolToOriginInfoMap[getSymbolId(symbol)]; if (!symbolOriginInfo) { return undefined; } @@ -407,20 +457,11 @@ namespace ts.Completions { compilerOptions: CompilerOptions, sourceFile: SourceFile, position: number, - entryName: string, + entryId: CompletionEntryIdentifier, allSourceFiles: ReadonlyArray, ): Symbol | undefined { - // Compute all the completion symbols again. - const completionData = getCompletionData(typeChecker, log, sourceFile, position, allSourceFiles); - if (!completionData) { - return undefined; - } - const { symbols, allowStringLiteral } = completionData; - // Find the symbol with the matching entry name. - // We don't need to perform character checks here because we're only comparing the - // name against 'entryName' (which is known to be good), not building a new - // completion entry. - return find(symbols, s => getCompletionEntryDisplayNameForSymbol(s, compilerOptions.target, /*performCharacterChecks*/ false, allowStringLiteral) === entryName); + const completion = getSymbolCompletionFromEntryId(typeChecker, log, compilerOptions, sourceFile, position, entryId, allSourceFiles); + return completion && completion.symbol; } interface CompletionData { @@ -923,7 +964,6 @@ namespace ts.Completions { function getSymbolsFromOtherSourceFileExports(symbols: Symbol[], tokenText: string): void { const tokenTextLowerCase = tokenText.toLowerCase(); - const symbolIdMap = arrayToNumericMap(symbols, s => getUniqueSymbolId(s, typeChecker)); codefix.forEachExternalModule(typeChecker, allSourceFiles, moduleSymbol => { if (moduleSymbol === sourceFile.symbol) { @@ -941,10 +981,14 @@ namespace ts.Completions { } } - const id = getUniqueSymbolId(symbol, typeChecker); - if (!symbolIdMap[id] && stringContainsCharactersInOrder(name.toLowerCase(), tokenTextLowerCase)) { + if (symbol.declarations && symbol.declarations.some(d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) { + // Don't add a completion for a re-export, only for the original. + continue; + } + + if (stringContainsCharactersInOrder(name.toLowerCase(), tokenTextLowerCase)) { symbols.push(symbol); - symbolToOriginInfoMap[id] = { moduleSymbol, isDefaultExport }; + symbolToOriginInfoMap[getSymbolId(symbol)] = { moduleSymbol, isDefaultExport }; } } }); diff --git a/src/services/services.ts b/src/services/services.ts index 3b90a80a3875d..d6e75868852ae 100644 --- a/src/services/services.ts +++ b/src/services/services.ts @@ -1327,16 +1327,31 @@ namespace ts { return Completions.getCompletionsAtPosition(host, program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, program.getSourceFiles()); } - function getCompletionEntryDetails(fileName: string, position: number, entryName: string, formattingOptions?: FormatCodeSettings): CompletionEntryDetails { + function getCompletionEntryDetails(fileName: string, position: number, name: string, formattingOptions?: FormatCodeSettings, source?: string): CompletionEntryDetails { synchronizeHostData(); const ruleProvider = formattingOptions ? getRuleProvider(formattingOptions) : undefined; return Completions.getCompletionEntryDetails( - program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName, program.getSourceFiles(), host, ruleProvider); + program.getTypeChecker(), + log, + program.getCompilerOptions(), + getValidSourceFile(fileName), + position, + { name, source }, + program.getSourceFiles(), + host, + ruleProvider); } - function getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol { + function getCompletionEntrySymbol(fileName: string, position: number, name: string, source?: string): Symbol { synchronizeHostData(); - return Completions.getCompletionEntrySymbol(program.getTypeChecker(), log, program.getCompilerOptions(), getValidSourceFile(fileName), position, entryName, program.getSourceFiles()); + return Completions.getCompletionEntrySymbol( + program.getTypeChecker(), + log, + program.getCompilerOptions(), + getValidSourceFile(fileName), + position, + { name, source }, + program.getSourceFiles()); } function getQuickInfoAtPosition(fileName: string, position: number): QuickInfo { diff --git a/src/services/types.ts b/src/services/types.ts index 075d2d6e643b4..c854eb8aaccfc 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -237,9 +237,9 @@ namespace ts { getEncodedSemanticClassifications(fileName: string, span: TextSpan): Classifications; getCompletionsAtPosition(fileName: string, position: number): CompletionInfo; - // "options" is optional only for backwards-compatibility - getCompletionEntryDetails(fileName: string, position: number, entryName: string, options?: FormatCodeOptions | FormatCodeSettings): CompletionEntryDetails; - getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol; + // "options" and "source" are optional only for backwards-compatibility + getCompletionEntryDetails(fileName: string, position: number, name: string, options?: FormatCodeOptions | FormatCodeSettings, source?: string): CompletionEntryDetails; + getCompletionEntrySymbol(fileName: string, position: number, name: string, source?: string): Symbol; getQuickInfoAtPosition(fileName: string, position: number): QuickInfo; @@ -696,6 +696,7 @@ namespace ts { */ replacementSpan?: TextSpan; hasAction?: true; + source?: string; } export interface CompletionEntryDetails { @@ -706,6 +707,7 @@ namespace ts { documentation: SymbolDisplayPart[]; tags: JSDocTagInfo[]; codeActions?: CodeAction[]; + source?: SymbolDisplayPart[]; } export interface OutliningSpan { diff --git a/tests/baselines/reference/api/tsserverlibrary.d.ts b/tests/baselines/reference/api/tsserverlibrary.d.ts index c5855e4cff7f3..9377a093108d4 100644 --- a/tests/baselines/reference/api/tsserverlibrary.d.ts +++ b/tests/baselines/reference/api/tsserverlibrary.d.ts @@ -3915,8 +3915,8 @@ declare namespace ts { getEncodedSyntacticClassifications(fileName: string, span: TextSpan): Classifications; getEncodedSemanticClassifications(fileName: string, span: TextSpan): Classifications; getCompletionsAtPosition(fileName: string, position: number): CompletionInfo; - getCompletionEntryDetails(fileName: string, position: number, entryName: string, options?: FormatCodeOptions | FormatCodeSettings): CompletionEntryDetails; - getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol; + getCompletionEntryDetails(fileName: string, position: number, name: string, options?: FormatCodeOptions | FormatCodeSettings, source?: string): CompletionEntryDetails; + getCompletionEntrySymbol(fileName: string, position: number, name: string, source?: string): Symbol; getQuickInfoAtPosition(fileName: string, position: number): QuickInfo; getNameOrDottedNameSpan(fileName: string, startPos: number, endPos: number): TextSpan; getBreakpointStatementAtPosition(fileName: string, position: number): TextSpan; @@ -4296,6 +4296,7 @@ declare namespace ts { */ replacementSpan?: TextSpan; hasAction?: true; + source?: string; } interface CompletionEntryDetails { name: string; @@ -4305,6 +4306,7 @@ declare namespace ts { documentation: SymbolDisplayPart[]; tags: JSDocTagInfo[]; codeActions?: CodeAction[]; + source?: SymbolDisplayPart[]; } interface OutliningSpan { /** The span of the document to actually collapse. */ @@ -6031,7 +6033,11 @@ declare namespace ts.server.protocol { /** * Names of one or more entries for which to obtain details. */ - entryNames: string[]; + entryNames: (string | CompletionEntryIdentifier)[]; + } + interface CompletionEntryIdentifier { + name: string; + source: string; } /** * Completion entry details request; value of command field is @@ -6087,6 +6093,10 @@ declare namespace ts.server.protocol { * made to avoid errors. The CompletionEntryDetails will have these actions. */ hasAction?: true; + /** + * Identifier (not necessarily human-readable) identifying where this completion came from. + */ + source?: string; } /** * Additional completion entry details, available on demand @@ -6120,6 +6130,10 @@ declare namespace ts.server.protocol { * The associated code actions for this entry */ codeActions?: CodeAction[]; + /** + * Human-readable description of the `source` from the CompletionEntry. + */ + source?: SymbolDisplayPart[]; } interface CompletionsResponse extends Response { body?: CompletionEntry[]; diff --git a/tests/baselines/reference/api/typescript.d.ts b/tests/baselines/reference/api/typescript.d.ts index a4ed91c12d057..c97d0549911e6 100644 --- a/tests/baselines/reference/api/typescript.d.ts +++ b/tests/baselines/reference/api/typescript.d.ts @@ -3915,8 +3915,8 @@ declare namespace ts { getEncodedSyntacticClassifications(fileName: string, span: TextSpan): Classifications; getEncodedSemanticClassifications(fileName: string, span: TextSpan): Classifications; getCompletionsAtPosition(fileName: string, position: number): CompletionInfo; - getCompletionEntryDetails(fileName: string, position: number, entryName: string, options?: FormatCodeOptions | FormatCodeSettings): CompletionEntryDetails; - getCompletionEntrySymbol(fileName: string, position: number, entryName: string): Symbol; + getCompletionEntryDetails(fileName: string, position: number, name: string, options?: FormatCodeOptions | FormatCodeSettings, source?: string): CompletionEntryDetails; + getCompletionEntrySymbol(fileName: string, position: number, name: string, source?: string): Symbol; getQuickInfoAtPosition(fileName: string, position: number): QuickInfo; getNameOrDottedNameSpan(fileName: string, startPos: number, endPos: number): TextSpan; getBreakpointStatementAtPosition(fileName: string, position: number): TextSpan; @@ -4296,6 +4296,7 @@ declare namespace ts { */ replacementSpan?: TextSpan; hasAction?: true; + source?: string; } interface CompletionEntryDetails { name: string; @@ -4305,6 +4306,7 @@ declare namespace ts { documentation: SymbolDisplayPart[]; tags: JSDocTagInfo[]; codeActions?: CodeAction[]; + source?: SymbolDisplayPart[]; } interface OutliningSpan { /** The span of the document to actually collapse. */ diff --git a/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts b/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts index 5662b57fdf678..734b3ff0f66cc 100644 --- a/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts +++ b/tests/cases/fourslash/completionsImport_default_addToNamedImports.ts @@ -9,10 +9,11 @@ ////f/**/; goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.applyCodeActionFromCompletion("", { name: "foo", + source: "/a", description: `Add 'foo' to existing import declaration from "./a".`, newFileContent: `import foo, { x } from "./a"; f;`, diff --git a/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts b/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts index b442ed21549e1..d4a81bc527240 100644 --- a/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts +++ b/tests/cases/fourslash/completionsImport_default_addToNamespaceImport.ts @@ -8,10 +8,11 @@ ////f/**/; goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.applyCodeActionFromCompletion("", { name: "foo", + source: "/a", description: `Add 'foo' to existing import declaration from "./a".`, newFileContent: `import foo, * as a from "./a"; f;`, diff --git a/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts b/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts index 3ee4decb0f8b7..f49f5850043d7 100644 --- a/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts +++ b/tests/cases/fourslash/completionsImport_default_alreadyExistedWithRename.ts @@ -8,10 +8,11 @@ ////f/**/; goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.applyCodeActionFromCompletion("", { name: "foo", + source: "/a", description: `Import 'foo' from "./a".`, // TODO: GH#18445 newFileContent: `import f_o_o from "./a"; diff --git a/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts index 6dbd437d36470..fb4d65a1c52db 100644 --- a/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts +++ b/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts @@ -7,10 +7,11 @@ ////f/**/; goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.applyCodeActionFromCompletion("", { name: "foo", + source: "/a", description: `Import 'foo' from "./a".`, // TODO: GH#18445 newFileContent: `import foo from "./a";\r diff --git a/tests/cases/fourslash/completionsImport_fromAmbientModule.ts b/tests/cases/fourslash/completionsImport_fromAmbientModule.ts index b45ad9824865a..05e7601c690be 100644 --- a/tests/cases/fourslash/completionsImport_fromAmbientModule.ts +++ b/tests/cases/fourslash/completionsImport_fromAmbientModule.ts @@ -10,6 +10,7 @@ verify.applyCodeActionFromCompletion("", { name: "x", + source: "m", description: `Import 'x' from "m".`, // TODO: GH#18445 newFileContent: `import { x } from "m";\r diff --git a/tests/cases/fourslash/completionsImport_matching.ts b/tests/cases/fourslash/completionsImport_matching.ts index 3470d59bff20b..edecf3592cdc5 100644 --- a/tests/cases/fourslash/completionsImport_matching.ts +++ b/tests/cases/fourslash/completionsImport_matching.ts @@ -14,9 +14,9 @@ goTo.marker(""); -verify.not.completionListContains("abcde"); -verify.not.completionListContains("dbf"); +verify.not.completionListContains({ name: "abcde", source: "/a" }); +verify.not.completionListContains({ name: "dbf", source: "/a" }); -verify.completionListContains("bdf", "function bdf(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); -verify.completionListContains("abcdef", "function abcdef(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); -verify.completionListContains("BDF", "function BDF(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "bdf", source: "/a" }, "function bdf(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "abcdef", source: "/a" }, "function abcdef(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "BDF", source: "/a" }, "function BDF(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); diff --git a/tests/cases/fourslash/completionsImport_multipleWithSameName.ts b/tests/cases/fourslash/completionsImport_multipleWithSameName.ts new file mode 100644 index 0000000000000..825a4ca0a9798 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_multipleWithSameName.ts @@ -0,0 +1,29 @@ +/// + +// @Filename: /global.d.ts +// A local variable would prevent import completions (see `completionsImport_shadowedByLocal.ts`), but a global doesn't. +////declare var foo: number; + +// @Filename: /a.ts +////export const foo = 0; + +// @Filename: /b.ts +////export const foo = 1; + +// @Filename: /c.ts +////fo/**/ + +goTo.marker(""); +verify.completionListContains("foo", "var foo: number", "", "var"); +verify.completionListContains({ name: "foo", source: "/a" }, "const foo: 0", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/b" }, "const foo: 1", "", "const", /*spanIndex*/ undefined, /*hasAction*/ true); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + source: "/b", + description: `Import 'foo' from "./b".`, + // TODO: GH#18445 + newFileContent: `import { foo } from "./b";\r +\r +fo`, +}); diff --git a/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts b/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts index 23119ad491f79..5444b25692aad 100644 --- a/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts +++ b/tests/cases/fourslash/completionsImport_named_addToNamedImports.ts @@ -9,10 +9,11 @@ ////f/**/; goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.applyCodeActionFromCompletion("", { name: "foo", + source: "/a", description: `Add 'foo' to existing import declaration from "./a".`, newFileContent: `import { x, foo } from "./a"; f;`, diff --git a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts index 95c68c2a05e88..809e40e7cf2ef 100644 --- a/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts +++ b/tests/cases/fourslash/completionsImport_named_didNotExistBefore.ts @@ -9,11 +9,13 @@ ////t/**/ goTo.marker(""); -verify.completionListContains("Test1", "function Test1(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "Test1", source: "/a" }, "function Test1(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.completionListContains("Test2", "import Test2", "", "alias", /*spanIndex*/ undefined, /*hasAction*/ undefined); +verify.not.completionListContains({ name: "Test2", source: "/a" }); verify.applyCodeActionFromCompletion("", { name: "Test1", + source: "/a", description: `Add 'Test1' to existing import declaration from "./a".`, newFileContent: `import { Test2, Test1 } from "./a"; t`, diff --git a/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts b/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts index da00907e60fb7..70cd75a4e41eb 100644 --- a/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts +++ b/tests/cases/fourslash/completionsImport_named_namespaceImportExists.ts @@ -8,10 +8,11 @@ ////f/**/; goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); verify.applyCodeActionFromCompletion("", { name: "foo", + source: "/a", description: `Import 'foo' from "./a".`, // TODO: GH#18445 newFileContent: `import * as a from "./a"; diff --git a/tests/cases/fourslash/completionsImport_ofAlias.ts b/tests/cases/fourslash/completionsImport_ofAlias.ts new file mode 100644 index 0000000000000..27bc29fa21058 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_ofAlias.ts @@ -0,0 +1,30 @@ +/// + +// Tests that we don't filter out a completion for an alias, +// so long as it's not an alias to a different module. + +// @Filename: /a.ts +////const foo = 0; +////export { foo }; + +// @Filename: /a_reexport.ts +// Should not show up in completions +////export { foo } from "./a"; + +// @Filename: /b.ts +////fo/**/ + +goTo.marker(""); +// https://github.com/Microsoft/TypeScript/issues/14003 +verify.completionListContains({ name: "foo", source: "/a" }, "import foo", "", "alias", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.not.completionListContains({ name: "foo", source: "/a_reexport" }); + +verify.applyCodeActionFromCompletion("", { + name: "foo", + source: "/a", + description: `Import 'foo' from "./a".`, + // TODO: GH#18445 + newFileContent: `import { foo } from "./a";\r +\r +fo`, +}); diff --git a/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts b/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts index 904de35354ac5..5eddb5037c177 100644 --- a/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts +++ b/tests/cases/fourslash/completionsImport_previousTokenIsSemicolon.ts @@ -8,4 +8,4 @@ /////**/ goTo.marker(""); -verify.completionListContains("foo", "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); +verify.completionListContains({ name: "foo", source: "/a" }, "function foo(): void", "", "function", /*spanIndex*/ undefined, /*hasAction*/ true); diff --git a/tests/cases/fourslash/completionsImport_shadowedByLocal.ts b/tests/cases/fourslash/completionsImport_shadowedByLocal.ts new file mode 100644 index 0000000000000..d35a021acc7c7 --- /dev/null +++ b/tests/cases/fourslash/completionsImport_shadowedByLocal.ts @@ -0,0 +1,12 @@ +/// + +// @Filename: /a.ts +////export const foo = 0; + +// @Filename: /b.ts +////const foo = 1; +////fo/**/ + +goTo.marker(""); +verify.completionListContains("foo", "const foo: 1", "", "const"); +verify.not.completionListContains({ name: "foo", source: "/a" }); diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index a7c54b5dda649..1dd1675b513a3 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -142,7 +142,7 @@ declare namespace FourSlashInterface { constructor(negative?: boolean); completionListCount(expectedCount: number): void; completionListContains( - symbol: string, + entryId: string | { name: string, source?: string }, text?: string, documentation?: string, kind?: string, @@ -196,6 +196,7 @@ declare namespace FourSlashInterface { ): void; //TODO: better type applyCodeActionFromCompletion(markerName: string, options: { name: string, + source?: string, description: string, newFileContent?: string, newRangeContent?: string,