Skip to content

Commit 478b404

Browse files
author
Andy
authored
Detect re-exports from "export *" in completions (#20043)
1 parent 94581c1 commit 478b404

11 files changed

+60
-50
lines changed

‎src/harness/fourslash.ts‎

Lines changed: 36 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1703,8 +1703,8 @@ Actual: ${stringify(fullActual)}`);
17031703
Harness.IO.log(stringify(sigHelp));
17041704
}
17051705

1706-
public printCompletionListMembers() {
1707-
const completions = this.getCompletionListAtCaret();
1706+
public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
1707+
const completions = this.getCompletionListAtCaret(options);
17081708
this.printMembersOrCompletions(completions);
17091709
}
17101710

@@ -3076,44 +3076,44 @@ Actual: ${stringify(fullActual)}`);
30763076
hasAction: boolean | undefined,
30773077
options: FourSlashInterface.VerifyCompletionListContainsOptions | undefined,
30783078
) {
3079-
for (const item of items) {
3080-
if (item.name === entryId.name && item.source === entryId.source) {
3081-
if (documentation !== undefined || text !== undefined || entryId.source !== undefined) {
3082-
const details = this.getCompletionEntryDetails(item.name, item.source);
3083-
3084-
if (documentation !== undefined) {
3085-
assert.equal(ts.displayPartsToString(details.documentation), documentation, this.assertionMessageAtLastKnownMarker("completion item documentation for " + entryId));
3086-
}
3087-
if (text !== undefined) {
3088-
assert.equal(ts.displayPartsToString(details.displayParts), text, this.assertionMessageAtLastKnownMarker("completion item detail text for " + entryId));
3089-
}
3090-
3091-
if (entryId.source === undefined) {
3092-
assert.equal(options && options.sourceDisplay, undefined);
3093-
}
3094-
else {
3095-
assert.deepEqual(details.source, [ts.textPart(options!.sourceDisplay)]);
3096-
}
3097-
}
3098-
3099-
if (kind !== undefined) {
3100-
assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
3101-
}
3079+
const matchingItems = items.filter(item => item.name === entryId.name && item.source === entryId.source);
3080+
if (matchingItems.length === 0) {
3081+
const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
3082+
this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
3083+
}
3084+
else if (matchingItems.length > 1 && !(options && options.allowDuplicate)) {
3085+
this.raiseError(`Found duplicate completion items for ${stringify(entryId)}`);
3086+
}
3087+
const item = matchingItems[0];
31023088

3103-
if (spanIndex !== undefined) {
3104-
const span = this.getTextSpanForRangeAtIndex(spanIndex);
3105-
assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId));
3106-
}
3089+
if (documentation !== undefined || text !== undefined || entryId.source !== undefined) {
3090+
const details = this.getCompletionEntryDetails(item.name, item.source);
31073091

3108-
assert.equal(item.hasAction, hasAction);
3092+
if (documentation !== undefined) {
3093+
assert.equal(ts.displayPartsToString(details.documentation), documentation, this.assertionMessageAtLastKnownMarker("completion item documentation for " + entryId));
3094+
}
3095+
if (text !== undefined) {
3096+
assert.equal(ts.displayPartsToString(details.displayParts), text, this.assertionMessageAtLastKnownMarker("completion item detail text for " + entryId));
3097+
}
31093098

3110-
return;
3099+
if (entryId.source === undefined) {
3100+
assert.equal(options && options.sourceDisplay, undefined);
31113101
}
3102+
else {
3103+
assert.deepEqual(details.source, [ts.textPart(options!.sourceDisplay)]);
3104+
}
3105+
}
3106+
3107+
if (kind !== undefined) {
3108+
assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
31123109
}
31133110

3114-
const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
3111+
if (spanIndex !== undefined) {
3112+
const span = this.getTextSpanForRangeAtIndex(spanIndex);
3113+
assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId));
3114+
}
31153115

3116-
this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
3116+
assert.equal(item.hasAction, hasAction);
31173117
}
31183118

