From 8346b48e6d7b178784a127c94c4625b783fae066 Mon Sep 17 00:00:00 2001 From: Jason Freeman Date: Tue, 7 Oct 2014 13:37:57 -0700 Subject: [PATCH] Add assserts to help diagnose signature help crash #832 --- src/compiler/parser.ts | 6 ------ src/services/signatureHelp.ts | 6 ++++++ src/services/utilities.ts | 9 ++++++++- 3 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts index a74ceb4e024..a43f31e583a 100644 --- a/src/compiler/parser.ts +++ b/src/compiler/parser.ts @@ -2397,9 +2397,6 @@ module ts { else { parseExpected(SyntaxKind.OpenParenToken); } - // It is an error to have a trailing comma in an argument list. However, the checker - // needs evidence of a trailing comma in order to give good results for signature help. - // That is why we do not allow a trailing comma, but we "preserve" a trailing comma. callExpr.arguments = parseDelimitedList(ParsingContext.ArgumentExpressions, parseArgumentExpression, /*allowTrailingComma*/ false); parseExpected(SyntaxKind.CloseParenToken); @@ -2627,9 +2624,6 @@ module ts { parseExpected(SyntaxKind.NewKeyword); node.func = parseCallAndAccess(parsePrimaryExpression(), /* inNewExpression */ true); if (parseOptional(SyntaxKind.OpenParenToken) || token === SyntaxKind.LessThanToken && (node.typeArguments = tryParse(parseTypeArgumentsAndOpenParen))) { - // It is an error to have a trailing comma in an argument list. However, the checker - // needs evidence of a trailing comma in order to give good results for signature help. - // That is why we do not allow a trailing comma, but we "preserve" a trailing comma. node.arguments = parseDelimitedList(ParsingContext.ArgumentExpressions, parseArgumentExpression, /*allowTrailingComma*/ false); parseExpected(SyntaxKind.CloseParenToken); diff --git a/src/services/signatureHelp.ts b/src/services/signatureHelp.ts index 080e4704b83..189d8d2e727 100644 --- a/src/services/signatureHelp.ts +++ b/src/services/signatureHelp.ts @@ -241,6 +241,12 @@ module ts.SignatureHelp { return undefined; } + // If the node is not a subspan of its parent, this is a big problem. + // There have been crashes that might be caused by this violation. + if (n.pos < n.parent.pos || n.end > n.parent.end) { + Debug.fail("Node of kind " + SyntaxKind[n.kind] + " is not a subspan of its parent of kind " + SyntaxKind[n.parent.kind]); + } + var argumentInfo = getImmediatelyContainingArgumentInfo(n); if (argumentInfo) { return argumentInfo; diff --git a/src/services/utilities.ts b/src/services/utilities.ts index af320ea0a0c..4ceb20fdcee 100644 --- a/src/services/utilities.ts +++ b/src/services/utilities.ts @@ -27,11 +27,18 @@ module ts { // for the position of the relevant node (or comma). var syntaxList = forEach(node.parent.getChildren(), c => { // find syntax list that covers the span of the node - if (c.kind == SyntaxKind.SyntaxList && c.pos <= node.pos && c.end >= node.end) { + if (c.kind === SyntaxKind.SyntaxList && c.pos <= node.pos && c.end >= node.end) { return c; } }); + // syntaxList should not be undefined here. If it is, there is a problem. Find out if + // there at least is a child that is a list. + if (!syntaxList) { + Debug.assert(findChildOfKind(node.parent, SyntaxKind.SyntaxList), + "Node of kind " + SyntaxKind[node.parent.kind] + " has no list children"); + } + return syntaxList; }