Merge pull request #31568 from andrewbranch/bug/31347

Fix containsPrecedingToken for tokens whose preceding token is a missing node
microsoft · May 24, 2019 · 9380b9f · 9380b9f
2 parents 7ff97d1 + 7359ff8
commit 9380b9f
Show file tree

Hide file tree

Showing 4 changed files with 34 additions and 14 deletions.
diff --git 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<string>) {
+        public verifySignatureHelpPresence(expectPresent: boolean, triggerReason: ts.SignatureHelpTriggerReason | undefined, markers: ReadonlyArray<string | Marker>) {
             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<string>;
+        readonly marker?: ArrayOrSingle<string | FourSlash.Marker>;
         /** @default 1 */
         readonly overloadsCount?: number;
         /** @default undefined */

diff --git 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
@@ -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<string>;
+        marker?: ArrayOrSingle<string | Marker>;
         /** @default 1 */
         overloadsCount?: number;
         docComment?: string;

diff --git a/tests/cases/fourslash/signatureHelpJSX.ts b/tests/cases/fourslash/signatureHelpJSX.ts
@@ -0,0 +1,10 @@
+/// <reference path="fourslash.ts" />
+
+//@Filename: test.tsx
+//@jsx: react
+////declare var React: any;
+////const z = <div>{[].map(x => </**/
+
+// This test exists because it used to crash: #31347
+goTo.marker();
+verify.noSignatureHelpForTriggerReason({ kind: "characterTyped", triggerCharacter: "<" });