Improve contextual types using jsdoc tags #16346
Conversation
| // @filename: index.js | ||
|
|
||
| /** @type {Array<[string, {x?:number, y?:number}]>} */ | ||
| const arr = [ |
There was a problem hiding this comment.
Also test with function return type and get/set accessor types.
src/compiler/checker.ts
Outdated
| @@ -12744,6 +12744,10 @@ namespace ts { | |||
| if (declaration.type) { | |||
| return getTypeFromTypeNode(declaration.type); | |||
| } | |||
| const jsDocType = isInJavaScriptFile(declaration) && getTypeForDeclarationFromJSDocComment(declaration); | |||
There was a problem hiding this comment.
Nit: the style here differs from the case immediately below it, although the two seem to be accomplishing the same thing. I personally prefer the below style (though with if (type) return type; on one line).
src/compiler/checker.ts
Outdated
| if (returnTag && returnTag.typeExpression) { | ||
| return getTypeFromTypeNode(returnTag.typeExpression.type); | ||
| const jsdocType = getJSDocReturnType(func); | ||
| if (jsdocType) { |
There was a problem hiding this comment.
return jsdocType && getTypeFromTypeNode(jsdocType);
src/compiler/utilities.ts
Outdated
| if (accessor && accessor.parameters.length > 0) { | ||
| const hasThis = accessor.parameters.length === 2 && parameterIsThisKeyword(accessor.parameters[0]); | ||
| return accessor.parameters[hasThis ? 1 : 0].type; | ||
| const parameter = accessor.parameters[hasThis ? 1 : 0]; | ||
| if (parameter) { |
There was a problem hiding this comment.
Why do we need the if here, since we just checked the length?
There was a problem hiding this comment.
Holdover from when I considered splitting this into two functions.
src/compiler/utilities.ts
Outdated
| /** Get the type annotaion for the value parameter. */ | ||
| export function getSetAccessorTypeAnnotationNode(accessor: SetAccessorDeclaration): TypeNode { | ||
| /** Get the type annotation for the value parameter. */ | ||
| export function getSetAccessorTypeAnnotationNode(accessor: SetAccessorDeclaration, includeJSDocType?: boolean): TypeNode { |
There was a problem hiding this comment.
Shouldn't getAnnotatedAccessorType and isGetAccessorWithAnnotatedSetAccessor call this with includeJSDocType? Would it ever hurt to include that?
There was a problem hiding this comment.
I considered actually making this a separate function, as getSetAccessorTypeAnnotationNode very clearly states its getting the type annotation, not the jsdoc comment. I don't want to just blindly grab the jsdoc comment.
There was a problem hiding this comment.
Why should JS have different behavior than TS here though? JS users have no option to use a "real" type annotation.
There was a problem hiding this comment.
getSetAccessorTypeAnnotationNode is used in the transforms as well as the checker. At the point where it is used it is expecting a lexical type annotation node, not an inferred type annotation node.
ghost
mentioned this pull request
This addresses the fact that we do not currently use jsdoc
/** @type */tags when getting the contextual type of a variable declaration in a JavaScript file.Fixes #15618
Fixes #16109