Full-signature types for JSDoc #1468

sandersn · 2025-07-28T17:18:35Z

This PR allows @type tags to apply as a whole to a function. This is a returning Strada feature, but it's no longer implemented as a fallback lookup for a type tag. Instead, FunctionLikeBase now has a property WholeType that gets set in the reparser. If we can come up with Typescript syntax for this feature, it will work immediately--with a couple of caveats described below.

In the checker, the implementation is basically the same as in Strada, except that it refers to a property on FunctionLikeBase instead of looking up a tag. I put it on FunctionLikeBase because it is more convenient to retrieve, even though it only applies to FunctionLikeWithBodyBase. Notably, there was one check that I skipped because it short-circuits contextual typing in favour of the whole type, which I don't believe is correct--or is perhaps unobservable. When I removed it no tests changed.

I didn't check that declaration emit works, but I think there may be some tests that exercise it. In any case, since the signature's types are pulled from the whole-type when needed, and parameter/return types are supposed to be nil when it's present, I expect that emit will generate types correctly from the signature.

Another difference is that the parameter names of the resulting signature come from the type annotation, not the function declaration. This is consistent with something like

const x: number = 'xx'

where x: number when used even though it is initialised with a string, which of course is a string.

Speaking of errors, I improved the wording of the existing error and added another that forbids using @type and @param/@return. Both errors are worded in terms of JSDoc tags even though we could implement this feature in TS. We can revisit this either after coming up with a good name for this feature, or after we implement it in TS, if ever.

Open Questions

What should this feature be called? Whole-function type annotation? Function annotation? TypeTag type annotation? This leads naturally to:
What should the property on FunctionLikeBase be called?
Can the errors be improved further, either the wording or the checks?
(bonus) What would TS syntax for this look like?

Open Tasks

Rename all the properties and functions to use the new terminology.

…s a returning Strada feature, but it's no longer implemented as a fallback lookup for a type tag. Instead, `FunctionLikeBase` now has a property `WholeType` that gets set in the reparser. If we can come up with Typescript syntax for this feature, it will work immediately--with a couple of caveats described below. In the checker, the implementation is basically the same as in Strada, except that it refers to a property on FunctionLikeBase instead of looking up a tag. I put it on FunctionLikeBase because it is more convenient to retrieve, even though it only applies to FunctionLikeWithBodyBase. Notably, there was one check that I skipped because it short-circuits contextual typing in favour of the whole type, which I don't believe is correct--or is perhaps unobservable. When I removed it no tests changed. I didn't check that declaration emit works, but I think there may be some tests that exercise it. In any case, since the signature's types are pulled from the whole-type when needed, and parameter/return types are supposed to be nil when it's present, I expect that emit will generate types correctly from the signature. Another difference is that the parameter names of the resulting signature come from the type annotation, not the function declaration. This is consistent with something like ```ts const x: number = 'xx' ``` where `x: number` when used even though it is initialised with a string, which of course is a string. Speaking of errors, I improved the wording of the existing error and added another that forbids using `@type` and `@param/@return`. Both errors are worded in terms of JSDoc tags even though we could implement this feature in TS. We can revisit this either after coming up with a good name for this feature, or after we implement it in TS, if ever. \#\# Open Questions 1. What should this feature be called? 2. Where should the code live? 3. Can the errors be improved further? 4. What would TS syntax for this look like?

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

testdata/baselines/reference/submodule/conformance/typeTagOnFunctionReferencesGeneric.js.diff

sandersn · 2025-07-28T17:25:32Z

...erence/submoduleAccepted/compiler/contextuallyTypedParametersOptionalInJSDoc.errors.txt.diff

+-!!! error TS2345:   Type 'undefined' is not assignable to type 'number'.
+         self("");
+        ~~~~
+!!! error TS2554: Expected 2 arguments, but got 1.


you can no longer make a parameter from a whole-type annotation optional by tacking on a nested /** @param [b] */, sorry, it's a horrible idea that should never have worked.

testdata/baselines/reference/submoduleAccepted/conformance/checkJsdocTypeTag6.errors.txt.diff

sandersn · 2025-07-28T17:27:04Z

...oduleAccepted/compiler/moduleExportsTypeNoExcessPropertyCheckFromContainedLiteral.types.diff

@@ -14,8 +14,8 @@
 const tseslint = require('./typescript-eslint.js');
 ->tseslint : typeof tseslint
 ->require('./typescript-eslint.js') : typeof tseslint
