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

Fix containsPrecedingToken for tokens whose preceding token is a missing node #31568

Merged
merged 2 commits into from May 24, 2019
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/harness/fourslash.ts
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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 */
Expand Down
20 changes: 15 additions & 5 deletions src/services/signatureHelp.ts
Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions tests/cases/fourslash/fourslash.ts
Expand Up @@ -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;
Expand Down Expand Up @@ -538,7 +538,7 @@ declare namespace FourSlashInterface {
}

interface VerifySignatureHelpOptions {
marker?: ArrayOrSingle<string>;
marker?: ArrayOrSingle<string | Marker>;
/** @default 1 */
overloadsCount?: number;
docComment?: string;
Expand Down
10 changes: 10 additions & 0 deletions 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: "<" });