Skip to content

Commit 0da4f59

Browse files
author
Andy Hanson
committed
Detect re-exports from "export *" in completions
1 parent f352e46 commit 0da4f59

11 files changed

+60
-50
lines changed

‎src/harness/fourslash.ts

+36-35
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

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

3100-
if (spanIndex !== undefined) {
3101-
const span = this.getTextSpanForRangeAtIndex(spanIndex);
3102-
assert.isTrue(TestState.textSpansEqual(span, item.replacementSpan), this.assertionMessageAtLastKnownMarker(stringify(span) + " does not equal " + stringify(item.replacementSpan) + " replacement span for " + entryId));
3103-
}
3086+
if (documentation !== undefined || text !== undefined || entryId.source !== undefined) {
3087+
const details = this.getCompletionEntryDetails(item.name, item.source);
31043088

3105-
assert.equal(item.hasAction, hasAction);
3089+
if (documentation !== undefined) {
3090+
assert.equal(ts.displayPartsToString(details.documentation), documentation, this.assertionMessageAtLastKnownMarker("completion item documentation for " + entryId));
3091+
}
3092+
if (text !== undefined) {
3093+
assert.equal(ts.displayPartsToString(details.displayParts), text, this.assertionMessageAtLastKnownMarker("completion item detail text for " + entryId));
3094+
}
31063095

3107-
return;
3096+
if (entryId.source === undefined) {
3097+
assert.equal(options && options.sourceDisplay, undefined);
31083098
}
3099+
else {
3100+
assert.deepEqual(details.source, [ts.textPart(options!.sourceDisplay)]);
3101+
}
3102+
}
3103+
3104+
if (kind !== undefined) {
3105+
assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
31093106
}
31103107

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

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

31163116
private findFile(indexOrName: string | number) {
@@ -4355,8 +4355,8 @@ namespace FourSlashInterface {
43554355
this.state.printCurrentSignatureHelp();
43564356
}
43574357

4358-
public printCompletionListMembers() {
4359-
this.state.printCompletionListMembers();
4358+
public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
4359+
this.state.printCompletionListMembers(options);
43604360
}
43614361

43624362
public printAvailableCodeFixes() {
@@ -4555,6 +4555,7 @@ namespace FourSlashInterface {
45554555

45564556
export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
45574557
sourceDisplay: string;
4558+
allowDuplicate: boolean; // TODO: GH#20042
45584559
}
45594560

45604561
export interface NewContentOptions {

‎src/services/completions.ts

+8-5
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

+1-1
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

+1-1
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

+1-2
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

+2-2
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

+2
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

+5
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

+2-2
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

+1-1
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

‎tests/cases/fourslash/javaScriptClass1.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,4 @@ verify.completionListContains("substr", /*displayText*/ undefined, /*documentati
2828
edit.backspace('bar.'.length);
2929

3030
edit.insert('union.');
31-
verify.completionListContains("toString", /*displayText*/ undefined, /*documentation*/ undefined, "method");
31+
verify.completionListContains("toString", /*displayText*/ undefined, /*documentation*/ undefined, "method", undefined, undefined, { allowDuplicate: true });

0 commit comments

Comments
 (0)