Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect re-exports from "export *" in completions #20043

Merged
1 commit merged into from
Nov 16, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 36 additions & 35 deletions src/harness/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -4555,6 +4555,7 @@ namespace FourSlashInterface {

export interface VerifyCompletionListContainsOptions extends ts.GetCompletionsAtPositionOptions {
sourceDisplay: string;
allowDuplicate: boolean; // TODO: GH#20042
}

export interface NewContentOptions {
Expand Down
13 changes: 8 additions & 5 deletions src/services/completions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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 };
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion tests/cases/fourslash/completionInJSDocFunctionNew.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,4 @@

goTo.marker();
verify.completionListCount(118);
verify.completionListContains('new');
verify.completionListContains('new', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
3 changes: 1 addition & 2 deletions tests/cases/fourslash/completionInJSDocFunctionThis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,4 @@

goTo.marker();
verify.completionListCount(119);
verify.completionListContains('this');

verify.completionListContains('this', undefined, undefined, undefined, undefined, undefined, { allowDuplicate: true }); // TODO: GH#20042
4 changes: 2 additions & 2 deletions tests/cases/fourslash/completionListPrimitives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/// <reference path="fourslash.ts" />

// @noLib: true

// @Filename: /a.ts
////export default function foo() {}

Expand Down
5 changes: 5 additions & 0 deletions tests/cases/fourslash/completionsImport_ofAlias.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions tests/cases/fourslash/fourslash.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion tests/cases/fourslash/getJavaScriptCompletions2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion tests/cases/fourslash/javaScriptClass1.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 });