JSDoc ...rest parameter syntax cannot support tuple types

[thw0rted]

Bug Report

🔎 Search Terms

jsdoc rest tuple

🕗 Version & Regression Information

• This is the behavior in every version I tried, and I reviewed the FAQ for entries about JSDoc

⏯ Playground Link

Playground link with relevant code

💻 Code

/**
 * @callback CBFunction
 * @param {number} n
 * @param {string} s
 */

// Desired outcome:
// typedef CBCaller = (...args: Parameters<CBFunction>) => void

/**
 * @param {...Parameters<CBFunction>} args
 */
function f(...args) {
  console.log(args);
}

🙁 Actual behavior

See Playground link: on .d.ts tab, the output is declare function f(...args: [number, string][]): void;, an array-of-tuples. Generally, JSDoc syntax treats {...X} args as args: X[].

🙂 Expected behavior

TS-flavored JSDoc should be able to express any type that TS can express.

[thw0rted]

Might be worth mentioning, this is kind of the opposite problem from #43072.

[sandersn]

TS' jsdoc doesn't currently have a good way to match any rest parameter to a @param tag. The code you wrote is unfortunately already used by closure type syntax in jsdoc, so this

/**
 * @param {...Parameters<CBFunction>} args
 */
function f(...args)

Is like writing this:

function f(...args: Parameters<CBFunction>[])

even if it weren't used, everything inside {} is supposed to be a type.

We need an alternate proposal. One possibility is to add the ... on the parameter name:

/**
 * @param {Parameters<CBFunction>} ...args
 */
function f(...args)

This might conflict with existing jsdoc, but I don't think it does off the top of my head. And I think it would be possible to enforce the usual rules on ...: eg that it can only occur once, at the end of a parameter list.

[thw0rted]

That sounds like it could work. What's the process for moving this forward? As far as I can tell, there is no standards body for JSDoc, and the core JSDoc project here on GitHub hasn't pushed a meaningful update since 2017. Do you just kind of discuss the change internally, then go for it? Do you talk to other "flavors" of JSDoc (like Closure) to try to avoid conflicts or incompatibilities?

[sandersn]

The next step is to add a proposal to this issue and maybe a prototype to see what kind of implementation issues arise.

The other jsdoc processors don't change much and mostly have a different focus. For adding a new parameter syntax, I'd check to see whether it conflicts with the jsdoc tool or closure, and perhaps tsdoc too—or whether any of those already have an alternative.

[thw0rted]

OK, I'm up for some of that legwork.

JSDoc

Already checked, no alternative. I can't find an online compiler to test the proposed syntax, but I ran this through the 3.6.9 CLI and got no warnings or errors:

/**
 * @param {string} a
 * @param {boolean} b
 */
export function g(a, b){
  console.log(a,b);
}

/**
 * @param {Parameters.<g>} ...args - all params
 */
export function f(...args){
    console.log(args);
}

The doc output just treats Parameters as an opaque token, I guess? It identified ...args as the param name, which is what we wanted.

TSDoc

They do not have a rest-param syntax and nobody has mentioned it in their issues. I found this test site and tried

/**
 * @param {[number, string]} ...args - all params
 */

but I guess TSDoc doesn't support redundant JSDoc types anyway, because it dumped out

(2,11): The @param block should not include a JSDoc-style '{type}'
(2,4): The @param block should be followed by a valid parameter name: The identifier cannot non-word characters
(2,11): Expecting a TSDoc tag starting with "{@"
(2,28): The "}" character should be escaped using a backslash to avoid confusion with a TSDoc inline tag

Closure

Looks like it still doesn't support tuples at all which I think would be a prerequisite. That said, if this online compiler I found is up to date, Closure would error on your proposed syntax (@param {Foo} ...args):

JSC_TYPE_PARSE_ERROR: Bad type annotation. expecting a variable name in a @param tag. See https://github.com/google/closure-compiler/wiki/Annotating-JavaScript-for-the-Closure-Compiler for more information. at line 2 character 19
 * @param {number} ...args
                   ^

---

TL;DR: I don't think {SomeType} ...args would conflict with another meaning, but it would be considered invalid by at least some other parsers. Is breaking TSDoc or Closure a problem? Do we need a feature request with them before moving forward here? (It's a good thing it doesn't break vanilla JSDoc, at least...)

[sandersn]

An error is fine -- I want to avoid re-using existing syntax because in Closure's case it makes using TS for the compiler frontend and Closure for the compiler backend (via tsickle) needlessly complex. And in TSDoc's case it would force them to somehow disambiguate between TSDoc syntax and TS syntax.

I would recommend looking at the parameter-matching code in the checker and coming up with some tricky examples that show that ...args works in all cases and doesn't have any surprises in the semantics. After that I'd be happy to put it on the backlog or seeing whether @RyanCavanaugh wants to have somebody from the team work on it.