-+>tseslint : { config: (...configs: any[]) => void; }
-+>require('./typescript-eslint.js') : { config: (...configs: any[]) => void; }
+>tseslint : { config: (...configs: import("./typescript-eslint.js").Config[]) => void; }


we now dereference the imported type instead of leaving it as typeof eslint.

...lines/reference/submoduleAccepted/conformance/asyncFunctionDeclaration16_es5.errors.txt.diff

testdata/baselines/reference/submoduleAccepted/conformance/checkJsdocTypeTag6.types.diff

...aselines/reference/submoduleAccepted/conformance/jsdocFunction_missingReturn.errors.txt.diff

sandersn · 2025-07-28T17:58:59Z

testdata/baselines/reference/submoduleAccepted/conformance/callbackTagNamespace.errors.txt.diff

+               ~~
+!!! error TS2702: 'NS' only refers to a type, but is being used as a namespace here.
+               ~~~~~~~~~~~~~~~
+!!! error TS8030: A JSDoc '@type' tag on a function must have a signature with the correct number of arguments.


our fancy span for the first error hurts us because the useless second error would get deduped if it used the whole span of the name.

testdata/baselines/reference/submoduleAccepted/conformance/checkJsdocTypeTag6.errors.txt.diff

internal/diagnostics/diagnostics_generated.go

.../baselines/reference/submoduleAccepted/conformance/errorOnFunctionReturnType.errors.txt.diff

1. Improve error spans on new/updated errors. 2. Improve check for new/updated errors. 3. Add (a lot of) code to emit type parameters in d.ts. I'm not at all fluent with the declaration emitter code, so I need careful review of it.

Thanks unified.js! Most complicated JSDoc I have ever seen.

paoloricciuti · 2025-07-29T15:47:23Z

I love this and i would love to have the same in TS. Could this be a potential syntax?

function my_function(){

} typed MyFunction;

this would require a new construct but it seems very legible

.../baselines/reference/submoduleAccepted/conformance/errorOnFunctionReturnType.errors.txt.diff

jakebailey · 2025-07-29T17:48:00Z

Other than the one error positioning thing, I think this seems good.

The CHANGES.md file also needs an update, but I think that PR was never merged so that's not in this PR.

sandersn · 2025-07-29T20:19:40Z

Yeah, I got sidetracked from updating CHANGES.md.
I'll take another look at the position tomorrow--yesterday I was convinced that TS would already work the new way in certain circumstances, but I need to double-check.

sandersn · 2025-07-29T21:45:07Z

@paoloricciuti I'm afraid that would be ambiguous with existing expression syntax.

Really what we should do is start a new issue to track this idea, but it sounds like the team has tried to come up with ideas in the past and failed to find any that more than 1 person likes.

sandersn · 2025-07-30T13:35:00Z

I renamed WholeType to FullSignature based on ideas from the design meeting yesterday, so I think this is ready for final review. I decided to keep the new error span since it's closer to the intent of Typescript's span. Also I didn't want to write any more special-case code for functions (which may be how the TS code was arrived at as well)

jakebailey · 2025-07-30T16:08:17Z

I decided to keep the new error span since it's closer to the intent of Typescript's span.

Can you explain what you mean by this?

I'm surprised by the current location just because the error goes on the type and not on the thing causing the error. It feel like if we had a function returning the wrong type, so put the error on the return type instead of the returned expression.

sandersn · 2025-07-30T16:40:08Z

Yes. The Typescript code is OrElse(fn.Type(), fn), and the errors show up on the return type annotation. I think the closest thing with here is the full signature type annotation. I agree that it's strange for this particular error but it's better for the other 3. I think it was OK by mistake in Strada but when I weighed the time needed to figure out the special case code to be equivalent versus following the intent of the (simpler) TS code path, I decided on the simpler way.

jakebailey · 2025-07-30T17:05:22Z

I think you might misunderstand my example? I'm comparing this PR's behavior to if you wrote:

function foo(): number {
    return "Hello!";
}

And the error were placed on number and not on the return expression, where the bug is.

I suppose if this PR had "real syntax", like:

function foo() :: Hmm {
    return "Hello!";
}

And the func were missing a parameter Hmm asks for or something, it'd be questionable where the error goes, but I just find it odd to have:

/** @type {Hmm} */
function foo() {
    return "Hello!";
}

And have the error appear on Hmm, when it's the function that would need to change?

jakebailey

Implementation wise, this seems fine, though.

The only other thing I was wondering is if we really need this on FunctionLikeData and not just a specific node or two (does this actually work on accessors? Is it tested?), but it's probably okay.

