Get return type from @type tag
#25580
Conversation
This only happens in the checker, where the type is easily accessible. The syntax-based check in getEffectiveReturnTypeNode as a fast path, and for other uses that don't want to make a call to getTypeFromTypeNode. Fixes #25525
src/compiler/checker.ts
Outdated
| if (nodeIsMissing((<FunctionLikeDeclaration>declaration).body)) { | ||
| return anyType; | ||
| } | ||
| } | ||
|
|
||
| function getReturnTypeOfTypeTag(node: Node) { |
There was a problem hiding this comment.
Think this would be neater with node: SignatureDeclaration | JSDocSignature
src/compiler/checker.ts
Outdated
| @@ -20521,23 +20536,28 @@ namespace ts { | |||
| return type; | |||
| } | |||
|
|
|||
| function getReturnOrPromisedType(node: ArrowFunction | FunctionExpression | FunctionDeclaration | MethodDeclaration | MethodSignature, functionFlags: FunctionFlags) { | |||
There was a problem hiding this comment.
Could be node: FunctionLikeDeclaration | MethodSignature
| } | ||
|
|
||
| /** @type {2 | 3} - overloads are not supported, so there will be an error */ | ||
| var twothree = monaLisa(false); |
There was a problem hiding this comment.
The error here doesn't say anything about overloads through. (error TS2322: Type '1 | 2' is not assignable to type '2 | 3'.) The inferred return type of the function 1 | 2 is used, but it really shouldn't if there was an explicit type annotation. If the function returned any, there would be silent failure here, even though it had an explicit type annotation.
There was a problem hiding this comment.
You're right, it's only an error because we go back to ignoring the type annotation. The correct thing is to treat a type annotation with multiple signatures as an overload set and use the normal typescript rules for checking that the implementation signature is assignable to the overload set.
But the JS implementation isn't typed, so that check doesn't really make sense. Basically, I left it the way it was because I couldn't think of any useful checking.
There was a problem hiding this comment.
After discussing this in person, here's what we decided
/** @type {number} */should error. It doesn't today, but I think it's clearly wrong enough to deserve an error./** @type {Gioconda} */should givemonaLisaan overload type, and the function parameters and return type should be checked according to the typescript overload rules. This will usually succeed since the parameters will beany, and the return type will therefore usually beany. But it will give the desired overload type to callers of the function, which is why somebody would write this type in the first place.- (2) is not just a simple fix so I will leave the current silent-fallback behaviour in this PR and open an issue to track it.
There was a problem hiding this comment.
Opened #25590 to track (2).
src/compiler/checker.ts
Outdated
| if (nodeIsMissing((<FunctionLikeDeclaration>declaration).body)) { | ||
| return anyType; | ||
| } | ||
| } | ||
|
|
||
| function getReturnTypeOfTypeTag(node: Node) { | ||
| if (isInJavaScriptFile(node)) { |
There was a problem hiding this comment.
Opinons may vary but I tend to prefer to be flatter when lots of sequential tests are involved:
const typeTag = isInJavaScriptFile(node) ? getJSDocTypeTag(node) : undefined;
const signatures = typeTag && typeTag.typeExpression && getSignaturesOfType(getTypeFromTypeNode(typeTag.typeExpression), SignatureKind.Call);
return signatures && signatures.length === 1 ? getReturnTypeOfSignature(signatures[0]) : undefined;
src/compiler/checker.ts
Outdated
| @@ -20521,23 +20536,28 @@ namespace ts { | |||
| return type; | |||
| } | |||
|
|
|||
| function getReturnOrPromisedType(node: ArrowFunction | FunctionExpression | FunctionDeclaration | MethodDeclaration | MethodSignature, functionFlags: FunctionFlags) { | |||
| const returnTypeNode = getEffectiveReturnTypeNode(node); | |||
There was a problem hiding this comment.
This is only used if (functionFlags & FunctionFlags.AsyncGenerator) !== FunctionFlags.Async, could it be computed only then?
There was a problem hiding this comment.
checkAsyncFunctionReturnType immediately calls getEffectiveReturnTypeNode, and assumes that its result is defined, so it implicitly used it. I added a parameter returnTypeNode to make the dependency explicit (and get rid of yet another TODO #18217)
|
I added a separate error for when the type tag isn't callable. It ended up separate from |
ghost
mentioned this pull request
This only happens in the checker, where the type is easily accessible. The syntax-based check in getEffectiveReturnTypeNode remains as a fast path, and for other uses that can't make a call to getTypeFromTypeNode.
Fixes #25525