microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.75k stars 12.46k forks source link

Support 'go to def' and interacting with inlay hint components #47693

Closed mjbvz closed 10 months ago

mjbvz commented 2 years ago

TypeScript Version: 4.6

Search Terms


VS Code recently updated our inlay hints to support tagging ranges within each hint with a location (file + position). This lets us perform operations such as go to definition on individual symbols within an inlay hint

You can see the VS Code side of this API here: https://github.com/microsoft/vscode/blob/516bef8deca4a3dc8544de536eb16d5e88ad8a83/src/vscode-dts/vscode.proposed.inlayHints.d.ts#L40

Proposal

I propose that TS also starts returning symbol locations in its inlay hint response. This would let users hover and click on parts of an inlay hint to interact with it

Here's what the protocol change would look like:

interface InlayHintItem {
     // Today this is just a string
     text: string | readonly InlayHintDisplayPart[]:

     ....
}

interface InlayHintDisplayPart {
   // Displayed text in the ui
   text: string;

    // Definition of the symbol. 
    // TODO: Does this need to be an array or do we always have a single primary location we can use?
    span?: FileSpan;
}

As an example, consider the TS:

class A { }
class B { }

declare const Foo: A | [B];

const a = Foo;

With variable type inlay hints enabled, we currently see:

Screen Shot 2022-02-01 at 6 02 10 PM

With the new proposal, we would want to return unique InlayHintDisplayPart elements for A and B. The response would look something like:

[Trace  - 02:03:33.592] <semantic> Response received: provideInlayHints (3969). Request took 2 ms. Success: true 
Result: [
    {
        "text": [
            {
                "text": ": "
            }, {
                "text": "A",
                "span": {
                    "file": "main.ts",
                    "start": { line: 1, column: 1 }, // made up location
                    "end": { line: 1, column: 99 },
                }
            }, {
                "text": " | ["
            }, {
                "text": "B",
                "span": {
                    "file": "main.ts",
                    "start": { line: 2, column: 1 },
                    "end": { line: 2, column: 99 },
                }
            }, {
                "text": "]"
            },
        ],
        "position": {
            "line": 6,
            "offset": 8
        },
        "kind": "Type",
        "whitespaceBefore": true
    }
]

/cc @jrieken

mjbvz commented 2 years ago

Adding some notes from our previous discussion

How expensive would this be to compute?

On the TS side, it sounds like we should already have the symbol locations when we compute the inlay hint text

The other things is that inlay hints are only returned for the current viewport, so even in a huge file we should not be requesting thousands of these all at once

Could this apply to other symbols displayed in the UI?

Potentially. Both hovers and error text are good candidates for this

However we are not sure if it makes sense to have a unique type for each use case or if we should try having a single SymbolDisplayPartWithSpan type that covers all of these use cases

Does this need to be opt-in?

Yes, since we have already shipped inlay hints support in TS, clients would need to opt-in to start receiving InlayHintDisplayPart instead of string.

mjbvz commented 2 years ago

@jrieken Once question I ran into while sketching out the TS api: do we need to support multiple locations for a given span? Here's a simple TS example:

interface Foo { 
    a: number
}

interface Foo { 
    b: string
}

declare const foo: Foo;

Go to def on the type Foo currently returns two definitions. I believe this is correct, however our current proposed api only supports a single Location for each InlayHintLabelPart.

Should we change location to be location: Location | readonly Location[] | undefined?

jrieken commented 2 years ago

Go to def on the type Foo currently returns two definitions. I believe this is correct, however our current proposed api only supports a single Location for each InlayHintLabelPart.

I initially wanted to make it N locations but it doesn't have to be because we don't use the location directly but like a "cursor location", e.g it is like hovering over the first Foo, than the language brain needs to figure out what locations to merge, same for go-to-def etc. The mental model should be: user triggered language feature with a simple position and not complete definitions of the symbol

MariaSolOs commented 1 year ago

@DanielRosenwasser this looks like the thing we saw in today's AUA.

DanielRosenwasser commented 10 months ago

Should we close this?

MariaSolOs commented 10 months ago

Should we close this?

I think we should (I might be biased)