Detect re-exports from "export *" in completions

microsoft · Nov 15, 2017 · d71a8b8 · d71a8b8
1 parent 4b96edf
commit d71a8b8
Show file tree

Hide file tree

Showing 11 changed files with 60 additions and 50 deletions.
diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts
@@ -1703,8 +1703,8 @@ Actual: ${stringify(fullActual)}`);
             Harness.IO.log(stringify(sigHelp));
         }
 
-        public printCompletionListMembers() {
-            const completions = this.getCompletionListAtCaret();
+        public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
+            const completions = this.getCompletionListAtCaret(options);
             this.printMembersOrCompletions(completions);
         }
 
@@ -3073,44 +3073,44 @@ Actual: ${stringify(fullActual)}`);
             hasAction: boolean | undefined,
             options: FourSlashInterface.VerifyCompletionListContainsOptions | undefined,
         ) {
-            for (const item of items) {
-                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 " + entryId));
-                        }
-                        if (text !== undefined) {
-                            assert.equal(ts.displayPartsToString(details.displayParts), text, this.assertionMessageAtLastKnownMarker("completion item detail text for " + entryId));
-                        }
-
-                        if (entryId.source === undefined) {
-                            assert.equal(options && options.sourceDisplay, undefined);
-                        }
-                        else {
-                            assert.deepEqual(details.source, [ts.textPart(options!.sourceDisplay)]);
-                        }
-                    }
-
-                    if (kind !== undefined) {
-                        assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
-                    }
+            const matchingItems = items.filter(item => item.name === entryId.name && item.source === entryId.source);
+            if (matchingItems.length === 0) {
+                const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
+                this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
+            }
+            else if (matchingItems.length > 1 && !(options && options.allowDuplicate)) {
+                this.raiseError(`Found duplicate completion items for ${stringify(entryId)}`);
+            }
+            const item = matchingItems[0];
 
-                    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 " + entryId));
-                    }
+            if (documentation !== undefined || text !== undefined || entryId.source !== undefined) {
+                const details = this.getCompletionEntryDetails(item.name, item.source);
 
-                    assert.equal(item.hasAction, hasAction);
+                if (documentation !== undefined) {
+                    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 " + entryId));
+                }
 
-                    return;
+                if (entryId.source === undefined) {
+                    assert.equal(options && options.sourceDisplay, undefined);
                 }
+                else {
+                    assert.deepEqual(details.source, [ts.textPart(options!.sourceDisplay)]);
+                }
+            }
+
+            if (kind !== undefined) {
+                assert.equal(item.kind, kind, this.assertionMessageAtLastKnownMarker("completion item kind for " + entryId));
             }
 
-            const itemsString = items.map(item => stringify({ name: item.name, source: item.source, kind: item.kind })).join(",\n");
+            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 " + entryId));
+            }
 
