Expandable hover

[gabritto]

Fixes #59029.

The basic idea of how this feature works is that the current quickinfo response corresponds to verbosityLevel 0, and each increase after that means expanding all* of the aliases we currently display.

For instance:

interface BaseType<T> {
    baseProp: () => T;
}

interface SomeType<T> extends BaseType<T>{
    prop1: T;
    prop2: T extends symbol ? string : number;
}

declare const a: SomeType<string>;
declare const b: SomeType<symbol>;

Hovering over a shows you this:
At level 0 (initial hover):

const a: SomeType<string>

At level 1:

const a: {
    prop1: string;
    prop2: number;
    baseProp: () => string;
}

Note how we show you the fully resolved type, i.e. fully instantiated, with conditional types resolved, inherited members, etc.
That's what I think is one of the main advantages of this feature, compared to just using "go to definition".

For top-level declarations, we also now expand to show the full declaration. Considering the same code as above, if we hover over SomeType, this is what we see:
Level 0:

interface SomeType<T>

Level 1:

interface SomeType<T> extends BaseType<T> {
    prop1: T;
    prop2: T extends symbol ? string : number;
}

So this also makes it convenient to see the declaration without having to navigate to a different place, e.g. a different file.

Implementation

The first kind of hover, hover on a type, works via typeToString, i.e. type printing, and the second kind of hover, on declarations, works via the serializeX functions, i.e. the functions responsible for d.ts emit.
For the latter, some noteworthy changes are that I added a new symbolToDeclarations function to nodeBuilder, and I added truncation to those functions to match the truncation that we have in typeToString (see f9d2309). I also had to change how some constructs are printed, e.g. instead of showing #private in a class with private identifiers, we now show all private identifiers.
All of those changes to the d.ts emit functions should only happen when we are calling those functions for quickinfo purposes.

The expansion is controlled by new properties on the NodeBuilderContext that is shared for all of the functions mentioned above. We have a maximum depth of expansion that corresponds to a verbosity level, and we also keep track of how many levels of aliases/definitions we have expanded so far, as we visit the tree to produce a hover. Whenever we decide to expand something, we increase that by one level.

We also need to produce output when we call those functions. We do this by having an out property on the context.
The output is basically two booleans:

• couldUnfoldMore, which will correspond to canIncreaseVerbosityLevel, and is true if increasing the verbosity level by one would produce a new expansion.
• truncated, which is true if we did truncation. If we did truncation, we will always return false for canIncreaseVerbosityLevel, because increasing verbosity would only produce a larger hover, and the current one is already too large.

Testing

To test this feature in vscode, you need to downgrade your vscode version to the January version (1.97): https://code.visualstudio.com/updates/v1_97
For reasons, that's the version that has the experimental expandable hover setting:

I will make a vscode PR to re-enable this on the editor side, but that would require testing this feature with a local build of vscode, which is far less convenient.

[typescript-bot]

Thanks for the PR! It looks like you've changed the TSServer protocol in some way. Please ensure that any changes here don't break consumers of the current TSServer API. For some extra review, we'll ping @sheetalkamat, @mjbvz, @zkat, and @joj for you. Feel free to loop in other consumers/maintainers if necessary.

[typescript-bot]

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

[Copilot]

Pull Request Overview

This PR implements an "Expandable hover" feature by adding a verbosity level to quick info responses so that increasingly detailed type information can be displayed on hover. Key changes include updates to type‐display and signature functions to accept verbosity level and output context, overloads and new properties in quick info interfaces, and corresponding adjustments in the language service, protocol, and fourslash harness test infrastructure.

Reviewed Changes

