Merge pull request #8942 from Microsoft/param_trailing_commas

Allow trailing commas in function parameter and argument lists

src/compiler/checker.ts

@@ -10322,8 +10322,8 @@ namespace ts {
             return -1;
         }
 
-        function hasCorrectArity(node: CallLikeExpression, args: Expression[], signature: Signature) {
-            let adjustedArgCount: number;            // Apparent number of arguments we will have in this call
+        function hasCorrectArity(node: CallLikeExpression, args: Expression[], signature: Signature, signatureHelpTrailingComma = false) {
+            let argCount: number;            // Apparent number of arguments we will have in this call
             let typeArguments: NodeArray<TypeNode>;  // Type arguments (undefined if none)
             let callIsIncomplete: boolean;           // In incomplete call we want to be lenient when we have too few arguments
             let isDecorator: boolean;
@@ -10334,7 +10334,7 @@ namespace ts {
 
                 // Even if the call is incomplete, we'll have a missing expression as our last argument,
                 // so we can say the count is just the arg list length
-                adjustedArgCount = args.length;
+                argCount = args.length;
                 typeArguments = undefined;
 
                 if (tagExpression.template.kind === SyntaxKind.TemplateExpression) {
@@ -10357,7 +10357,7 @@ namespace ts {
             else if (node.kind === SyntaxKind.Decorator) {
                 isDecorator = true;
                 typeArguments = undefined;
-                adjustedArgCount = getEffectiveArgumentCount(node, /*args*/ undefined, signature);
+                argCount = getEffectiveArgumentCount(node, /*args*/ undefined, signature);
             }
             else {
                 const callExpression = <CallExpression>node;
@@ -10368,8 +10368,7 @@ namespace ts {
                     return signature.minArgumentCount === 0;
                 }
 
-                // For IDE scenarios we may have an incomplete call, so a trailing comma is tantamount to adding another argument.
-                adjustedArgCount = callExpression.arguments.hasTrailingComma ? args.length + 1 : args.length;
+                argCount = signatureHelpTrailingComma ? args.length + 1 : args.length;
 
                 // If we are missing the close paren, the call is incomplete.
                 callIsIncomplete = (<CallExpression>callExpression).arguments.end === callExpression.end;
@@ -10393,12 +10392,12 @@ namespace ts {
             }
 
             // Too many arguments implies incorrect arity.
-            if (!signature.hasRestParameter && adjustedArgCount > signature.parameters.length) {
+            if (!signature.hasRestParameter && argCount > signature.parameters.length) {
                 return false;
             }
 
             // If the call is incomplete, we should skip the lower bound check.
-            const hasEnoughArguments = adjustedArgCount >= signature.minArgumentCount;
+            const hasEnoughArguments = argCount >= signature.minArgumentCount;
             return callIsIncomplete || hasEnoughArguments;
         }
 
@@ -10970,6 +10969,11 @@ namespace ts {
             let resultOfFailedInference: InferenceContext;
             let result: Signature;
 
+            // If we are in signature help, a trailing comma indicates that we intend to provide another argument,
+            // so we will only accept overloads with arity at least 1 higher than the current number of provided arguments.
+            const signatureHelpTrailingComma =
+                candidatesOutArray && node.kind === SyntaxKind.CallExpression && (<CallExpression>node).arguments.hasTrailingComma;
+
             // Section 4.12.1:
             // if the candidate list contains one or more signatures for which the type of each argument
             // expression is a subtype of each corresponding parameter type, the return type of the first
@@ -10981,14 +10985,14 @@ namespace ts {
             // is just important for choosing the best signature. So in the case where there is only one
             // signature, the subtype pass is useless. So skipping it is an optimization.
             if (candidates.length > 1) {
-                result = chooseOverload(candidates, subtypeRelation);
+                result = chooseOverload(candidates, subtypeRelation, signatureHelpTrailingComma);
             }
             if (!result) {
                 // Reinitialize these pointers for round two
                 candidateForArgumentError = undefined;
                 candidateForTypeArgumentError = undefined;
                 resultOfFailedInference = undefined;
-                result = chooseOverload(candidates, assignableRelation);
+                result = chooseOverload(candidates, assignableRelation, signatureHelpTrailingComma);
             }
             if (result) {
                 return result;
@@ -11059,9 +11063,9 @@ namespace ts {
                 diagnostics.add(createDiagnosticForNodeFromMessageChain(node, errorInfo));
             }
 
-            function chooseOverload(candidates: Signature[], relation: Map<RelationComparisonResult>) {
+            function chooseOverload(candidates: Signature[], relation: Map<RelationComparisonResult>, signatureHelpTrailingComma = false) {
                 for (const originalCandidate of candidates) {
-                    if (!hasCorrectArity(node, args, originalCandidate)) {
+                    if (!hasCorrectArity(node, args, originalCandidate, signatureHelpTrailingComma)) {
                         continue;
                     }
 
@@ -18028,10 +18032,6 @@ namespace ts {
         }
 
         function checkGrammarParameterList(parameters: NodeArray<ParameterDeclaration>) {
-            if (checkGrammarForDisallowedTrailingComma(parameters)) {
-                return true;
-            }
-
             let seenOptionalParameter = false;
             const parameterCount = parameters.length;
 
@@ -18150,8 +18150,7 @@ namespace ts {
         }
 
         function checkGrammarArguments(node: CallExpression, args: NodeArray<Expression>): boolean {
-            return checkGrammarForDisallowedTrailingComma(args) ||
-                checkGrammarForOmittedArgument(node, args);
+            return checkGrammarForOmittedArgument(node, args);
         }
 
         function checkGrammarHeritageClause(node: HeritageClause): boolean {

tests/baselines/reference/ArrowFunction2.errors.txt

@@ -1,12 +0,0 @@
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction2.ts(1,13): error TS2304: Cannot find name 'b'.
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction2.ts(1,14): error TS1009: Trailing comma not allowed.
-
-
-==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction2.ts (2 errors) ====
-    var v = (a: b,) => {
-                ~
-!!! error TS2304: Cannot find name 'b'.
-                 ~
-!!! error TS1009: Trailing comma not allowed.
-       
-    };

tests/baselines/reference/ArrowFunction2.js

@@ -1,8 +0,0 @@
-//// [ArrowFunction2.ts]
-var v = (a: b,) => {
-   
-};
-
-//// [ArrowFunction2.js]
-var v = function (a) {
-};

tests/baselines/reference/parserErrorRecovery_ArgumentList5.errors.txt

@@ -1,16 +0,0 @@
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArgumentLists/parserErrorRecovery_ArgumentList5.ts(2,4): error TS2304: Cannot find name 'bar'.
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArgumentLists/parserErrorRecovery_ArgumentList5.ts(2,8): error TS2304: Cannot find name 'a'.
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArgumentLists/parserErrorRecovery_ArgumentList5.ts(2,9): error TS1009: Trailing comma not allowed.
-
-
-==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArgumentLists/parserErrorRecovery_ArgumentList5.ts (3 errors) ====
-    function foo() {
-       bar(a,)
-       ~~~
-!!! error TS2304: Cannot find name 'bar'.
-           ~
-!!! error TS2304: Cannot find name 'a'.
-            ~
-!!! error TS1009: Trailing comma not allowed.
-       return;
-    }

tests/baselines/reference/parserErrorRecovery_ArgumentList5.js

@@ -1,11 +0,0 @@
-//// [parserErrorRecovery_ArgumentList5.ts]
-function foo() {
-   bar(a,)
-   return;
-}
-
-//// [parserErrorRecovery_ArgumentList5.js]
-function foo() {
-    bar(a);
-    return;
-}

tests/baselines/reference/parserErrorRecovery_ParameterList3.errors.txt

@@ -1,8 +0,0 @@
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ParameterLists/parserErrorRecovery_ParameterList3.ts(1,13): error TS1009: Trailing comma not allowed.
-
-
-==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ParameterLists/parserErrorRecovery_ParameterList3.ts (1 errors) ====
-    function f(a,) {
-                ~
-!!! error TS1009: Trailing comma not allowed.
-    }

tests/baselines/reference/parserErrorRecovery_ParameterList3.js

@@ -1,7 +0,0 @@
-//// [parserErrorRecovery_ParameterList3.ts]
-function f(a,) {
-}
-
-//// [parserErrorRecovery_ParameterList3.js]
-function f(a) {
-}

tests/baselines/reference/parserParameterList12.errors.txt

@@ -1,8 +0,0 @@
-tests/cases/conformance/parser/ecmascript5/ParameterLists/parserParameterList12.ts(1,13): error TS1009: Trailing comma not allowed.
-
-
-==== tests/cases/conformance/parser/ecmascript5/ParameterLists/parserParameterList12.ts (1 errors) ====
-    function F(a,) {
-                ~
-!!! error TS1009: Trailing comma not allowed.
-    }

tests/baselines/reference/parserParameterList12.symbols

@@ -0,0 +1,5 @@
+=== tests/cases/conformance/parser/ecmascript5/ParameterLists/parserParameterList12.ts ===
+function F(a,) {
+>F : Symbol(F, Decl(parserParameterList12.ts, 0, 0))
+>a : Symbol(a, Decl(parserParameterList12.ts, 0, 11))
+}

tests/baselines/reference/parserParameterList12.types

@@ -0,0 +1,5 @@
+=== tests/cases/conformance/parser/ecmascript5/ParameterLists/parserParameterList12.ts ===
+function F(a,) {
+>F : (a: any) => void
+>a : any
+}

tests/baselines/reference/parserX_ArrowFunction2.errors.txt

@@ -1,12 +0,0 @@
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction2.ts(1,13): error TS2304: Cannot find name 'b'.
-tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction2.ts(1,14): error TS1009: Trailing comma not allowed.
-
-
-==== tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction2.ts (2 errors) ====
-    var v = (a: b,) => {
-                ~
-!!! error TS2304: Cannot find name 'b'.
-                 ~
-!!! error TS1009: Trailing comma not allowed.
-       
-    };

tests/baselines/reference/parserX_ArrowFunction2.js

@@ -1,8 +0,0 @@
-//// [parserX_ArrowFunction2.ts]
-var v = (a: b,) => {
-   
-};
-
-//// [parserX_ArrowFunction2.js]
-var v = function (a) {
-};

tests/baselines/reference/trailingCommasInFunctionParametersAndArguments.js

@@ -0,0 +1,56 @@
+//// [trailingCommasInFunctionParametersAndArguments.ts]
+
+function f1(x,) {}
+
+f1(1,);
+
+function f2(...args,) {}
+
+f2(...[],);
+
+// Not confused by overloads
+declare function f3(x, ): number;
+declare function f3(x, y,): string;
+
+<number>f3(1,);
+<string>f3(1, 2,);
+
+// Works for constructors too
+class X {
+    constructor(a,) { }
+    // See trailingCommasInGetter.ts
+    set x(value,) { }
+}
+interface Y {
+    new(x,);
+    (x,);
+}
+
+new X(1,);
+
+
+//// [trailingCommasInFunctionParametersAndArguments.js]
+function f1(x) { }
+f1(1);
+function f2() {
+    var args = [];
+    for (var _i = 0; _i < arguments.length; _i++) {
+        args[_i - 0] = arguments[_i];
+    }
+}
+f2.apply(void 0, []);
+f3(1);
+f3(1, 2);
+// Works for constructors too
+var X = (function () {
+    function X(a) {
+    }
+    Object.defineProperty(X.prototype, "x", {
+        // See trailingCommasInGetter.ts
+        set: function (value) { },
+        enumerable: true,
+        configurable: true
+    });
+    return X;
+}());
+new X(1);

tests/baselines/reference/trailingCommasInFunctionParametersAndArguments.symbols

@@ -0,0 +1,57 @@
+=== tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts ===
+
+function f1(x,) {}
+>f1 : Symbol(f1, Decl(trailingCommasInFunctionParametersAndArguments.ts, 0, 0))
+>x : Symbol(x, Decl(trailingCommasInFunctionParametersAndArguments.ts, 1, 12))
+
+f1(1,);
+>f1 : Symbol(f1, Decl(trailingCommasInFunctionParametersAndArguments.ts, 0, 0))
+
+function f2(...args,) {}
+>f2 : Symbol(f2, Decl(trailingCommasInFunctionParametersAndArguments.ts, 3, 7))
+>args : Symbol(args, Decl(trailingCommasInFunctionParametersAndArguments.ts, 5, 12))
+
+f2(...[],);
+>f2 : Symbol(f2, Decl(trailingCommasInFunctionParametersAndArguments.ts, 3, 7))
+
+// Not confused by overloads
+declare function f3(x, ): number;
+>f3 : Symbol(f3, Decl(trailingCommasInFunctionParametersAndArguments.ts, 7, 11), Decl(trailingCommasInFunctionParametersAndArguments.ts, 10, 33))
+>x : Symbol(x, Decl(trailingCommasInFunctionParametersAndArguments.ts, 10, 20))
+
+declare function f3(x, y,): string;
+>f3 : Symbol(f3, Decl(trailingCommasInFunctionParametersAndArguments.ts, 7, 11), Decl(trailingCommasInFunctionParametersAndArguments.ts, 10, 33))
+>x : Symbol(x, Decl(trailingCommasInFunctionParametersAndArguments.ts, 11, 20))
+>y : Symbol(y, Decl(trailingCommasInFunctionParametersAndArguments.ts, 11, 22))
+
+<number>f3(1,);
+>f3 : Symbol(f3, Decl(trailingCommasInFunctionParametersAndArguments.ts, 7, 11), Decl(trailingCommasInFunctionParametersAndArguments.ts, 10, 33))
+
+<string>f3(1, 2,);
+>f3 : Symbol(f3, Decl(trailingCommasInFunctionParametersAndArguments.ts, 7, 11), Decl(trailingCommasInFunctionParametersAndArguments.ts, 10, 33))
+
+// Works for constructors too
+class X {
+>X : Symbol(X, Decl(trailingCommasInFunctionParametersAndArguments.ts, 14, 18))
+
+    constructor(a,) { }
+>a : Symbol(a, Decl(trailingCommasInFunctionParametersAndArguments.ts, 18, 16))
+
+    // See trailingCommasInGetter.ts
+    set x(value,) { }
+>x : Symbol(X.x, Decl(trailingCommasInFunctionParametersAndArguments.ts, 18, 23))
+>value : Symbol(value, Decl(trailingCommasInFunctionParametersAndArguments.ts, 20, 10))
+}
+interface Y {
+>Y : Symbol(Y, Decl(trailingCommasInFunctionParametersAndArguments.ts, 21, 1))
+
+    new(x,);
+>x : Symbol(x, Decl(trailingCommasInFunctionParametersAndArguments.ts, 23, 8))
+
+    (x,);
+>x : Symbol(x, Decl(trailingCommasInFunctionParametersAndArguments.ts, 24, 5))
+}
+
+new X(1,);
+>X : Symbol(X, Decl(trailingCommasInFunctionParametersAndArguments.ts, 14, 18))
+

tests/baselines/reference/trailingCommasInFunctionParametersAndArguments.types

@@ -0,0 +1,71 @@
+=== tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts ===
+
+function f1(x,) {}
+>f1 : (x: any) => void
+>x : any
+
+f1(1,);
+>f1(1,) : void
+>f1 : (x: any) => void
+>1 : number
+
+function f2(...args,) {}
+>f2 : (...args: any[]) => void
+>args : any[]
+
+f2(...[],);
+>f2(...[],) : void
+>f2 : (...args: any[]) => void
+>...[] : undefined
+>[] : undefined[]
+
+// Not confused by overloads
+declare function f3(x, ): number;
+>f3 : { (x: any): number; (x: any, y: any): string; }
+>x : any
+
+declare function f3(x, y,): string;
+>f3 : { (x: any): number; (x: any, y: any): string; }
+>x : any
+>y : any
+
+<number>f3(1,);
+><number>f3(1,) : number
+>f3(1,) : number
+>f3 : { (x: any): number; (x: any, y: any): string; }
+>1 : number
+
+<string>f3(1, 2,);
+><string>f3(1, 2,) : string
+>f3(1, 2,) : string
+>f3 : { (x: any): number; (x: any, y: any): string; }
+>1 : number
+>2 : number
+
+// Works for constructors too
+class X {
+>X : X
+
+    constructor(a,) { }
+>a : any
+
+    // See trailingCommasInGetter.ts
+    set x(value,) { }
+>x : any
+>value : any
+}
+interface Y {
+>Y : Y
+
+    new(x,);
+>x : any
+
+    (x,);
+>x : any
+}
+
+new X(1,);
+>new X(1,) : X
+>X : typeof X
+>1 : number
+

tests/baselines/reference/trailingCommasInGetter.errors.txt

@@ -0,0 +1,10 @@
+tests/cases/conformance/es7/trailingCommasInGetter.ts(2,11): error TS1138: Parameter declaration expected.
+
+
+==== tests/cases/conformance/es7/trailingCommasInGetter.ts (1 errors) ====
+    class X {
+        get x(,) { return 0; }
+              ~
+!!! error TS1138: Parameter declaration expected.
+    }
+    

tests/baselines/reference/trailingCommasInGetter.js

@@ -0,0 +1,17 @@
+//// [trailingCommasInGetter.ts]
+class X {
+    get x(,) { return 0; }
+}
+
+
+//// [trailingCommasInGetter.js]
+var X = (function () {
+    function X() {
+    }
+    Object.defineProperty(X.prototype, "x", {
+        get: function () { return 0; },
+        enumerable: true,
+        configurable: true
+    });
+    return X;
+}());

tests/baselines/reference/trailingSeparatorInFunctionCall.errors.txt

@@ -1,24 +0,0 @@
-tests/cases/compiler/trailingSeparatorInFunctionCall.ts(4,1): error TS2346: Supplied parameters do not match any signature of call target.
-tests/cases/compiler/trailingSeparatorInFunctionCall.ts(4,7): error TS1009: Trailing comma not allowed.
-tests/cases/compiler/trailingSeparatorInFunctionCall.ts(9,1): error TS2346: Supplied parameters do not match any signature of call target.
-tests/cases/compiler/trailingSeparatorInFunctionCall.ts(9,8): error TS1009: Trailing comma not allowed.
-
-
-==== tests/cases/compiler/trailingSeparatorInFunctionCall.ts (4 errors) ====
-    function f(x, y) {
-    }
-    
-    f(1, 2, );
-    ~~~~~~~~~
-!!! error TS2346: Supplied parameters do not match any signature of call target.
-          ~
-!!! error TS1009: Trailing comma not allowed.
-    
-    function f2<T>(x: T, y: T) {
-    }
-    
-    f2(1, 2, );
-    ~~~~~~~~~~
-!!! error TS2346: Supplied parameters do not match any signature of call target.
-           ~
-!!! error TS1009: Trailing comma not allowed.

tests/baselines/reference/trailingSeparatorInFunctionCall.js

@@ -1,18 +0,0 @@
-//// [trailingSeparatorInFunctionCall.ts]
-function f(x, y) {
-}
-
-f(1, 2, );
-
-function f2<T>(x: T, y: T) {
-}
-
-f2(1, 2, );
-
-//// [trailingSeparatorInFunctionCall.js]
-function f(x, y) {
-}
-f(1, 2);
-function f2(x, y) {
-}
-f2(1, 2);

tests/cases/compiler/trailingSeparatorInFunctionCall.ts

@@ -1,9 +0,0 @@
-function f(x, y) {
-}
-
-f(1, 2, );
-
-function f2<T>(x: T, y: T) {
-}
-
-f2(1, 2, );

tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts

@@ -0,0 +1,29 @@
+// @target: es5
+
+function f1(x,) {}
+
+f1(1,);
+
+function f2(...args,) {}
+
+f2(...[],);
+
+// Not confused by overloads
+declare function f3(x, ): number;
+declare function f3(x, y,): string;
+
+<number>f3(1,);
+<string>f3(1, 2,);
+
+// Works for constructors too
+class X {
+    constructor(a,) { }
+    // See trailingCommasInGetter.ts
+    set x(value,) { }
+}
+interface Y {
+    new(x,);
+    (x,);
+}
+
+new X(1,);

tests/cases/conformance/es7/trailingCommasInGetter.ts

@@ -0,0 +1,3 @@
+class X {
+    get x(,) { return 0; }
+}

tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArgumentLists/parserErrorRecovery_ArgumentList5.ts

@@ -1,4 +0,0 @@
-function foo() {
-   bar(a,)
-   return;
-}

tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/ArrowFunction2.ts

@@ -1,3 +0,0 @@
-var v = (a: b,) => {
-   
-};

tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ArrowFunctions/parserX_ArrowFunction2.ts

@@ -1,3 +0,0 @@
-var v = (a: b,) => {
-   
-};

tests/cases/conformance/parser/ecmascript5/ErrorRecovery/ParameterLists/parserErrorRecovery_ParameterList3.ts

@@ -1,2 +0,0 @@
-function f(a,) {
-}

tests/cases/fourslash/trailingCommaSignatureHelp.ts

@@ -0,0 +1,15 @@
+/// <reference path="fourslash.ts" />
+
+////function str(n: number): string;
+/////**
+//// * Stringifies a number with radix
+//// * @param radix The radix
+//// */
+////function str(n: number, radix: number): string;
+////function str(n: number, radix?: number): string { return ""; }
+
+edit.insert("str(1,");
+verify.currentParameterHelpArgumentNameIs("radix");
+verify.currentParameterHelpArgumentDocCommentIs("The radix");
+verify.currentSignatureHelpIs("str(n: number, radix: number): string");
+verify.currentSignatureHelpDocCommentIs("Stringifies a number with radix");