Some things to think about:

• nested object syntax .. doesn't make much sense but somebody will probably want to write it. What does it mean here? Anything?

/** 
 * Does this make any sense? Should it be allowed at all?
 * People might want to document individual tuple properties!
 * @param {Object} ...args
 * @param {number} ...args.0 - should the ... be required/allowed here?
 * @param {string} ...args.1 - how do you even write the name for tuple properties?
 */

• enforcing order. I think the checker will do this by default if it treats the tag as a normal rest parameter. Maaaaybe any order should be allowed, but I think being strict is a good idea for new syntax.
• people are 100% going to write @param {...string} ...args. That's probably fine, but we want to give a nice error when they write @param {...string} args by mistake. Actually this is an implementation concern.
• We need to avoid breaking code that relies on the existing positional rest parameter matching. I don't remember the rules for that, just that it doesn't work very well.

[thw0rted]

I'm going to need some time to process most of that but wanted to point out that @param {...string} args is currently legal JSDoc, meaning f(...args: string[]). Did you mean "when they write @param {...string} ...args by mistake"? (I do agree, I'm pretty sure having spread in both places wouldn't make sense. Uh, right?)

[sandersn]

Well, now I'm not even sure this feature is required. I thought that @param {string[]} x wouldn't match a rest parameter ...x, but it does. In fact, tuples work fine with it:

Playground link

// @ts-check
/**
 * @callback CBFunction
 * @param {number} n
 * @param {string} s
 */
/**
 * @param {Parameters<CBFunction>} x
 */
function f(...x) {

}
f(12,'b')

The only difference from the original example in the issue is that the param type is Parameters<CBFunction> instead of ...Parameters<CBFunction>.

[thw0rted]

Full disclosure: I filed this issue because a library I use frequently builds their type defs from JSDoc, using tsd-jsdoc. I contributed some updates, one of which was adding generic params to a class definition, so that I could strongly type the parameters passed to

/**
  * @param {any} arguments - arguments to pass to a callback
  */
Foo.prototype.raiseEvent = function() {
  // ...
  this.callback.apply(this.scope, arguments);
}

If I were writing the code myself, I'd use a named rest-param, rather than arguments, but it's not up to me. The project currently disallows the use of rest parameters. Maybe I just need to talk them out of that rule? Will investigate.

[thw0rted]

Well, good news and bad news. It turns out the library in question updated their eslint config a month or two ago and I didn't notice, and rest-params are now allowed. So the good news is, I can make the change. The bad news is, tsd-jsdoc incorrectly translates

/**
  * @param {Parameters<CBFunction>} args- arguments to pass to a callback
  */
Foo.prototype.raiseEvent = function(...args) {
  // ...
  this.callback.apply(this.scope, args);
}

into raiseEvent(args: Parameters<CBFunction>): void. I guess maybe it only looks at the JSDoc, not the declared function params in the JS code?

The question then becomes, is the official position of the team is that @param tags for rest-params should work differently than the same tag for a regular parameter (i.e., current TS parser behavior)? If so, I will file a bug with tsd-jsdoc to work the same way the TS compiler does now. It's incompatible with vanilla JSDoc, though, so I think your earlier proposal (@param {TupleType} ...args) might make sense to implement instead.

[sandersn]

Wait, won't @param {TupleType} ...args also be incompatible with existing vanilla JSDoc? It's a new feature as far as I know, so it'd only help in the future, and only if the jsdoc tool implemented support for it.

I'd vote to change tsd-jsdoc to work they way TS does.

[thw0rted]

I ran the JSDoc CLI against @param {TupleType} ...args and it parses ...args as the variable name. I couldn't find a "playground" to demonstrate this online, I just updated the library I'm working on to use that syntax and their existing template was populated with ...args in the Name column of the relevant parameter table. I'm not saying there aren't other parsers out there that will break, but at least the JSDoc 3.6.9 CLI handles it fine.

ETA: as for tsd-jsdoc working like TS does, that could be a really big change since it would mean parsing the code as well as the JSDoc tags. Right?

[sandersn]

I don't know anything about tsd-jsdoc, but wouldn't it have to? Doesn't tsd-jsdoc use the Typescript compiler to emit types? Otherwise how does it emit the correct types for all the constructs supported in JS?

/** 
 * @template T 
 * @param {T} t
 */
function C(t) { this.t = t }
C.prototype.m = function () { return this.t }
// should generate
class C<T> {
  m(): T
}

or

const o = {
  a: 1
}
o.C = class {
}
// should generate
declare const o: { a: number }
declare namespace o {
  export class C { }
}

A lot of JS types don't have associated JSDoc because they can be inferred from values.

[thw0rted]

tsd-jsdoc does not currently use the TS compiler, though they have been thinking about it for a while. I believe the answer to your question is, they only support a subset of all the JSDoc constructs that the TS compiler does.