Respond to PR comments

Author: JsonFreeman

src/compiler/checker.ts

@@ -4137,6 +4137,10 @@ module ts {
 
             // For error recovery, since we have parsed OmittedExpressions for any extra commas
             // in the argument list, if we see any OmittedExpressions, just return true.
+            // The reason this is ok is because omitted expressions here are syntactically
+            // illegal, and will cause a parse error.
+            // Note: It may be worth keeping the upper bound check on arity, but removing
+            // the lower bound check if there are omitted expressions.
             if (!isCorrect && forEach(node.arguments, arg => arg.kind === SyntaxKind.OmittedExpression)) {
                 return true;
             }
@@ -4287,7 +4291,7 @@ module ts {
             // If candidate is undefined, it means that no candidates had a suitable arity. In that case,
             // skip the checkApplicableSignature check.
             if (candidateWithCorrectArity) {
-                checkApplicableSignature(node, candidateWithCorrectArity, relation, undefined, /*reportErrors*/ true);
+                checkApplicableSignature(node, candidateWithCorrectArity, relation, /*excludeArgument*/ undefined, /*reportErrors*/ true);
             }
             else {
                 error(node, Diagnostics.Supplied_parameters_do_not_match_any_signature_of_call_target);

src/compiler/core.ts

@@ -121,10 +121,6 @@ module ts {
         }
 
         return ~low;
-    } 
-    
-    export function integerDivide(numerator: number, denominator: number): number {
-        return (numerator / denominator) >> 0;
     }
 
     var hasOwnProperty = Object.prototype.hasOwnProperty;

src/compiler/types.ts

@@ -1174,7 +1174,6 @@ module ts {
 
     export interface CancellationToken {
         isCancellationRequested(): boolean;
-        throwIfCancellationRequested?(): void;
     }
 
     export interface CompilerHost {

src/harness/fourslash.ts

@@ -959,7 +959,7 @@ module FourSlash {
             var state = this.languageService.getSignatureHelpCurrentArgumentState(this.activeFile.fileName, this.currentCaretPosition, help.applicableSpan.start());
 
             // Same logic as in getActiveSignatureHelp - this value might be -1 until a parameter value actually gets typed
-            var currentParam = state === null || state.argumentIndex < 0 ? 0 : state.argumentIndex;
+            var currentParam = state === undefined ? 0 : state.argumentIndex;
             return item.parameters[currentParam];
         }
 

src/services/services.ts

@@ -966,7 +966,7 @@ module ts {
 
     export class OperationCanceledException { }
 
-    class CancellationTokenObject {
+    export class CancellationTokenObject {
 
         public static None: CancellationTokenObject = new CancellationTokenObject(null)
 
@@ -3480,6 +3480,7 @@ module ts {
         }
 
         // Signature help
+        // This is a semantic operation
         function getSignatureHelpItems(fileName: string, position: number): SignatureHelpItems {
             synchronizeHostData();
 
@@ -3490,6 +3491,7 @@ module ts {
             return SignatureHelp.getSignatureHelpItems(sourceFile, position, node, typeInfoResolver, cancellationToken);
         }
 
+        // This is a syntactic operation
         function getSignatureHelpCurrentArgumentState(fileName: string, position: number, applicableSpanStart: number): SignatureHelpState {
             fileName = TypeScript.switchToForwardSlashes(fileName);
             var sourceFile = getCurrentSourceFile(fileName);

src/services/signatureHelp.ts

@@ -333,25 +333,26 @@ module ts.SignatureHelp {
     //}
     var emptyArray: any[] = [];
 
-    export function getSignatureHelpItems(sourceFile: SourceFile, position: number, startingNode: Node, typeInfoResolver: TypeChecker, cancellationToken: CancellationToken): SignatureHelpItems {
+    export function getSignatureHelpItems(sourceFile: SourceFile, position: number, startingNode: Node, typeInfoResolver: TypeChecker, cancellationToken: CancellationTokenObject): SignatureHelpItems {
         // Decide whether to show signature help
         var argumentList = getContainingArgumentList(startingNode);
         cancellationToken.throwIfCancellationRequested();
 
         // Semantic filtering of signature help
-        if (argumentList) {
-            var call = <CallExpression>argumentList.parent;
-            var candidates = <Signature[]>[];
-            var resolvedSignature = typeInfoResolver.getResolvedSignature(call, candidates);
-            cancellationToken.throwIfCancellationRequested();
-
-            return candidates.length
-                ? createSignatureHelpItems(candidates, resolvedSignature, argumentList)
-                : undefined;
+        if (!argumentList) {
+            return undefined;
         }
 
-        return undefined;
+        var call = <CallExpression>argumentList.parent;
+        var candidates = <Signature[]>[];
+        var resolvedSignature = typeInfoResolver.getResolvedSignature(call, candidates);
+        cancellationToken.throwIfCancellationRequested();
+
+        if (!candidates.length) {
+            return undefined;
+        }
 
+        return createSignatureHelpItems(candidates, resolvedSignature, argumentList);
 
         // If node is an argument, returns its index in the argument list
         // If not, returns -1
@@ -420,7 +421,7 @@ module ts.SignatureHelp {
                         display += "?";
                     }
                     display += ": " + typeInfoResolver.typeToString(typeInfoResolver.getTypeOfSymbol(p), argumentListOrTypeArgumentList);
-                    return new SignatureHelpParameter(p.name, "", display, /*isOptional*/ false);
+                    return new SignatureHelpParameter(p.name, "", display, isOptional);
                 });
                 var callTargetNode = (<CallExpression>argumentListOrTypeArgumentList.parent).func;
                 var callTargetSymbol = typeInfoResolver.getSymbolInfo(callTargetNode);
@@ -439,6 +440,14 @@ module ts.SignatureHelp {
                 selectedItemIndex = 0;
             }
 
+            // We use full start and skip trivia on the end because we want to include trivia on
+            // both sides. For example,
+            //
+            //    foo(   /*comment */     a, b, c      /*comment*/     )
+            //        |                                               |
+            //
+            // The applicable span is from the first bar to the second bar (inclusive,
+            // but not including parentheses)
             var applicableSpanStart = argumentListOrTypeArgumentList.getFullStart();
             var applicableSpanEnd = skipTrivia(sourceFile.text, argumentListOrTypeArgumentList.end, /*stopAfterLineBreak*/ false);
             var applicableSpan = new TypeScript.TextSpan(applicableSpanStart, applicableSpanEnd - applicableSpanStart);
@@ -459,15 +468,14 @@ module ts.SignatureHelp {
 
         var tokenPrecedingCurrentPosition = ServicesSyntaxUtilities.findPrecedingToken(position, sourceFile);
         var call = <CallExpression>tokenPrecedingSpanStart.parent;
+        Debug.assert(call.kind === SyntaxKind.CallExpression || call.kind === SyntaxKind.NewExpression, "wrong call kind " + SyntaxKind[call.kind]);
         if (tokenPrecedingCurrentPosition.kind === SyntaxKind.CloseParenToken || tokenPrecedingCurrentPosition.kind === SyntaxKind.GreaterThanToken) {
             if (tokenPrecedingCurrentPosition.parent === call) {
                 // This call expression is complete. Stop signature help.
                 return undefined;
             }
         }
 
-        Debug.assert(call.kind === SyntaxKind.CallExpression || call.kind === SyntaxKind.NewExpression, "wrong call kind " + SyntaxKind[call.kind]);
-
         var argumentListOrTypeArgumentList = getChildListThatStartsWithOpenerToken(call, tokenPrecedingSpanStart, sourceFile);
         // Debug.assert(argumentListOrTypeArgumentList.getChildCount() === 0 || argumentListOrTypeArgumentList.getChildCount() % 2 === 1, "Even number of children");
 
@@ -478,8 +486,6 @@ module ts.SignatureHelp {
 
         var numberOfCommas = countWhere(argumentListOrTypeArgumentList.getChildren(), arg => arg.kind === SyntaxKind.CommaToken);
         var argumentCount = numberOfCommas + 1;
-
-
         if (argumentCount <= 1) {
             return new SignatureHelpState(/*argumentIndex*/ 0, argumentCount);
         }
@@ -490,8 +496,8 @@ module ts.SignatureHelp {
         // possible that we are to the right of all children. Assume that we are still within
         // the applicable span and that we are typing the last argument
         // Alternatively, we could be in range of one of the arguments, in which case we need to divide
-        // by 2 to exclude commas
-        var argumentIndex = indexOfNodeContainingPosition < 0 ? argumentCount - 1 : integerDivide(indexOfNodeContainingPosition, 2);
+        // by 2 to exclude commas. Use bit shifting in order to take the floor of the division.
+        var argumentIndex = indexOfNodeContainingPosition < 0 ? argumentCount - 1 : indexOfNodeContainingPosition >> 1;
         return new SignatureHelpState(argumentIndex, argumentCount);
     }