In JSDoc, `?` of conditional is frequently parsed as postfix-`?`

[ntninja]

TypeScript Version: 3.2.0-dev.20180927
Search Terms: JSDoc conditional types

Using Conditional Types in JSDoc comments confuses the TypeScript parser since the T extends Y ? A : B syntax looks similar to the Y? (meaning Y|null) syntax from JSDoc.

Code

/**
 * @template {{}} T
 * @param {T} o
 * @returns {T extends Readonly<infer U> ? (keyof U)[] : string[]}
 */
function Object_keys(o) {
	let keys = Object.keys(o);
	// @ts-ignore: Type assertion for stripping `Readonly<…>`
	return keys;
}

Expected behavior:

No error should be reported.
The type of Object_keys should be: <T extends {}>(o: T): T extends Readonly<infer U> ? (keyof U)[] : string[].

Actual behavior:

TypeScript compiler shows an error ("?" expected) and mistakes the existing ? operator for JSDoc <type>|null syntax.

The actual type therefor ends up being: <T extends {}>(o: T): T extends Readonly<infer U> | null ? (keyof U)[] : string[].

Possible fixes:

1. When encountering a top-level (not within parenthesis) ? token in an extends clause, scan ahead and check whether it is followed by an | or & token (type intersection or union operators). If yes, treat it as |null and keep processing; otherwise, assume it's the start of a conditional type declaration. (Requires at least an LF(2) parser.)
2. Prohibit the JSDoc ? operator in the top-level of an extends clause, always causing it to be treated as the start of the conditional type declaration.

Playground Link: None, Playground does not seem to support JavaScript with JSDoc instead of TypeScript as input.

[sandersn]

3. Forbid conditional types in JSDoc. Postfix-? support is more important.

(2) is probably the best option if the implementation isn't too messy.

[noinkling]

I also just ran into this bug (v3.4.3), was very confused about how the null got in there until I found this issue.

[sandersn]

After discussion with the JSDoc people, who talked to Closure people, here's option (4):

4. Forbid postfix-? support. It is rarely used and was only included in Closure as legacy syntax.

[brendankenny]

👍 for (4) Forbid postfix-? support. It's confusing having both pre and post ? variants anyway, especially because in the postfix position it starts looking a lot like a ts optional property.

[goodmind]

Found same problem when reprinting AST from jsdoc

[ExE-Boss]

I’ve found that moving the ? on a new line makes TypeScript parse it correctly as a conditional type:

/**
 * @template {{}} T
 * @param {T} o
 * @returns {T extends Readonly<infer U>
	? (keyof U)[] : string[]} */
function Object_keys(o) {
	return /** @type {any} */ (Object.keys(o));
}

[sandersn]

#39123 does not remove postfix-?, but at least it parses ? as a conditional type if possible. @ExE-Boss how does your example behave on a recent build of typescript (4.0 beta or nightly)?

[ExE-Boss]

It works correctly both with and without the new line.

[sandersn]

That may be as close as we get to closing this bug then, because tuples now use postfix-? to indicate optional elements: [number,string?]. That means we can't get rid of postfix-? parsing entirely.

[ExE-Boss]

Well, [number, string?] is valid in TypeScript as well.

And the string? bit becomes (string | undefined)? instead of string | null, so it’s not the null union postfix‑?, but is instead the optional tuple element postfix‑?.

I would however expect that [?string?] would become [(string | null | undefined)?] instead of [string | null] as it currently does in TypeScript 4.0‑beta.

---

Note that [number, string?] in JSDoc incorrectly produces [number, string | null] in TypeScript <= 3.9.

[sandersn]

I would like to get rid of jsdoc-style postfix-? semantics entirely, including the parsing. It's an old syntax copied from Closure that Closure included for backward compatibility. Its remnants are going to cause minor errors like [?string?] as long as they're around.

I guess we could still get rid of the jsdoc interpretation of string? as string | null, if we made sure it wasn't too big of a breaking change.