sandersn · 2025-07-30T18:02:40Z

it is supposed to work on accessors; I don't know if we have test coverage for it.

sandersn added 2 commits July 28, 2025 10:04

Remove TODOs

aa27dcc

Copilot AI review requested due to automatic review settings July 28, 2025 17:18

Copilot AI reviewed Jul 28, 2025

View reviewed changes

jakebailey reviewed Jul 28, 2025

View reviewed changes

testdata/baselines/reference/submodule/conformance/typeTagOnFunctionReferencesGeneric.js.diff Outdated Show resolved Hide resolved

sandersn commented Jul 28, 2025

View reviewed changes

jakebailey reviewed Jul 28, 2025

View reviewed changes

testdata/baselines/reference/submoduleAccepted/conformance/checkJsdocTypeTag6.errors.txt.diff Outdated Show resolved Hide resolved

sandersn commented Jul 28, 2025

View reviewed changes

...lines/reference/submoduleAccepted/conformance/asyncFunctionDeclaration16_es5.errors.txt.diff Outdated Show resolved Hide resolved

jakebailey reviewed Jul 28, 2025

View reviewed changes

testdata/baselines/reference/submoduleAccepted/conformance/checkJsdocTypeTag6.types.diff Outdated Show resolved Hide resolved

jakebailey reviewed Jul 28, 2025

View reviewed changes

...aselines/reference/submoduleAccepted/conformance/jsdocFunction_missingReturn.errors.txt.diff Show resolved Hide resolved

sandersn added 2 commits July 28, 2025 10:32

Merge branch 'main' into whole-ass-type

b415fc3

Do not double-reparse @type to functions and variables

37480d7

sandersn commented Jul 28, 2025

View reviewed changes

testdata/baselines/reference/submoduleAccepted/conformance/checkJsdocTypeTag6.errors.txt.diff Show resolved Hide resolved

jakebailey reviewed Jul 28, 2025

View reviewed changes

internal/diagnostics/diagnostics_generated.go Show resolved Hide resolved

jakebailey reviewed Jul 28, 2025

View reviewed changes

.../baselines/reference/submoduleAccepted/conformance/errorOnFunctionReturnType.errors.txt.diff Outdated Show resolved Hide resolved

sandersn added 2 commits July 29, 2025 07:39

Address PR comments

f26e2e8

1. Improve error spans on new/updated errors. 2. Improve check for new/updated errors. 3. Add (a lot of) code to emit type parameters in d.ts. I'm not at all fluent with the declaration emitter code, so I need careful review of it.

Fix this-type retrieval

be0d410

Thanks unified.js! Most complicated JSDoc I have ever seen.

sandersn mentioned this pull request Jul 29, 2025

chore:Update for TS7 sveltejs/svelte#16485

Merged

jakebailey reviewed Jul 29, 2025

View reviewed changes

.../baselines/reference/submoduleAccepted/conformance/errorOnFunctionReturnType.errors.txt.diff Show resolved Hide resolved

sandersn changed the title ~~Whole-assignment types for JSDoc~~ Full-signature types for JSDoc Jul 29, 2025

Rename WholeType->FullSignature

5fb637f

jakebailey approved these changes Jul 30, 2025

View reviewed changes

sandersn added this pull request to the merge queue Jul 30, 2025

Merged via the queue into microsoft:main with commit 008942f Jul 30, 2025
22 checks passed

sandersn deleted the whole-ass-type branch July 30, 2025 18:16

sandersn mentioned this pull request Aug 11, 2025

Custom JSDoc type cannot be extended microsoft/TypeScript#62021

Open

Full-signature types for JSDoc #1468

Full-signature types for JSDoc #1468

Uh oh!

Conversation

sandersn commented Jul 28, 2025

Open Questions

Open Tasks

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sandersn Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

sandersn Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sandersn Jul 28, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

paoloricciuti commented Jul 29, 2025

Uh oh!

Uh oh!

jakebailey commented Jul 29, 2025

Uh oh!

sandersn commented Jul 29, 2025

Uh oh!

sandersn commented Jul 29, 2025

Uh oh!

sandersn commented Jul 30, 2025

Uh oh!

jakebailey commented Jul 30, 2025

Uh oh!

sandersn commented Jul 30, 2025

Uh oh!

jakebailey commented Jul 30, 2025

Uh oh!

jakebailey left a comment

Choose a reason for hiding this comment

Uh oh!

sandersn commented Jul 30, 2025

Uh oh!

Uh oh!

Uh oh!