Merge pull request #848 from Microsoft/sigHelpCrash

JsonFreeman · JsonFreeman · commit 37a839fcf79a · 2014-10-07T16:05:35.000-07:00
Add asserts to help diagnose signature help crash #832
diff --git 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
@@ -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
@@ -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;
     }
 
