From f97f57c1556bde40149ef30f034480917c27fce0 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 23 May 2019 12:15:50 -0700 Subject: [PATCH 1/2] Fix containsPrecedingToken for tokens whose preceding token is a missing node --- src/harness/fourslash.ts | 10 +++++----- src/services/signatureHelp.ts | 20 +++++++++++++++----- tests/cases/fourslash/fourslash.ts | 8 ++++---- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/src/harness/fourslash.ts b/src/harness/fourslash.ts index 8a0a2bffbd4d6..bdcb3e082d2ed 100644 --- a/src/harness/fourslash.ts +++ b/src/harness/fourslash.ts @@ -1208,7 +1208,7 @@ Actual: ${stringify(fullActual)}`); } } - public verifySignatureHelpPresence(expectPresent: boolean, triggerReason: ts.SignatureHelpTriggerReason | undefined, markers: ReadonlyArray) { + public verifySignatureHelpPresence(expectPresent: boolean, triggerReason: ts.SignatureHelpTriggerReason | undefined, markers: ReadonlyArray) { if (markers.length) { for (const marker of markers) { this.goToMarker(marker); @@ -3768,15 +3768,15 @@ namespace FourSlashInterface { assert(ranges.length !== 0, "Array of ranges is expected to be non-empty"); } - public noSignatureHelp(...markers: string[]): void { + public noSignatureHelp(...markers: (string | FourSlash.Marker)[]): void { this.state.verifySignatureHelpPresence(/*expectPresent*/ false, /*triggerReason*/ undefined, markers); } - public noSignatureHelpForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: string[]): void { + public noSignatureHelpForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: (string | FourSlash.Marker)[]): void { this.state.verifySignatureHelpPresence(/*expectPresent*/ false, reason, markers); } - public signatureHelpPresentForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: string[]): void { + public signatureHelpPresentForTriggerReason(reason: ts.SignatureHelpTriggerReason, ...markers: (string | FourSlash.Marker)[]): void { this.state.verifySignatureHelpPresence(/*expectPresent*/ true, reason, markers); } @@ -5124,7 +5124,7 @@ namespace FourSlashInterface { } export interface VerifySignatureHelpOptions { - readonly marker?: ArrayOrSingle; + readonly marker?: ArrayOrSingle; /** @default 1 */ readonly overloadsCount?: number; /** @default undefined */ diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 25f958dfc6a75..998e45bd4616c 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -133,11 +133,21 @@ namespace ts.SignatureHelp { } function containsPrecedingToken(startingToken: Node, sourceFile: SourceFile, container: Node) { - const precedingToken = Debug.assertDefined( - findPrecedingToken(startingToken.getFullStart(), sourceFile, startingToken.parent, /*excludeJsdoc*/ true) - ); - - return rangeContainsRange(container, precedingToken); + const pos = startingToken.getFullStart(); + // There’s a possibility that `startingToken.parent` contains only `startingToken` and + // missing nodes, none of which are valid to be returned by `findPrecedingToken`. In that + // case, the preceding token we want is actually higher up the tree—almost definitely the + // next parent, but theoretically the situation with missing nodes might be happening on + // multiple nested levels. + let currentParent: Node | undefined = startingToken.parent; + while (currentParent) { + const precedingToken = findPrecedingToken(pos, sourceFile, currentParent, /*excludeJsdoc*/ true); + if (precedingToken) { + return rangeContainsRange(container, precedingToken); + } + currentParent = currentParent.parent; + } + return Debug.fail("Could not find preceding token"); } export interface ArgumentInfoForCompletions { diff --git a/tests/cases/fourslash/fourslash.ts b/tests/cases/fourslash/fourslash.ts index fa9cdd61c2f6f..b3253bcd59fff 100644 --- a/tests/cases/fourslash/fourslash.ts +++ b/tests/cases/fourslash/fourslash.ts @@ -232,9 +232,9 @@ declare namespace FourSlashInterface { rangesWithSameTextAreRenameLocations(): void; rangesAreRenameLocations(options?: Range[] | { findInStrings?: boolean, findInComments?: boolean, ranges?: Range[] }); findReferencesDefinitionDisplayPartsAtCaretAre(expected: ts.SymbolDisplayPart[]): void; - noSignatureHelp(...markers: string[]): void; - noSignatureHelpForTriggerReason(triggerReason: SignatureHelpTriggerReason, ...markers: string[]): void - signatureHelpPresentForTriggerReason(triggerReason: SignatureHelpTriggerReason, ...markers: string[]): void + noSignatureHelp(...markers: (string | Marker)[]): void; + noSignatureHelpForTriggerReason(triggerReason: SignatureHelpTriggerReason, ...markers: (string | Marker)[]): void + signatureHelpPresentForTriggerReason(triggerReason: SignatureHelpTriggerReason, ...markers: (string | Marker)[]): void signatureHelp(...options: VerifySignatureHelpOptions[], ): void; // Checks that there are no compile errors. noErrors(): void; @@ -538,7 +538,7 @@ declare namespace FourSlashInterface { } interface VerifySignatureHelpOptions { - marker?: ArrayOrSingle; + marker?: ArrayOrSingle; /** @default 1 */ overloadsCount?: number; docComment?: string; From 7359ff8158df18207ff66cb7040509051eab5242 Mon Sep 17 00:00:00 2001 From: Andrew Branch Date: Thu, 23 May 2019 13:33:38 -0700 Subject: [PATCH 2/2] Add test --- tests/cases/fourslash/signatureHelpJSX.ts | 10 ++++++++++ 1 file changed, 10 insertions(+) create mode 100644 tests/cases/fourslash/signatureHelpJSX.ts diff --git a/tests/cases/fourslash/signatureHelpJSX.ts b/tests/cases/fourslash/signatureHelpJSX.ts new file mode 100644 index 0000000000000..181ff85541b72 --- /dev/null +++ b/tests/cases/fourslash/signatureHelpJSX.ts @@ -0,0 +1,10 @@ +/// + +//@Filename: test.tsx +//@jsx: react +////declare var React: any; +////const z =
{[].map(x =>