Treat ... in jsdoc type as creating a synthetic rest parameter -- not as an array type (#19483)

Andy · web-flow · commit 4b96edf72faf · 2017-11-15T13:04:08.000-08:00
* Treat `...` in jsdoc type as creating a synthetic rest parameter -- not as an array type

* Change type parsing so `...T[]` parses as `...(T[])` and not `(...T)[]`

* Replace the last parameter with ...args, and make access to it potentially undefined

* Code review
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -3547,6 +3547,10 @@
         "category": "Error",
         "code": 8027
     },
+    "JSDoc '...' may only appear in the last parameter of a signature.": {
+        "category": "Error",
+        "code": 8028
+    },
     "Only identifiers/qualified-names with optional type arguments are currently supported in a class 'extends' clause.": {
         "category": "Error",
         "code": 9002
diff --git a/src/compiler/parser.ts b/src/compiler/parser.ts
@@ -2678,8 +2678,6 @@ namespace ts {
                     return parseJSDocUnknownOrNullableType();
                 case SyntaxKind.FunctionKeyword:
                     return parseJSDocFunctionType();
-                case SyntaxKind.DotDotDotToken:
-                    return parseJSDocNodeWithType(SyntaxKind.JSDocVariadicType);
                 case SyntaxKind.ExclamationToken:
                     return parseJSDocNodeWithType(SyntaxKind.JSDocNonNullableType);
                 case SyntaxKind.NoSubstitutionTemplateLiteral:
@@ -2819,6 +2817,12 @@ namespace ts {
             switch (token()) {
                 case SyntaxKind.KeyOfKeyword:
                     return parseTypeOperator(SyntaxKind.KeyOfKeyword);
+                case SyntaxKind.DotDotDotToken: {
+                    const result = createNode(SyntaxKind.JSDocVariadicType) as JSDocVariadicType;
+                    nextToken();
+                    result.type = parsePostfixTypeOrHigher();
+                    return finishNode(result);
+                }
             }
             return parsePostfixTypeOrHigher();
         }
diff --git a/src/compiler/utilities.ts b/src/compiler/utilities.ts
@@ -1626,16 +1626,22 @@ namespace ts {
             return undefined;
         }
         const name = node.name.escapedText;
+        const decl = getHostSignatureFromJSDoc(node);
+        if (!decl) {
+            return undefined;
+        }
+        const parameter = find(decl.parameters, p => p.name.kind === SyntaxKind.Identifier && p.name.escapedText === name);
+        return parameter && parameter.symbol;
+    }
+
+    export function getHostSignatureFromJSDoc(node: JSDocParameterTag): FunctionLike | undefined {
         const host = getJSDocHost(node);
         const decl = getSourceOfAssignment(host) ||
             getSingleInitializerOfVariableStatement(host) ||
             getSingleVariableOfVariableStatement(host) ||
             getNestedModuleDeclaration(host) ||
             host;
-        if (decl && isFunctionLike(decl)) {
-            const parameter = find(decl.parameters, p => p.name.kind === SyntaxKind.Identifier && p.name.escapedText === name);
-            return parameter && parameter.symbol;
-        }
+        return decl && isFunctionLike(decl) ? decl : undefined;
     }
 
     export function getJSDocHost(node: JSDocTag): HasJSDoc {
diff --git a/tests/baselines/reference/jsFileCompilationRestParamJsDocFunction.js b/tests/baselines/reference/jsFileCompilationRestParamJsDocFunction.js
@@ -9,7 +9,7 @@
  * @param {...*} args The arguments to invoke `func` with.
  * @returns {*} Returns the result of `func`.
  */
-function apply(func, thisArg, args) {
+function apply(func, thisArg, ...args) {
     var length = args.length;
     switch (length) {
         case 0: return func.call(thisArg);
@@ -36,7 +36,11 @@ define("_apply", ["require", "exports"], function (require, exports) {
      * @param {...*} args The arguments to invoke `func` with.
      * @returns {*} Returns the result of `func`.
      */
-    function apply(func, thisArg, args) {
+    function apply(func, thisArg) {
+        var args = [];
+        for (var _i = 2; _i < arguments.length; _i++) {
+            args[_i - 2] = arguments[_i];
+        }
         var length = args.length;
         switch (length) {
             case 0: return func.call(thisArg);
diff --git a/tests/baselines/reference/jsFileCompilationRestParamJsDocFunction.symbols b/tests/baselines/reference/jsFileCompilationRestParamJsDocFunction.symbols
@@ -9,7 +9,7 @@
  * @param {...*} args The arguments to invoke `func` with.
  * @returns {*} Returns the result of `func`.
  */
-function apply(func, thisArg, args) {
+function apply(func, thisArg, ...args) {
 >apply : Symbol(apply, Decl(_apply.js, 0, 0))
 >func : Symbol(func, Decl(_apply.js, 10, 15))
 >thisArg : Symbol(thisArg, Decl(_apply.js, 10, 20))
diff --git a/tests/baselines/reference/jsFileCompilationRestParamJsDocFunction.types b/tests/baselines/reference/jsFileCompilationRestParamJsDocFunction.types
@@ -9,8 +9,8 @@
  * @param {...*} args The arguments to invoke `func` with.
  * @returns {*} Returns the result of `func`.
  */
-function apply(func, thisArg, args) {
->apply : (func: Function, thisArg: any, args: any[]) => any
+function apply(func, thisArg, ...args) {
+>apply : (func: Function, thisArg: any, ...args: any[]) => any
 >func : Function
 >thisArg : any
 >args : any[]
@@ -84,5 +84,5 @@ function apply(func, thisArg, args) {
 }
 
 export default apply;
->apply : (func: Function, thisArg: any, args: any[]) => any
+>apply : (func: Function, thisArg: any, ...args: any[]) => any
 
diff --git a/tests/baselines/reference/jsdocDisallowedInTypescript.errors.txt b/tests/baselines/reference/jsdocDisallowedInTypescript.errors.txt
@@ -7,7 +7,10 @@ tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(11,12): error TS255
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(13,14): error TS8020: JSDoc types can only be used inside documentation comments.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(14,11): error TS8020: JSDoc types can only be used inside documentation comments.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(15,8): error TS8020: JSDoc types can only be used inside documentation comments.
+tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(16,5): error TS2322: Type 'boolean[]' is not assignable to type 'boolean | undefined'.
+  Type 'boolean[]' is not assignable to type 'false'.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(16,15): error TS8020: JSDoc types can only be used inside documentation comments.
+tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(16,15): error TS8028: JSDoc '...' may only appear in the last parameter of a signature.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(17,11): error TS8020: JSDoc types can only be used inside documentation comments.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(18,17): error TS8020: JSDoc types can only be used inside documentation comments.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(19,5): error TS2322: Type 'undefined' is not assignable to type 'number | null'.
@@ -16,9 +19,10 @@ tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(21,16): error TS802
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(22,16): error TS8020: JSDoc types can only be used inside documentation comments.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(23,17): error TS8020: JSDoc types can only be used inside documentation comments.
 tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(24,17): error TS8020: JSDoc types can only be used inside documentation comments.
+tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(24,17): error TS8028: JSDoc '...' may only appear in the last parameter of a signature.
 
 
-==== tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts (18 errors) ====
+==== tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts (21 errors) ====
     // grammar error from checker
     var ara: Array.<number> = [1,2,3];
                   ~
@@ -53,8 +57,13 @@ tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(24,17): error TS802
            ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 !!! error TS8020: JSDoc types can only be used inside documentation comments.
     var variadic: ...boolean = [true, false, true];
+        ~~~~~~~~
+!!! error TS2322: Type 'boolean[]' is not assignable to type 'boolean | undefined'.
+!!! error TS2322:   Type 'boolean[]' is not assignable to type 'false'.
                   ~~~~~~~~~~
 !!! error TS8020: JSDoc types can only be used inside documentation comments.
+                  ~~~~~~~~~~
+!!! error TS8028: JSDoc '...' may only appear in the last parameter of a signature.
     var most: !string = 'definite';
               ~~~~~~~
 !!! error TS8020: JSDoc types can only be used inside documentation comments.
@@ -79,5 +88,7 @@ tests/cases/conformance/jsdoc/jsdocDisallowedInTypescript.ts(24,17): error TS802
     var vars: Array<...number>;
                     ~~~~~~~~~
 !!! error TS8020: JSDoc types can only be used inside documentation comments.
+                    ~~~~~~~~~
+!!! error TS8028: JSDoc '...' may only appear in the last parameter of a signature.
     
     
diff --git a/tests/baselines/reference/jsdocDisallowedInTypescript.types b/tests/baselines/reference/jsdocDisallowedInTypescript.types
@@ -65,7 +65,7 @@ var g: function(number, number): number = (n,m) => n + m;
 >m : number
 
 var variadic: ...boolean = [true, false, true];
->variadic : boolean[]
+>variadic : boolean | undefined
 >[true, false, true] : boolean[]
 >true : true
 >false : false
@@ -96,7 +96,7 @@ var anys: Array<*>;
 >Array : T[]
 
 var vars: Array<...number>;
->vars : number[][]
+>vars : (number | undefined)[]
 >Array : T[]
 
 
diff --git a/tests/baselines/reference/jsdocPrefixPostfixParsing.errors.txt b/tests/baselines/reference/jsdocPrefixPostfixParsing.errors.txt
@@ -0,0 +1,43 @@
+tests/cases/conformance/jsdoc/prefixPostfix.js(8,13): error TS8028: JSDoc '...' may only appear in the last parameter of a signature.
+tests/cases/conformance/jsdoc/prefixPostfix.js(9,12): error TS1014: A rest parameter must be last in a parameter list.
+tests/cases/conformance/jsdoc/prefixPostfix.js(10,12): error TS1014: A rest parameter must be last in a parameter list.
+tests/cases/conformance/jsdoc/prefixPostfix.js(11,12): error TS1014: A rest parameter must be last in a parameter list.
+tests/cases/conformance/jsdoc/prefixPostfix.js(12,12): error TS1014: A rest parameter must be last in a parameter list.
+tests/cases/conformance/jsdoc/prefixPostfix.js(13,12): error TS1014: A rest parameter must be last in a parameter list.
+tests/cases/conformance/jsdoc/prefixPostfix.js(14,12): error TS1014: A rest parameter must be last in a parameter list.
+
+
+==== tests/cases/conformance/jsdoc/prefixPostfix.js (7 errors) ====
+    /**
+     * @param {number![]} x - number[]
+     * @param {!number[]} y - number[]
+     * @param {(number[])!} z - number[]
+     * @param {number?[]} a - (number | null)[]
+     * @param {?number[]} b - number[] | null
+     * @param {(number[])?} c - number[] | null
+     * @param {?...number} d - number[] | null
+                ~~~~~~~~~
+!!! error TS8028: JSDoc '...' may only appear in the last parameter of a signature.
+     * @param {...?number} e - (number | null)[]
+               ~~~~~~~~~~
+!!! error TS1014: A rest parameter must be last in a parameter list.
+     * @param {...number?} f - number[] | null
+               ~~~~~~~~~~
+!!! error TS1014: A rest parameter must be last in a parameter list.
+     * @param {...number!?} g - number[] | null
+               ~~~~~~~~~~~
+!!! error TS1014: A rest parameter must be last in a parameter list.
+     * @param {...number?!} h - number[] | null
+               ~~~~~~~~~~~
+!!! error TS1014: A rest parameter must be last in a parameter list.
+     * @param {...number[]} i - number[][]
+               ~~~~~~~~~~~
+!!! error TS1014: A rest parameter must be last in a parameter list.
+     * @param {...number![]?} j - number[][] | null
+               ~~~~~~~~~~~~~
+!!! error TS1014: A rest parameter must be last in a parameter list.
+     * @param {...number?[]!} k - (number[] | null)[]
+     */
+    function f(x, y, z, a, b, c, d, e, f, g, h, i, j, k) {
+    }
+    
diff --git a/tests/baselines/reference/jsdocPrefixPostfixParsing.types b/tests/baselines/reference/jsdocPrefixPostfixParsing.types
@@ -16,20 +16,20 @@
  * @param {...number?[]!} k - (number[] | null)[]
  */
 function f(x, y, z, a, b, c, d, e, f, g, h, i, j, k) {
->f : (x: number[], y: number[], z: number[], a: (number | null)[], b: number[] | null, c: number[] | null, d: number[] | null, e: (number | null)[], f: number[] | null, g: number[] | null, h: number[] | null, i: number[][], j: number[][] | null, k: (number[] | null)[]) => void
+>f : (x: number[], y: number[], z: number[], a: (number | null)[], b: number[] | null, c: number[] | null, d: number | null | undefined, e: number | null | undefined, f: number | null | undefined, g: number | null | undefined, h: number | null | undefined, i: number[] | undefined, j: number[] | null | undefined, ...args: (number | null)[][]) => void
 >x : number[]
 >y : number[]
 >z : number[]
 >a : (number | null)[]
 >b : number[] | null
 >c : number[] | null
->d : number[] | null
->e : (number | null)[]
->f : number[] | null
->g : number[] | null
->h : number[] | null
->i : number[][]
->j : number[][] | null
->k : (number[] | null)[]
+>d : number | null | undefined
+>e : number | null | undefined
+>f : number | null | undefined
+>g : number | null | undefined
+>h : number | null | undefined
+>i : number[] | undefined
+>j : number[] | null | undefined
+>k : (number | null)[] | undefined
 }
 
diff --git a/tests/baselines/reference/jsdocRestParameter.errors.txt b/tests/baselines/reference/jsdocRestParameter.errors.txt
@@ -0,0 +1,19 @@
+/a.js(7,3): error TS2345: Argument of type 'number[]' is not assignable to parameter of type 'number'.
+/a.js(8,6): error TS2345: Argument of type '"2"' is not assignable to parameter of type 'number'.
+
+
+==== /a.js (2 errors) ====
+    /** @param {...number} a */
+    function f(a) {
+        a; // number | undefined
+        // Ideally this would be a number. But currently checker.ts has only one `argumentsSymbol`, so it's `any`.
+        arguments[0];
+    }
+    f([1, 2]); // Error
+      ~~~~~~
+!!! error TS2345: Argument of type 'number[]' is not assignable to parameter of type 'number'.
+    f(1, "2"); // Error
+         ~~~
+!!! error TS2345: Argument of type '"2"' is not assignable to parameter of type 'number'.
+    f(1, 2);
+    
diff --git a/tests/baselines/reference/jsdocRestParameter.symbols b/tests/baselines/reference/jsdocRestParameter.symbols
@@ -0,0 +1,22 @@
+=== /a.js ===
+/** @param {...number} a */
+function f(a) {
+>f : Symbol(f, Decl(a.js, 0, 0))
+>a : Symbol(a, Decl(a.js, 1, 11))
+
+    a; // number | undefined
+>a : Symbol(a, Decl(a.js, 1, 11))
+
+    // Ideally this would be a number. But currently checker.ts has only one `argumentsSymbol`, so it's `any`.
+    arguments[0];
+>arguments : Symbol(arguments)
+}
+f([1, 2]); // Error
+>f : Symbol(f, Decl(a.js, 0, 0))
+
+f(1, "2"); // Error
+>f : Symbol(f, Decl(a.js, 0, 0))
+
+f(1, 2);
+>f : Symbol(f, Decl(a.js, 0, 0))
+
diff --git a/tests/baselines/reference/jsdocRestParameter.types b/tests/baselines/reference/jsdocRestParameter.types
@@ -0,0 +1,34 @@
+=== /a.js ===
+/** @param {...number} a */
+function f(a) {
+>f : (...args: number[]) => void
+>a : number | undefined
+
+    a; // number | undefined
+>a : number | undefined
+
+    // Ideally this would be a number. But currently checker.ts has only one `argumentsSymbol`, so it's `any`.
+    arguments[0];
+>arguments[0] : any
+>arguments : IArguments
+>0 : 0
+}
+f([1, 2]); // Error
+>f([1, 2]) : void
+>f : (...args: number[]) => void
+>[1, 2] : number[]
+>1 : 1
+>2 : 2
+
+f(1, "2"); // Error
+>f(1, "2") : void
+>f : (...args: number[]) => void
+>1 : 1
+>"2" : "2"
+
+f(1, 2);
+>f(1, 2) : void
+>f : (...args: number[]) => void
+>1 : 1
+>2 : 2
+
diff --git a/tests/baselines/reference/jsdocRestParameter_es6.symbols b/tests/baselines/reference/jsdocRestParameter_es6.symbols
@@ -0,0 +1,10 @@
+=== /a.js ===
+/** @param {...number} a */
+function f(...a) {
+>f : Symbol(f, Decl(a.js, 0, 0))
+>a : Symbol(a, Decl(a.js, 1, 11))
+
+    a; // number[]
+>a : Symbol(a, Decl(a.js, 1, 11))
+}
+
diff --git a/tests/baselines/reference/jsdocRestParameter_es6.types b/tests/baselines/reference/jsdocRestParameter_es6.types
@@ -0,0 +1,10 @@
+=== /a.js ===
+/** @param {...number} a */
+function f(...a) {
+>f : (...a: number[]) => void
+>a : number[]
+
+    a; // number[]
+>a : number[]
+}
+
diff --git a/tests/cases/compiler/jsFileCompilationRestParamJsDocFunction.ts b/tests/cases/compiler/jsFileCompilationRestParamJsDocFunction.ts
@@ -13,7 +13,7 @@
  * @param {...*} args The arguments to invoke `func` with.
  * @returns {*} Returns the result of `func`.
  */
-function apply(func, thisArg, args) {
+function apply(func, thisArg, ...args) {
     var length = args.length;
     switch (length) {
         case 0: return func.call(thisArg);
diff --git a/tests/cases/compiler/jsdocRestParameter.ts b/tests/cases/compiler/jsdocRestParameter.ts
@@ -0,0 +1,15 @@
+// @allowJs: true
+// @checkJs: true
+// @strict: true
+// @noEmit: true
+
+// @Filename: /a.js
+/** @param {...number} a */
+function f(a) {
+    a; // number | undefined
+    // Ideally this would be a number. But currently checker.ts has only one `argumentsSymbol`, so it's `any`.
+    arguments[0];
+}
+f([1, 2]); // Error
+f(1, "2"); // Error
+f(1, 2);
diff --git a/tests/cases/compiler/jsdocRestParameter_es6.ts b/tests/cases/compiler/jsdocRestParameter_es6.ts
@@ -0,0 +1,10 @@
+// @allowJs: true
+// @checkJs: true
+// @strict: true
+// @noEmit: true
+
+// @Filename: /a.js
+/** @param {...number} a */
+function f(...a) {
+    a; // number[]
+}
diff --git a/tests/cases/fourslash/codeFixChangeJSDocSyntax27.ts b/tests/cases/fourslash/codeFixChangeJSDocSyntax27.ts
diff --git a/tests/cases/fourslash/codeFixChangeJSDocSyntax3.ts b/tests/cases/fourslash/codeFixChangeJSDocSyntax3.ts
diff --git a/tests/cases/fourslash/getJavaScriptCompletions6.ts b/tests/cases/fourslash/getJavaScriptCompletions6.ts
diff --git a/tests/cases/fourslash/getJavaScriptCompletions7.ts b/tests/cases/fourslash/getJavaScriptCompletions7.ts
