Parens on union type fail for function arguments (JSDoc style)

[tschaub]

TypeScript Version: [email protected]

Search Terms: function arguments grouped union type

Code

I notice that if a union type is grouped with ( and ) it works as a return type but not as an argument type. For example, this does not work:

/**
 * @typedef {function((string|undefined)): (string|undefined)} SomeType
 */

While this does work:

/**
 * @typedef {function(string|undefined): (string|undefined)} SomeType
 */

Expected behavior:

With this tsconfig.json:

{
  "compilerOptions": {
    "target": "ES2017",
    "allowJs": true,
    "checkJs": true,
    "noEmit": true,
    "strict": false,
  },
  "include": [
    "src/**/*.js"
  ]
}

I expected tsc to succeed.

Actual behavior:

src/main.js:2:23 - error TS1138: Parameter declaration expected.

2  * @typedef {function((string|undefined)): (string|undefined)} SomeType
                        ~

src/main.js:2:41 - error TS1005: '}' expected.

2  * @typedef {function((string|undefined)): (string|undefined)} SomeType
                                          ~

Perhaps this is a known limitation. I can remove the parens. But I thought it might also be a parsing bug.

[mhegazy]

@sandersn would be nice to get this in to unblock the openlayers/openlayers PR

[tschaub]

It looks like removing the parens from the union is not an easy alternative unless we remove our ESLint valid-jsdoc rule.

That rule considers this a valid JSDoc comment:

  /**
   * @param {function((string|boolean)): boolean} bar A function.
   * @returns {boolean} True.
   */
  foo(bar) {
    return true;
  }

And this is an error:

  /**
   * @param {function(string|boolean): boolean} bar A function.
   * @returns {boolean} True.
   */
  foo(bar) {
    return true;
  }

The lack of a JSDoc spec makes it tough to say which one is "correct" - but the Closure Compiler docs have a sensible suggestion for type unions:

The parentheses may be omitted at the top-level expression, but the parentheses should be included in sub-expressions to avoid ambiguity.

I'll dig around to see if I can find where a fix might be applied. Any pointers are appreciated.

[sandersn]

Looks like you don't even need a union type to repro this:

/** @type {function((string)): string} */
var x = s => s.toString()

I'm looking at this. It might be easy to fix, as long as the parser isn't getting confused when trying to speculatively parse an arrow function type.

[sandersn]

Ah, the problem is that the parameters to a closure-style function() type are parsed just like normal parameters, and parameters don't normally start with a parenthesis:

function x(a, (b), c) { } // parse error at '(b)'

We have exceptions in the parser when parsing parameters of a jsdoc function type, but they don't apply at this parameter-start location. I think I can fix it by explicitly special-casing the parameter list of jsdoc functions.

[sandersn]

Fix is up at #25799

[mhegazy]

@tschaub the fix should be in tomorrow's typescript@next.

[tschaub]

Awesome. Thanks @sandersn and @mhegazy. I'll test it out tomorrow.

As an aside, I'm hoping to get ESLint's valid-jsdoc rule working together with TypeScript's JSDoc support. Currently, the valid-jsdoc rule treat's TypeScript's inline imports as invalid types. I've proposed eslint/doctrine#216 to fix this. If anybody knows an ESLint contributor or wants to weigh in there, I'd appreciate it. (TypeScript's support for importing types feels like a pretty critical feature.)