-            this.raiseError(`Expected "${stringify({ entryId, text, documentation, kind })}" to be in list [${itemsString}]`);
+            assert.equal(item.hasAction, hasAction);
         }
 
         private findFile(indexOrName: string | number) {
@@ -4355,8 +4355,8 @@ namespace FourSlashInterface {
             this.state.printCurrentSignatureHelp();
         }
 
-        public printCompletionListMembers() {
-            this.state.printCompletionListMembers();
+        public printCompletionListMembers(options: ts.GetCompletionsAtPositionOptions | undefined) {
+            this.state.printCompletionListMembers(options);
         }
 
         public printAvailableCodeFixes() {
@@ -4555,6 +4555,7 @@ namespace FourSlashInterface {
 
     export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
         sourceDisplay: string;
+        allowDuplicate: boolean; // TODO: GH#20042
     }
 
     export interface NewContentOptions {

diff --git a/src/services/completions.ts b/src/services/completions.ts
@@ -1025,6 +1025,14 @@ namespace ts.Completions {
 
                 for (let symbol of typeChecker.getExportsOfModule(moduleSymbol)) {
                     let { name } = symbol;
+
+                    // Don't add a completion for a re-export, only for the original.
+                    // If `symbol.parent !== moduleSymbol`, this comes from an `export * from "foo"` re-export. Those don't create new symbols.
+                    // 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).
+                    if (symbol.parent !== moduleSymbol || some(symbol.declarations, d => isExportSpecifier(d) && !!d.parent.parent.moduleSpecifier)) {
+                        continue;
+                    }
+
                     const isDefaultExport = name === "default";
                     if (isDefaultExport) {
                         const localSymbol = getLocalSymbolForExportDefault(symbol);
@@ -1037,11 +1045,6 @@ namespace ts.Completions {
                         }
                     }
 
-                    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[getSymbolId(symbol)] = { moduleSymbol, isDefaultExport };

diff --git a/tests/cases/fourslash/completionForStringLiteralNonrelativeImport8.ts b/tests/cases/fourslash/completionForStringLiteralNonrelativeImport8.ts
@@ -51,5 +51,5 @@ for (const kind of kinds) {
     verify.completionListContains("1test");
 
     goTo.marker(kind + "2");
-    verify.completionListContains("2test");
+    verify.completionListContains("2test", undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
 }
diff --git a/tests/cases/fourslash/completionInJSDocFunctionNew.ts b/tests/cases/fourslash/completionInJSDocFunctionNew.ts
@@ -7,4 +7,4 @@
 
 goTo.marker();
 verify.completionListCount(117);
-verify.completionListContains('new');
+verify.completionListContains('new', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
diff --git a/tests/cases/fourslash/completionInJSDocFunctionThis.ts b/tests/cases/fourslash/completionInJSDocFunctionThis.ts
@@ -6,5 +6,4 @@
 
 goTo.marker();
 verify.completionListCount(118);
-verify.completionListContains('this');
-
+verify.completionListContains('this', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
diff --git a/tests/cases/fourslash/completionListPrimitives.ts b/tests/cases/fourslash/completionListPrimitives.ts
@@ -8,5 +8,5 @@ verify.completionListContains("boolean");
 verify.completionListContains("null");
 verify.completionListContains("number");
 verify.completionListContains("string");
-verify.completionListContains("undefined");
-verify.completionListContains("void");
+verify.completionListContains("undefined", undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
+verify.completionListContains("void");
diff --git a/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts b/tests/cases/fourslash/completionsImport_default_didNotExistBefore.ts
@@ -1,5 +1,7 @@
 /// <reference path="fourslash.ts" />
 
+// @noLib: true
+
 // @Filename: /a.ts
 ////export default function foo() {}
 

diff --git a/tests/cases/fourslash/completionsImport_ofAlias.ts b/tests/cases/fourslash/completionsImport_ofAlias.ts
@@ -11,14 +11,19 @@
 // Should not show up in completions
 ////export { foo } from "./a";
 
+// @Filename: /a_reexport_2.ts
+////export * from "./a";
+
 // @Filename: /b.ts
 ////fo/**/
 
 goTo.marker("");
 const options = { includeExternalModuleExports: true, sourceDisplay: "./a" };
 // TODO: https://github.com/Microsoft/TypeScript/issues/14003
+//TODO: verify that there's only one!
 verify.completionListContains({ name: "foo", source: "/a" }, "import foo", "", "alias", /*spanIndex*/ undefined, /*hasAction*/ true, options);
 verify.not.completionListContains({ name: "foo", source: "/a_reexport" }, undefined, undefined, undefined, undefined, undefined, options);
+verify.not.completionListContains({ name: "foo", source: "/a_reexport_2" }, undefined, undefined, undefined, undefined, undefined, options);
 
 verify.applyCodeActionFromCompletion("", {
     name: "foo",

diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts
@@ -148,7 +148,7 @@ declare namespace FourSlashInterface {
             kind?: string,
             spanIndex?: number,
             hasAction?: boolean,
-            options?: { includeExternalModuleExports: boolean, sourceDisplay: string },
+            options?: { includeExternalModuleExports?: boolean, sourceDisplay?: string, allowDuplicate?: boolean },
         ): void;
         completionListItemsCountIsGreaterThan(count: number): void;
         completionListIsEmpty(): void;
@@ -358,7 +358,7 @@ declare namespace FourSlashInterface {
         printCurrentFileStateWithoutCaret(): void;
         printCurrentQuickInfo(): void;
         printCurrentSignatureHelp(): void;
-        printCompletionListMembers(): void;
+        printCompletionListMembers(options?: { includeExternalModuleExports: boolean }): void;
         printAvailableCodeFixes(): void;
         printBreakpointLocation(pos: number): void;
         printBreakpointAtCurrentLocation(): void;

diff --git a/tests/cases/fourslash/getJavaScriptCompletions2.ts b/tests/cases/fourslash/getJavaScriptCompletions2.ts
@@ -7,4 +7,4 @@
 ////v./**/
 
 goTo.marker();
-verify.completionListContains("valueOf", /*displayText:*/ undefined, /*documentation*/ undefined, "method");
+verify.completionListContains("valueOf", /*displayText:*/ undefined, /*documentation*/ undefined, "method", undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
diff --git a/tests/cases/fourslash/javaScriptClass1.ts b/tests/cases/fourslash/javaScriptClass1.ts
@@ -28,4 +28,4 @@ verify.completionListContains("substr", /*displayText*/ undefined, /*documentati
 edit.backspace('bar.'.length);
 
 edit.insert('union.');
-verify.completionListContains("toString", /*displayText*/ undefined, /*documentation*/ undefined, "method");
+verify.completionListContains("toString", /*displayText*/ undefined, /*documentation*/ undefined, "method", undefined, undefined, { allowDuplicate: true });