Skip to content

Commit 3b6ae85

Browse files
sandersnsandersn
authored
JSDoc ?Type adds optionality to parameters (microsoft#22646)
* jsdoc ?Type adds optionality to parameters Chrome devtools expects that parameters with type `?T` (or `T?`) add null to `T` and optionality to the parameter. Previously it only added null to the type. Currently the PR does *not* add undefined to the type of `T`, which is expected by chrome-devtools-frontend, but is inconsistent with typescript's rules. The implementation achieves this inconsistency by exploiting the fact that checking the signature adds optionality and checking the parameter adds `undefined`. * Update chrome-devtools-frontend baseline * Add optionality only for jsdoc postfix= * Skip jsdoc prefix types in isJSDocOptionalParameter Previously isJSDocOptionalParameter was incorrect for types like `?number=`, which are optional but have JSDocNullableType as their root type node.
1 parent 99d5058 commit 3b6ae85

6 files changed

Lines changed: 229 additions & 250 deletions

File tree

‎src/compiler/checker.ts‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4164,7 +4164,7 @@ namespace ts {
41644164
}
41654165

41664166
const isOptional = includeOptionality && (
4167-
isInJavaScriptFile(declaration) && isParameter(declaration) && getJSDocParameterTags(declaration).some(tag => tag.isBracketed)
4167+
isParameter(declaration) && isJSDocOptionalParameter(declaration)
41684168
|| !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken);
41694169

41704170
// Use type from type annotation if one is present
@@ -6571,9 +6571,17 @@ namespace ts {
65716571

65726572
function isJSDocOptionalParameter(node: ParameterDeclaration) {
65736573
return isInJavaScriptFile(node) && (
6574+
// node.type should only be a JSDocOptionalType when node is a parameter of a JSDocFunctionType
65746575
node.type && node.type.kind === SyntaxKind.JSDocOptionalType
65756576
|| getJSDocParameterTags(node).some(({ isBracketed, typeExpression }) =>
6576-
isBracketed || !!typeExpression && typeExpression.type.kind === SyntaxKind.JSDocOptionalType));
6577+
isBracketed || !!typeExpression && skipJSDocPrefixTypes(typeExpression.type).kind === SyntaxKind.JSDocOptionalType));
6578+
}
6579+
6580+
function skipJSDocPrefixTypes(type: TypeNode): TypeNode {
6581+
while (type.kind === SyntaxKind.JSDocNullableType || type.kind === SyntaxKind.JSDocNonNullableType) {
6582+
type = (type as JSDocNullableType | JSDocNonNullableType).type;
6583+
}
6584+
return type;
65776585
}
65786586

