Error on rest parameter with trailing comma (#22262)

* Error on rest parameter with trailing comma

* Error on binding patterns and improve error location

Author: Andy

src/compiler/checker.ts

@@ -19164,6 +19164,7 @@ namespace ts {
 
         function checkObjectLiteralAssignment(node: ObjectLiteralExpression, sourceType: Type): Type {
             const properties = node.properties;
+            checkGrammarForDisallowedTrailingComma(properties, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
             if (strictNullChecks && properties.length === 0) {
                 return checkNonNullType(sourceType, node);
             }
@@ -19222,6 +19223,8 @@ namespace ts {
         }
 
         function checkArrayLiteralAssignment(node: ArrayLiteralExpression, sourceType: Type, checkMode?: CheckMode): Type {
+            const elements = node.elements;
+            checkGrammarForDisallowedTrailingComma(elements, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
             if (languageVersion < ScriptTarget.ES2015 && compilerOptions.downlevelIteration) {
                 checkExternalEmitHelpers(node, ExternalEmitHelpers.Read);
             }
@@ -19230,7 +19233,6 @@ namespace ts {
             // present (aka the tuple element property). This call also checks that the parentType is in
             // fact an iterable or array (depending on target language).
             const elementType = checkIteratedTypeOrElementType(sourceType, node, /*allowStringInput*/ false, /*allowAsyncIterables*/ false) || unknownType;
-            const elements = node.elements;
             for (let i = 0; i < elements.length; i++) {
                 checkArrayLiteralDestructuringElementAssignment(node, sourceType, i, elementType, checkMode);
             }
@@ -26500,11 +26502,9 @@ namespace ts {
             return grammarErrorOnNode(asyncModifier, Diagnostics._0_modifier_cannot_be_used_here, "async");
         }
 
-        function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>): boolean {
+        function checkGrammarForDisallowedTrailingComma(list: NodeArray<Node>, diag = Diagnostics.Trailing_comma_not_allowed): boolean {
             if (list && list.hasTrailingComma) {
-                const start = list.end - ",".length;
-                const end = list.end;
-                return grammarErrorAtPos(list[0], start, end - start, Diagnostics.Trailing_comma_not_allowed);
+                return grammarErrorAtPos(list[0], list.end - ",".length, ",".length, diag);
             }
         }
 
@@ -26526,6 +26526,7 @@ namespace ts {
                     if (i !== (parameterCount - 1)) {
                         return grammarErrorOnNode(parameter.dotDotDotToken, Diagnostics.A_rest_parameter_must_be_last_in_a_parameter_list);
                     }
+                    checkGrammarForDisallowedTrailingComma(parameters, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
 
                     if (isBindingPattern(parameter.name)) {
                         return grammarErrorOnNode(parameter.name, Diagnostics.A_rest_element_cannot_contain_a_binding_pattern);
@@ -27140,6 +27141,7 @@ namespace ts {
                 if (node !== last(elements)) {
                     return grammarErrorOnNode(node, Diagnostics.A_rest_element_must_be_last_in_a_destructuring_pattern);
                 }
+                checkGrammarForDisallowedTrailingComma(elements, Diagnostics.A_rest_parameter_or_binding_pattern_may_not_have_a_trailing_comma);
 
                 if (node.name.kind === SyntaxKind.ArrayBindingPattern || node.name.kind === SyntaxKind.ObjectBindingPattern) {
                     return grammarErrorOnNode(node.name, Diagnostics.A_rest_element_cannot_contain_a_binding_pattern);

src/compiler/diagnosticMessages.json

@@ -27,6 +27,10 @@
         "category": "Error",
         "code": 1012
     },
+    "A rest parameter or binding pattern may not have a trailing comma.": {
+        "category": "Error",
+        "code": 1013
+    },
     "A rest parameter must be last in a parameter list.": {
         "category": "Error",
         "code": 1014

tests/baselines/reference/trailingCommasInBindingPatterns.errors.txt

@@ -0,0 +1,21 @@
+tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(1,12): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(2,12): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(4,7): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts(5,7): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+
+
+==== tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts (4 errors) ====
+    const [...a,] = [];
+               ~
+!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+    const {...b,} = {};
+               ~
+!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+    let c, d;
+    ([...c,] = []);
+          ~
+!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+    ({...d,} = {});
+          ~
+!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+    

tests/baselines/reference/trailingCommasInBindingPatterns.js

@@ -0,0 +1,23 @@
+//// [trailingCommasInBindingPatterns.ts]
+const [...a,] = [];
+const {...b,} = {};
+let c, d;
+([...c,] = []);
+({...d,} = {});
+
+
+//// [trailingCommasInBindingPatterns.js]
+var __rest = (this && this.__rest) || function (s, e) {
+    var t = {};
+    for (var p in s) if (Object.prototype.hasOwnProperty.call(s, p) && e.indexOf(p) < 0)
+        t[p] = s[p];
+    if (s != null && typeof Object.getOwnPropertySymbols === "function")
+        for (var i = 0, p = Object.getOwnPropertySymbols(s); i < p.length; i++) if (e.indexOf(p[i]) < 0)
+            t[p[i]] = s[p[i]];
+    return t;
+};
+var a = [].slice(0);
+var b = __rest({}, []);
+var c, d;
+(c = [].slice(0));
+(d = __rest({}, []));

tests/baselines/reference/trailingCommasInBindingPatterns.symbols

@@ -0,0 +1,17 @@
+=== tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts ===
+const [...a,] = [];
+>a : Symbol(a, Decl(trailingCommasInBindingPatterns.ts, 0, 7))
+
+const {...b,} = {};
+>b : Symbol(b, Decl(trailingCommasInBindingPatterns.ts, 1, 7))
+
+let c, d;
+>c : Symbol(c, Decl(trailingCommasInBindingPatterns.ts, 2, 3))
+>d : Symbol(d, Decl(trailingCommasInBindingPatterns.ts, 2, 6))
+
+([...c,] = []);
+>c : Symbol(c, Decl(trailingCommasInBindingPatterns.ts, 2, 3))
+
+({...d,} = {});
+>d : Symbol(d, Decl(trailingCommasInBindingPatterns.ts, 2, 6))
+

tests/baselines/reference/trailingCommasInBindingPatterns.types

@@ -0,0 +1,28 @@
+=== tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts ===
+const [...a,] = [];
+>a : any[]
+>[] : undefined[]
+
+const {...b,} = {};
+>b : {}
+>{} : {}
+
+let c, d;
+>c : any
+>d : any
+
+([...c,] = []);
+>([...c,] = []) : undefined[]
+>[...c,] = [] : undefined[]
+>[...c,] : undefined[]
+>...c : any
+>c : any
+>[] : undefined[]
+
+({...d,} = {});
+>({...d,} = {}) : {}
+>{...d,} = {} : {}
+>{...d,} : any
+>d : any
+>{} : {}
+

tests/baselines/reference/trailingCommasInFunctionParametersAndArguments.errors.txt

@@ -0,0 +1,34 @@
+tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts(5,20): error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+
+
+==== tests/cases/conformance/es7/trailingCommasInFunctionParametersAndArguments.ts (1 errors) ====
+    function f1(x,) {}
+    
+    f1(1,);
+    
+    function f2(...args,) {}
+                       ~
+!!! error TS1013: A rest parameter or binding pattern may not have a trailing comma.
+    
+    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/baselines/reference/unusedLocalsAndObjectSpread2.errors.txt

@@ -1,14 +1,14 @@
 tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(5,3): error TS6133: 'rest' is declared but its value is never read.
 tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(8,1): error TS6133: 'foo' is declared but its value is never read.
-tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(12,5): error TS6133: 'rest' is declared but its value is never read.
+tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(12,9): error TS6133: 'rest' is declared but its value is never read.
 
 
 ==== tests/cases/compiler/unusedLocalsAndObjectSpread2.ts (3 errors) ====
     declare let props: any;
     const {
         children, // here!
         active: _a, // here!
-      ...rest,
+      ...rest
       ~~~~~~~
 !!! error TS6133: 'rest' is declared but its value is never read.
     } = props;
@@ -19,10 +19,10 @@ tests/cases/compiler/unusedLocalsAndObjectSpread2.ts(12,5): error TS6133: 'rest'
         const {
             children,
             active: _a,
-        ...rest,
-        ~~~~~~~
+            ...rest
+            ~~~~~~~
 !!! error TS6133: 'rest' is declared but its value is never read.
-      } = props;
+        } = props;
     }
 
     export const asdf = 123;

tests/baselines/reference/unusedLocalsAndObjectSpread2.js

@@ -3,15 +3,15 @@ declare let props: any;
 const {
     children, // here!
     active: _a, // here!
-  ...rest,
+  ...rest
 } = props;
 
 function foo() {
     const {
         children,
         active: _a,
-    ...rest,
-  } = props;
+        ...rest
+    } = props;
 }
 
 export const asdf = 123;

tests/baselines/reference/unusedLocalsAndObjectSpread2.symbols

@@ -9,7 +9,7 @@ const {
     active: _a, // here!
 >_a : Symbol(_a, Decl(unusedLocalsAndObjectSpread2.ts, 2, 13))
 
-  ...rest,
+  ...rest
 >rest : Symbol(rest, Decl(unusedLocalsAndObjectSpread2.ts, 3, 15))
 
 } = props;
@@ -25,10 +25,10 @@ function foo() {
         active: _a,
 >_a : Symbol(_a, Decl(unusedLocalsAndObjectSpread2.ts, 9, 17))
 
-    ...rest,
+        ...rest
 >rest : Symbol(rest, Decl(unusedLocalsAndObjectSpread2.ts, 10, 19))
 
-  } = props;
+    } = props;
 >props : Symbol(props, Decl(unusedLocalsAndObjectSpread2.ts, 0, 11))
 }
 

tests/baselines/reference/unusedLocalsAndObjectSpread2.types

@@ -10,7 +10,7 @@ const {
 >active : any
 >_a : any
 
-  ...rest,
+  ...rest
 >rest : any
 
 } = props;
@@ -27,10 +27,10 @@ function foo() {
 >active : any
 >_a : any
 
-    ...rest,
+        ...rest
 >rest : any
 
-  } = props;
+    } = props;
 >props : any
 }
 

tests/cases/compiler/unusedLocalsAndObjectSpread2.ts

@@ -4,15 +4,15 @@ declare let props: any;
 const {
     children, // here!
     active: _a, // here!
-  ...rest,
+  ...rest
 } = props;
 
 function foo() {
     const {
         children,
         active: _a,
-    ...rest,
-  } = props;
+        ...rest
+    } = props;
 }
 
 export const asdf = 123;

tests/cases/conformance/es7/trailingCommasInBindingPatterns.ts

@@ -0,0 +1,5 @@
+const [...a,] = [];
+const {...b,} = {};
+let c, d;
+([...c,] = []);
+({...d,} = {});