JSDoc ?Type adds optionality to parameters (#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.
This commit is contained in:
Nathan Shively-Sanders
2018-03-16 13:28:24 -07:00
committed by GitHub
parent 99d5058568
commit 3b6ae8536f
6 changed files with 229 additions and 250 deletions

View File

@@ -4164,7 +4164,7 @@ namespace ts {
}
const isOptional = includeOptionality && (
isInJavaScriptFile(declaration) && isParameter(declaration) && getJSDocParameterTags(declaration).some(tag => tag.isBracketed)
isParameter(declaration) && isJSDocOptionalParameter(declaration)
|| !isBindingElement(declaration) && !isVariableDeclaration(declaration) && !!declaration.questionToken);
// Use type from type annotation if one is present
@@ -6571,9 +6571,17 @@ namespace ts {
function isJSDocOptionalParameter(node: ParameterDeclaration) {
return isInJavaScriptFile(node) && (
// node.type should only be a JSDocOptionalType when node is a parameter of a JSDocFunctionType
node.type && node.type.kind === SyntaxKind.JSDocOptionalType
|| getJSDocParameterTags(node).some(({ isBracketed, typeExpression }) =>
isBracketed || !!typeExpression && typeExpression.type.kind === SyntaxKind.JSDocOptionalType));
isBracketed || !!typeExpression && skipJSDocPrefixTypes(typeExpression.type).kind === SyntaxKind.JSDocOptionalType));
}
function skipJSDocPrefixTypes(type: TypeNode): TypeNode {
while (type.kind === SyntaxKind.JSDocNullableType || type.kind === SyntaxKind.JSDocNonNullableType) {
type = (type as JSDocNullableType | JSDocNonNullableType).type;
}
return type;
}
function tryFindAmbientModule(moduleName: string, withAugmentations: boolean) {

View File

@@ -0,0 +1,31 @@
tests/cases/conformance/jsdoc/a.js(4,5): error TS2322: Type 'null' is not assignable to type 'number | undefined'.
tests/cases/conformance/jsdoc/a.js(8,3): error TS2345: Argument of type 'null' is not assignable to parameter of type 'number | undefined'.
==== tests/cases/conformance/jsdoc/a.js (2 errors) ====
/** @param {number=} a */
function f(a) {
a = 1
a = null // should not be allowed
~
!!! error TS2322: Type 'null' is not assignable to type 'number | undefined'.
a = undefined
}
f()
f(null) // should not be allowed
~~~~
!!! error TS2345: Argument of type 'null' is not assignable to parameter of type 'number | undefined'.
f(undefined)
f(1)
/** @param {???!?number?=} a */
function g(a) {
a = 1
a = null
a = undefined
}
g()
g(null)
g(undefined)
g(1)

View File

@@ -0,0 +1,57 @@
=== tests/cases/conformance/jsdoc/a.js ===
/** @param {number=} a */
function f(a) {
>f : Symbol(f, Decl(a.js, 0, 0))
>a : Symbol(a, Decl(a.js, 1, 11))
a = 1
>a : Symbol(a, Decl(a.js, 1, 11))
a = null // should not be allowed
>a : Symbol(a, Decl(a.js, 1, 11))
a = undefined
>a : Symbol(a, Decl(a.js, 1, 11))
>undefined : Symbol(undefined)
}
f()
>f : Symbol(f, Decl(a.js, 0, 0))
f(null) // should not be allowed
>f : Symbol(f, Decl(a.js, 0, 0))
f(undefined)
>f : Symbol(f, Decl(a.js, 0, 0))
>undefined : Symbol(undefined)
f(1)
>f : Symbol(f, Decl(a.js, 0, 0))
/** @param {???!?number?=} a */
function g(a) {
>g : Symbol(g, Decl(a.js, 9, 4))
>a : Symbol(a, Decl(a.js, 12, 11))
a = 1
>a : Symbol(a, Decl(a.js, 12, 11))
a = null
>a : Symbol(a, Decl(a.js, 12, 11))
a = undefined
>a : Symbol(a, Decl(a.js, 12, 11))
>undefined : Symbol(undefined)
}
g()
>g : Symbol(g, Decl(a.js, 9, 4))
g(null)
>g : Symbol(g, Decl(a.js, 9, 4))
g(undefined)
>g : Symbol(g, Decl(a.js, 9, 4))
>undefined : Symbol(undefined)
g(1)
>g : Symbol(g, Decl(a.js, 9, 4))

View File

@@ -0,0 +1,79 @@
=== tests/cases/conformance/jsdoc/a.js ===
/** @param {number=} a */
function f(a) {
>f : (a?: number | undefined) => void
>a : number | undefined
a = 1
>a = 1 : 1
>a : number | undefined
>1 : 1
a = null // should not be allowed
>a = null : null
>a : number | undefined
>null : null
a = undefined
>a = undefined : undefined
>a : number | undefined
>undefined : undefined
}
f()
>f() : void
>f : (a?: number | undefined) => void
f(null) // should not be allowed
>f(null) : void
>f : (a?: number | undefined) => void
>null : null
f(undefined)
>f(undefined) : void
>f : (a?: number | undefined) => void
>undefined : undefined
f(1)
>f(1) : void
>f : (a?: number | undefined) => void
>1 : 1
/** @param {???!?number?=} a */
function g(a) {
>g : (a?: number | null | undefined) => void
>a : number | null | undefined
a = 1
>a = 1 : 1
>a : number | null | undefined
>1 : 1
a = null
>a = null : null
>a : number | null | undefined
>null : null
a = undefined
>a = undefined : undefined
>a : number | null | undefined
>undefined : undefined
}
g()
>g() : void
>g : (a?: number | null | undefined) => void
g(null)
>g(null) : void
>g : (a?: number | null | undefined) => void
>null : null
g(undefined)
>g(undefined) : void
>g : (a?: number | null | undefined) => void
>undefined : undefined
g(1)
>g(1) : void
>g : (a?: number | null | undefined) => void
>1 : 1

File diff suppressed because it is too large Load Diff

View File

@@ -0,0 +1,26 @@
// @Filename: a.js
// @noEmit: true
// @allowJs: true
// @checkJs: true
// @strict: true
/** @param {number=} a */
function f(a) {
a = 1
a = null // should not be allowed
a = undefined
}
f()
f(null) // should not be allowed
f(undefined)
f(1)
/** @param {???!?number?=} a */
function g(a) {
a = 1
a = null
a = undefined
}
g()
g(null)
g(undefined)
g(1)