65796587
function tryFindAmbientModule(moduleName: string, withAugmentations: boolean) {

‎tests/baselines/reference/jsdocPostfixEqualsAddsOptionality.errors.txt‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
tests/cases/conformance/jsdoc/a.js(4,5): error TS2322: Type 'null' is not assignable to type 'number | undefined'.
2+
tests/cases/conformance/jsdoc/a.js(8,3): error TS2345: Argument of type 'null' is not assignable to parameter of type 'number | undefined'.
3+
4+
5+
==== tests/cases/conformance/jsdoc/a.js (2 errors) ====
6+
/** @param {number=} a */
7+
function f(a) {
8+
a = 1
9+
a = null // should not be allowed
10+
~
11+
!!! error TS2322: Type 'null' is not assignable to type 'number | undefined'.
12+
a = undefined
13+
}
14+
f()
15+
f(null) // should not be allowed
16+
~~~~
17+
!!! error TS2345: Argument of type 'null' is not assignable to parameter of type 'number | undefined'.
18+
f(undefined)
19+
f(1)
20+
21+
/** @param {???!?number?=} a */
22+
function g(a) {
23+
a = 1
24+
a = null
25+
a = undefined
26+
}
27+
g()
28+
g(null)
29+
g(undefined)
30+
g(1)
31+

‎tests/baselines/reference/jsdocPostfixEqualsAddsOptionality.symbols‎

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
=== tests/cases/conformance/jsdoc/a.js ===
2+
/** @param {number=} a */
3+
function f(a) {
4+
>f : Symbol(f, Decl(a.js, 0, 0))
5+
>a : Symbol(a, Decl(a.js, 1, 11))
6+
7+
a = 1
8+
>a : Symbol(a, Decl(a.js, 1, 11))
9+
10+
a = null // should not be allowed
11+
>a : Symbol(a, Decl(a.js, 1, 11))
12+
13+
a = undefined
14+
>a : Symbol(a, Decl(a.js, 1, 11))
15+
>undefined : Symbol(undefined)
16+
}
17+
f()
18+
>f : Symbol(f, Decl(a.js, 0, 0))
19+
20+
f(null) // should not be allowed
21+
>f : Symbol(f, Decl(a.js, 0, 0))
22+
23+
f(undefined)
24+
>f : Symbol(f, Decl(a.js, 0, 0))
25+
>undefined : Symbol(undefined)
26+
27+
f(1)
28+
>f : Symbol(f, Decl(a.js, 0, 0))
29+
30+
/** @param {???!?number?=} a */
31+
function g(a) {
32+
>g : Symbol(g, Decl(a.js, 9, 4))
33+
>a : Symbol(a, Decl(a.js, 12, 11))
34+
35+
a = 1
36+
>a : Symbol(a, Decl(a.js, 12, 11))
37+
38+
a = null
39+
>a : Symbol(a, Decl(a.js, 12, 11))
40+
41+
a = undefined
42+
>a : Symbol(a, Decl(a.js, 12, 11))
43+
>undefined : Symbol(undefined)
44+
}
45+
g()
46+
>g : Symbol(g, Decl(a.js, 9, 4))
47+
48+
g(null)
49+
>g : Symbol(g, Decl(a.js, 9, 4))
50+
51+
g(undefined)
52+
>g : Symbol(g, Decl(a.js, 9, 4))
53+
>undefined : Symbol(undefined)
54+
55+
g(1)
56+
>g : Symbol(g, Decl(a.js, 9, 4))
57+

‎tests/baselines/reference/jsdocPostfixEqualsAddsOptionality.types‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
=== tests/cases/conformance/jsdoc/a.js ===
2+
/** @param {number=} a */
3+
function f(a) {
4+
>f : (a?: number | undefined) => void
5+
>a : number | undefined
6+
7+
a = 1
8+
>a = 1 : 1
9+
>a : number | undefined
10+
>1 : 1
11+
12+
a = null // should not be allowed
13+
>a = null : null
14+
>a : number | undefined
15+
>null : null
16+
17+
a = undefined
18+
>a = undefined : undefined
19+
>a : number | undefined
20+
>undefined : undefined
21+
}
22+
f()
23+
>f() : void
24+
>f : (a?: number | undefined) => void
25+
26+
f(null) // should not be allowed
27+
>f(null) : void
28+
>f : (a?: number | undefined) => void
29+
>null : null
30+
31+
f(undefined)
32+
>f(undefined) : void
33+
>f : (a?: number | undefined) => void
34+
>undefined : undefined
35+
36+
f(1)
37+
>f(1) : void
38+
>f : (a?: number | undefined) => void
39+
>1 : 1
40+
41+
/** @param {???!?number?=} a */
42+
function g(a) {
43+
>g : (a?: number | null | undefined) => void
44+
>a : number | null | undefined
45+
46+
a = 1
47+
>a = 1 : 1
48+
>a : number | null | undefined
49+
>1 : 1
50+
51+
a = null
52+
>a = null : null
53+
>a : number | null | undefined
54+
>null : null
55+
56+
a = undefined
57+
>a = undefined : undefined
58+
>a : number | null | undefined
59+
>undefined : undefined
60+
}
61+
g()
62+
>g() : void
63+
>g : (a?: number | null | undefined) => void
64+
65+
g(null)
66+
>g(null) : void
67+
>g : (a?: number | null | undefined) => void
68+
>null : null
69+
70+
g(undefined)
71+
>g(undefined) : void
72+
>g : (a?: number | null | undefined) => void
73+
>undefined : undefined
74+
75+
g(1)
76+
>g(1) : void
77+
>g : (a?: number | null | undefined) => void
78+
>1 : 1
79+

0 commit comments

Comments
 (0)