31193119
private findFile(indexOrName: string | number) {
@@ -4358,8 +4358,8 @@ namespace FourSlashInterface {
43584358
this.state.printCurrentSignatureHelp();
43594359
}
43604360

4361-
public printCompletionListMembers() {
4362-
this.state.printCompletionListMembers();
4361+
public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
4362+
this.state.printCompletionListMembers(options);
43634363
}
43644364

43654365
public printAvailableCodeFixes() {
@@ -4558,6 +4558,7 @@ namespace FourSlashInterface {
45584558

45594559
export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
45604560
sourceDisplay: string;
4561+
allowDuplicate: boolean; // TODO: GH#20042
45614562
}
45624563

45634564
export interface NewContentOptions {

‎src/services/completions.ts‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1025,6 +1025,14 @@ namespace ts.Completions {
10251025

10261026
for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) {
10271027
let { name } = symbol;
1028+
1029+
// Don't add a completion for a re-export, only for the original.
1030+
// If `symbol.parent !== moduleSymbol`, this comes from an `export * from "foo"` re-export. Those don't create new symbols.
1031+
// If `some(...)`, this comes from an `export { foo } from "foo"` re-export, which creates a new symbol (thus isn't caught by the first check).
1032+
if (symbol.parent !== moduleSymbol || some(symbol.declarations, d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
1033+
continue;
1034+
}
1035+
10281036
const isDefaultExport = name === "default";
10291037
if (isDefaultExport) {
10301038
const localSymbol = getLocalSymbolForExportDefault(symbol);
@@ -1037,11 +1045,6 @@ namespace ts.Completions {
10371045
}
10381046
}
10391047

1040-
if (symbol.declarations && symbol.declarations.some(d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
1041-
// Don't add a completion for a re-export, only for the original.
1042-
continue;
1043-
}
1044-
10451048
if (stringContainsCharactersInOrder(name.toLowerCase(), tokenTextLowerCase)) {
10461049
symbols.push(symbol);
10471050
symbolToOriginInfoMap[getSymbolId(symbol)] = { moduleSymbol, isDefaultExport };

‎tests/cases/fourslash/completionForStringLiteralNonrelativeImport8.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,5 +51,5 @@ for (const kind of kinds) {
5151
verify.completionListContains("1test");
5252

5353
goTo.marker(kind + "2");
54-
verify.completionListContains("2test");
54+
verify.completionListContains("2test", undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
5555
}

‎tests/cases/fourslash/completionInJSDocFunctionNew.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@
77

88
goTo.marker();
99
verify.completionListCount(118);
10-
verify.completionListContains('new');
10+
verify.completionListContains('new', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042

‎tests/cases/fourslash/completionInJSDocFunctionThis.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,5 +6,4 @@
66

77
goTo.marker();
88
verify.completionListCount(119);
9-
verify.completionListContains('this');
10-
9+
verify.completionListContains('this', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042

‎tests/cases/fourslash/completionListPrimitives.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@ verify.completionListContains("boolean");
88
verify.completionListContains("null");
99
verify.completionListContains("number");
1010
verify.completionListContains("string");
11-
verify.completionListContains("undefined");
12-
verify.completionListContains("void");
11+
verify.completionListContains("undefined", undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
12+
verify.completionListContains("void");

‎tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
/// <reference path="fourslash.ts" />
22

3+
// @noLib: true
4+
35
// @Filename: /a.ts
46
////export default function foo() {}
57

‎tests/cases/fourslash/completionsImport_ofAlias.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,19 @@
1111
// Should not show up in completions
1212
////export { foo } from "./a";
1313

14+
// @Filename: /a_reexport_2.ts
15+
////export * from "./a";
16+
1417
// @Filename: /b.ts
1518
////fo/**/
1619

1720
goTo.marker("");
1821
const options = { includeExternalModuleExports: true, sourceDisplay: "./a" };
1922
// TODO: https://github.com/Microsoft/TypeScript/issues/14003
23+
//TODO: verify that there's only one!
2024
verify.completionListContains({ name: "foo", source: "/a" }, "import foo", "", "alias", /*spanIndex*/ undefined, /*hasAction*/ true, options);
2125
verify.not.completionListContains({ name: "foo", source: "/a_reexport" }, undefined, undefined, undefined, undefined, undefined, options);
26+
verify.not.completionListContains({ name: "foo", source: "/a_reexport_2" }, undefined, undefined, undefined, undefined, undefined, options);
2227

2328
verify.applyCodeActionFromCompletion("", {
2429
name: "foo",

‎tests/cases/fourslash/fourslash.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ declare namespace FourSlashInterface {
148148
kind?: string,
149149
spanIndex?: number,
150150
hasAction?: boolean,
151-
options?: { includeExternalModuleExports: boolean, sourceDisplay: string },
151+
options?: { includeExternalModuleExports?: boolean, sourceDisplay?: string, allowDuplicate?: boolean },
152152
): void;
153153
completionListItemsCountIsGreaterThan(count: number): void;
154154
completionListIsEmpty(): void;
@@ -358,7 +358,7 @@ declare namespace FourSlashInterface {
358358
printCurrentFileStateWithoutCaret(): void;
359359
printCurrentQuickInfo(): void;
360360
printCurrentSignatureHelp(): void;
361-
printCompletionListMembers(): void;
361+
printCompletionListMembers(options?: { includeExternalModuleExports: boolean }): void;
362362
printAvailableCodeFixes(): void;
363363
printBreakpointLocation(pos: number): void;
364364
printBreakpointAtCurrentLocation(): void;

‎tests/cases/fourslash/getJavaScriptCompletions2.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,4 +7,4 @@
77
////v./**/
88

99
goTo.marker();
10-
verify.completionListContains("valueOf", /*displayText:*/ undefined, /*documentation*/ undefined, "method");
10+
verify.completionListContains("valueOf", /*displayText:*/ undefined, /*documentation*/ undefined, "method", undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042

0 commit comments

Comments
 (0)