Skip to content

Commit

Permalink
In JSDoc, parse postfix-? below conditional types/tuple types (#39123)
Browse files Browse the repository at this point in the history
Outside of JSDoc comments, postfix-? is parsed at lower precedence than
the `?` of conditional types, and a postfix-? inside a tuple type
results in the type being marked optional.

This PR changes JSDoc parsing to behave the same way, which means that

1. Conditional types are allowed in JSDoc. Fixes #37166.
2. Tuple types' postfix-? syntax is interpreted correctly in JSDoc.
Fixes #38747.

The breaking change is that a postfix-? type followed by another postfix type,
like `[]` or `!`, is parsed as a conditional type. [Postfix-? is not
common](#37166 (comment)),
so this is an acceptable breaking change.

A postfix-? type `T?` is still parsed everywhere else and treated as `T | null`.
  • Loading branch information
sandersn authored Jun 17, 2020
1 parent d7aa5f3 commit c3c6be6
Show file tree
Hide file tree
Showing 5 changed files with 75 additions and 39 deletions.
4 changes: 2 additions & 2 deletions src/compiler/parser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3115,7 +3115,7 @@ namespace ts {
return finishNode(factory.createRestTypeNode(parseType()), pos);
}
const type = parseType();
if (!(contextFlags & NodeFlags.JSDoc) && isJSDocNullableType(type) && type.pos === type.type.pos) {
if (isJSDocNullableType(type) && type.pos === type.type.pos) {
const node = factory.createOptionalTypeNode(type.type);
setTextRange(node, type);
(node as Mutable<Node>).flags = type.flags;
Expand Down Expand Up @@ -3356,7 +3356,7 @@ namespace ts {
break;
case SyntaxKind.QuestionToken:
// If not in JSDoc and next token is start of a type we have a conditional type
if (!(contextFlags & NodeFlags.JSDoc) && lookAhead(nextTokenIsStartOfType)) {
if (lookAhead(nextTokenIsStartOfType)) {
return type;
}
nextToken();
Expand Down
42 changes: 34 additions & 8 deletions tests/baselines/reference/jsdocPrefixPostfixParsing.errors.txt
Original file line number Diff line number Diff line change
@@ -1,17 +1,29 @@
tests/cases/conformance/jsdoc/prefixPostfix.js(5,18): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
tests/cases/conformance/jsdoc/prefixPostfix.js(5,18): error TS1005: '}' expected.
tests/cases/conformance/jsdoc/prefixPostfix.js(8,12): error TS1014: A rest parameter must be last in a parameter list.
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(11,21): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
tests/cases/conformance/jsdoc/prefixPostfix.js(11,21): error TS1005: '}' expected.
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,21): error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
tests/cases/conformance/jsdoc/prefixPostfix.js(14,21): error TS1005: '}' expected.
tests/cases/conformance/jsdoc/prefixPostfix.js(18,21): error TS7006: Parameter 'a' implicitly has an 'any' type.
tests/cases/conformance/jsdoc/prefixPostfix.js(18,39): error TS7006: Parameter 'h' implicitly has an 'any' type.
tests/cases/conformance/jsdoc/prefixPostfix.js(18,48): error TS7006: Parameter 'k' implicitly has an 'any' type.


==== tests/cases/conformance/jsdoc/prefixPostfix.js (6 errors) ====
==== tests/cases/conformance/jsdoc/prefixPostfix.js (14 errors) ====
/**
* @param {number![]} x - number[]
* @param {!number[]} y - number[]
* @param {(number[])!} z - number[]
* @param {number?[]} a - (number | null)[]
* @param {number?[]} a - parse error without parentheses

!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
~
!!! error TS1005: '}' expected.
* @param {?number[]} b - number[] | null
* @param {(number[])?} c - number[] | null
* @param {...?number} e - (number | null)[]
Expand All @@ -23,17 +35,31 @@ tests/cases/conformance/jsdoc/prefixPostfix.js(13,12): error TS1014: A rest para
* @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?!} h - parse error without parentheses (also nonsensical)

!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
~
!!! error TS1005: '}' expected.
* @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)[]
* @param {...number?[]!} k - parse error without parentheses

!!! error TS8024: JSDoc '@param' tag has name '', but there is no parameter with that name.
~
!!! error TS1005: '}' expected.
* @param {number extends number ? true : false} l - conditional types work
* @param {[number, number?]} m - [number, (number | undefined)?]
*/
function f(x, y, z, a, b, c, e, f, g, h, i, j, k) {
function f(x, y, z, a, b, c, e, f, g, h, i, j, k, l, m) {
~
!!! error TS7006: Parameter 'a' implicitly has an 'any' type.
~
!!! error TS7006: Parameter 'h' implicitly has an 'any' type.
~
!!! error TS7006: Parameter 'k' implicitly has an 'any' type.
}

38 changes: 21 additions & 17 deletions tests/baselines/reference/jsdocPrefixPostfixParsing.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,35 @@
* @param {number![]} x - number[]
* @param {!number[]} y - number[]
* @param {(number[])!} z - number[]
* @param {number?[]} a - (number | null)[]
* @param {number?[]} a - parse error without parentheses
* @param {?number[]} b - number[] | null
* @param {(number[])?} c - number[] | null
* @param {...?number} e - (number | null)[]
* @param {...number?} f - number[] | null
* @param {...number!?} g - number[] | null
* @param {...number?!} h - number[] | null
* @param {...number?!} h - parse error without parentheses (also nonsensical)
* @param {...number[]} i - number[][]
* @param {...number![]?} j - number[][] | null
* @param {...number?[]!} k - (number[] | null)[]
* @param {...number?[]!} k - parse error without parentheses
* @param {number extends number ? true : false} l - conditional types work
* @param {[number, number?]} m - [number, (number | undefined)?]
*/
function f(x, y, z, a, b, c, e, f, g, h, i, j, k) {
function f(x, y, z, a, b, c, e, f, g, h, i, j, k, l, m) {
>f : Symbol(f, Decl(prefixPostfix.js, 0, 0))
>x : Symbol(x, Decl(prefixPostfix.js, 15, 11))
>y : Symbol(y, Decl(prefixPostfix.js, 15, 13))
>z : Symbol(z, Decl(prefixPostfix.js, 15, 16))
>a : Symbol(a, Decl(prefixPostfix.js, 15, 19))
>b : Symbol(b, Decl(prefixPostfix.js, 15, 22))
>c : Symbol(c, Decl(prefixPostfix.js, 15, 25))
>e : Symbol(e, Decl(prefixPostfix.js, 15, 28))
>f : Symbol(f, Decl(prefixPostfix.js, 15, 31))
>g : Symbol(g, Decl(prefixPostfix.js, 15, 34))
>h : Symbol(h, Decl(prefixPostfix.js, 15, 37))
>i : Symbol(i, Decl(prefixPostfix.js, 15, 40))
>j : Symbol(j, Decl(prefixPostfix.js, 15, 43))
>k : Symbol(k, Decl(prefixPostfix.js, 15, 46))
>x : Symbol(x, Decl(prefixPostfix.js, 17, 11))
>y : Symbol(y, Decl(prefixPostfix.js, 17, 13))
>z : Symbol(z, Decl(prefixPostfix.js, 17, 16))
>a : Symbol(a, Decl(prefixPostfix.js, 17, 19))
>b : Symbol(b, Decl(prefixPostfix.js, 17, 22))
>c : Symbol(c, Decl(prefixPostfix.js, 17, 25))
>e : Symbol(e, Decl(prefixPostfix.js, 17, 28))
>f : Symbol(f, Decl(prefixPostfix.js, 17, 31))
>g : Symbol(g, Decl(prefixPostfix.js, 17, 34))
>h : Symbol(h, Decl(prefixPostfix.js, 17, 37))
>i : Symbol(i, Decl(prefixPostfix.js, 17, 40))
>j : Symbol(j, Decl(prefixPostfix.js, 17, 43))
>k : Symbol(k, Decl(prefixPostfix.js, 17, 46))
>l : Symbol(l, Decl(prefixPostfix.js, 17, 49))
>m : Symbol(m, Decl(prefixPostfix.js, 17, 52))
}

20 changes: 12 additions & 8 deletions tests/baselines/reference/jsdocPrefixPostfixParsing.types
Original file line number Diff line number Diff line change
Expand Up @@ -3,31 +3,35 @@
* @param {number![]} x - number[]
* @param {!number[]} y - number[]
* @param {(number[])!} z - number[]
* @param {number?[]} a - (number | null)[]
* @param {number?[]} a - parse error without parentheses
* @param {?number[]} b - number[] | null
* @param {(number[])?} c - number[] | null
* @param {...?number} e - (number | null)[]
* @param {...number?} f - number[] | null
* @param {...number!?} g - number[] | null
* @param {...number?!} h - number[] | null
* @param {...number?!} h - parse error without parentheses (also nonsensical)
* @param {...number[]} i - number[][]
* @param {...number![]?} j - number[][] | null
* @param {...number?[]!} k - (number[] | null)[]
* @param {...number?[]!} k - parse error without parentheses
* @param {number extends number ? true : false} l - conditional types work
* @param {[number, number?]} m - [number, (number | undefined)?]
*/
function f(x, y, z, a, b, c, e, f, g, h, i, j, k) {
>f : (x: number[], y: number[], z: (number[]), a: (number | null)[], b: number[] | null, c: (number[]) | null, e: (number | null)[], f: (number | null)[], g: (number | null)[], h: (number | null)[], i: number[][], j: (number[] | null)[], k: (number | null)[][]) => void
function f(x, y, z, a, b, c, e, f, g, h, i, j, k, l, m) {
>f : (x: number[], y: number[], z: (number[]), a: any, b: number[] | null, c: (number[]) | null, e: (number | null)[], f: (number | null)[], g: (number | null)[], h: any, i: number[][], j: (number[] | null)[], k: any, l: number extends number ? true : false, m: [number, number?]) => void
>x : number[]
>y : number[]
>z : number[]
>a : (number | null)[]
>a : any
>b : number[] | null
>c : number[] | null
>e : number | null | undefined
>f : number | null | undefined
>g : number | null | undefined
>h : number | null | undefined
>h : any
>i : number[] | undefined
>j : number[] | null | undefined
>k : (number | null)[] | undefined
>k : any
>l : true
>m : [number, (number | undefined)?]
}

10 changes: 6 additions & 4 deletions tests/cases/conformance/jsdoc/jsdocPrefixPostfixParsing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,18 @@
* @param {number![]} x - number[]
* @param {!number[]} y - number[]
* @param {(number[])!} z - number[]
* @param {number?[]} a - (number | null)[]
* @param {number?[]} a - parse error without parentheses
* @param {?number[]} b - number[] | null
* @param {(number[])?} c - number[] | null
* @param {...?number} e - (number | null)[]
* @param {...number?} f - number[] | null
* @param {...number!?} g - number[] | null
* @param {...number?!} h - number[] | null
* @param {...number?!} h - parse error without parentheses (also nonsensical)
* @param {...number[]} i - number[][]
* @param {...number![]?} j - number[][] | null
* @param {...number?[]!} k - (number[] | null)[]
* @param {...number?[]!} k - parse error without parentheses
* @param {number extends number ? true : false} l - conditional types work
* @param {[number, number?]} m - [number, (number | undefined)?]
*/
function f(x, y, z, a, b, c, e, f, g, h, i, j, k) {
function f(x, y, z, a, b, c, e, f, g, h, i, j, k, l, m) {
}

0 comments on commit c3c6be6

Please sign in to comment.