Copilot reviewed 65 out of 68 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/services/utilities.ts	Updated typeToDisplayParts and signatureToDisplayParts to pass verbosity level and writer context.
src/services/types.ts	Added an overload and a canIncreaseVerbosityLevel property to QuickInfo.
src/services/symbolDisplay.ts	Propagated verbosity level through symbol display computation and introduced unfolding helpers.
src/services/services.ts	Modified getQuickInfoAtPosition to forward verbosity level to type printing functions.
src/server/session.ts & src/server/protocol.ts	Updated quick info request/response signatures to include verbosity level and expanded type printing.
src/harness/*	Adjusted fourslash baseline and client code to allow controlling verbosity levels in tests.
src/compiler/types.ts	Extended internal type writer and symbol declaration functions with verbosity level support.

Files not reviewed (3)

• tests/baselines/reference/quickinfoVerbosity1.baseline: Language not supported
• tests/baselines/reference/quickinfoVerbosityConditionalType.baseline: Language not supported
• tests/baselines/reference/quickinfoVerbosityIntersection1.baseline: Language not supported

Comments suppressed due to low confidence (1)

src/services/symbolDisplay.ts:282

• [nitpick] Consider renaming 'typeWriterOut' to something more descriptive of its role (e.g. 'writerContextOut') to improve clarity about its purpose in managing verbosity state.

const typeWriterOut: WriterContextOut = { couldUnfoldMore: false, truncated: false };

[weswigham]

Nice work on all the extra approximateLength checks throughout the node builder, too - all the js declaration emit only methods were missing it, since they never truncated. I wish there was a more elegant way to handle it, but it's the best I've come up with without basically printing the nodes as we build them.

[weswigham]

Should couldUnfoldMore not be part of the cache, then? And potentially always be calculated? (along with the truncated out parameter?)

[weswigham]

Actually, I think the current expansion depth in general needs to be part of the cache key, too, so we don't end up in an odd situation where an expanded type is reused in multiple places with incorrect expansion levels... Probably a bit difficult to craft a test to hit, though, since it has to happen within a single builder context scope, and nested scopes get new contexts with new (copied) caches, iirc.

Maybe just skipping the cache is just the best way to handle unfolds - it's probably a headache to cache correctly, and probably isn't an issue expect for some degenerately large types that would get aggressively truncated anyway. Time will tell, I suppose.

[weswigham]

The intent here to seems to be show the enum's calculated value as its' initializer, even if said initialized value isn't an important part of the type. For non-literal enums, the calculated value is possibly less important than just the initializer as-written (and probably carries less documenting power). - 1 << 2 or A | B is more documenting than 4. If you only want to show calculated values on enums where the value is part of the type, you can just get the .value of the literal type that getTypeOfSymbol(member) returns. If the desire is to show more for non-literal enums, it might be better to use the initializer node on the declaration verbatim, rather than showing a calculated result that may or may not actually affect how we interpret the type because another member was non-literal.

[gabritto]

I updated this to use the original initializer, if present. I don't think I need to be very concerned with the initializer referring to things that are out of scope, etc, since this is just for a hover, but I wasn't sure how to handle the initializer node reuse. Let me know if I need to do something more fancy than just deep cloning.

[weswigham]

You should probably produce a /* ... */; (empty statement with a synthetic leading comment) result here when IgnoreErrors isn't passed, similar to other truncation sites. I understand it's not used internally today, but API consumers (and quickfixes that use the formatter) get weird behaviors when they try to roundtrip these non-identifier identifiers, so it's best to limit them to cases where we're planning on throwing an (ignored) error anyway.

Also, shouldn't this set the truncated out param inside this function? Or is that being done at all use-sites right now, instead? I think it'd be a bit cleaner to try and keep it inside all the factories making the different truncation statements, if possible.

[gabritto]

1. I'm not sure what you meant by ignoreErrors, I couldn't find it. But IIUC the places that were already doing truncation checked if the truncation flag was disabled (which it will be for scenarios like quickfixes) as a signal to use a /* ... */ instead of a bad identifier name, so I added that.

2. The places that were already doing truncation don't all use separate functions to construct truncation nodes, so that wouldn't work unless I refactored all of those places.
   I also considered setting this inside checkTruncationLength(), which also doesn't work because we check for additional conditions before truncating.

[weswigham]

Hm. Applying this to all lib types feels like a broad brush, but I agree that I definitely don't want to see Array or Promise expand. Meanwhile something like AddressErrors might still be useful to me. Is it worth whitelisting lib types in some way or making it configurable? Eh, future improvement. This is probably fine for now.

[gabritto]

Yeah, this occurred to me. I think in the future we'd want to make this more configurable, since it's likely to vary for each person. There's a chance we might get vscode to change their API to allow a better way of choosing what to expand, and then this problem could go away.

[sandersn]

I read through the non-tests but only have some comments about naming, nothing particularly useful.