Include both symbol span and whole definition span for tsserver definitions

[mjbvz]

TypeScript Version: 3.4.0-dev.201xxxxx

Search Terms:

• definition
• definitionandboundspan

Problem
VS Code uses typescript's definition requests to render small previews of a declaration when you hover over a symbol and hold cmd. However because TS currently only returns the symbol span with definitions, we cannot always guess the range to preview correctly (see microsoft/vscode#72017)

For example, for the code:

function foo() {
	// noop
}

foo()

the definitionAndBoundSpan result for the invocation of foo is:

[Trace  - 11:35:52 AM] Sending request: definitionAndBoundSpan (12). Response expected: yes. Current queue length: 0
Arguments: {
    "file": "untitled:^Untitled-1",
    "line": 5,
    "offset": 3
}
[Trace  - 11:35:52 AM] Response received: definitionAndBoundSpan (12). Request took 25 ms. Success: true 
Result: {
    "definitions": [
        {
            "file": "untitled:^Untitled-1",
            "start": {
                "line": 1,
                "offset": 10
            },
            "end": {
                "line": 1,
                "offset": 13
            }
        }
    ],
    "textSpan": {
        "start": {
            "line": 5,
            "offset": 1
        },
        "end": {
            "line": 5,
            "offset": 4
        }
    }
}

To make our UI more reliable, we need both the symbol span (covering just foo in the line function foo() {) as well as entire declaration span (covering all of the function declaration)

Proposed api
Keep the existing behavior of definitionAndBoundSpan, but tack on additional definition start/end information to each returned span:

    interface DefinitionInfoAndBoundSpan {
        definitions: ReadonlyArray<FileSpan & { definitionStart?: Location, definitionEnd?: Location }>;
        textSpan: TextSpan;
    }

This should be a backwards compatible change

Related Issues:

• DefinitionResponse selects full type range #8808

[DanielRosenwasser]

Which one is which in this API? Does the original FileSpan stay the same?

[mjbvz]

Yes, the existing FileSpan data would remain unchanced and track the symbol range. The added definitionStart and definitionEnd properties would track the range of the whole definition

[sheetalkamat]

@mjbvz Does this mean we need to add this to everything related to defintions.
Eg.

Definition = "definition",
        /* @internal */
        DefinitionFull = "definition-full",
        DefinitionAndBoundSpan = "definitionAndBoundSpan",
        /* @internal */
        DefinitionAndBoundSpanFull = "definitionAndBoundSpan-full",
        Implementation = "implementation",
        /* @internal */
        ImplementationFull = "implementation-full",
       TypeDefinition = "typeDefinition",
        References = "references",
        /* @internal */
        ReferencesFull = "references-full",

[mjbvz]

Ideally yes. I believe that VS Code can use this information in our UI for implementations and type definitions as well.

The definition one is the most important to us since it is the most noticeable in the UI

[sheetalkamat]

@mjbvz Few more question.
Do we want this only for funciton or any declaration name?
Do we want to call this contextSpan instead of definitionSpan?
Eg. for import { x as B } from "a" I think the whole declaration makes sense rather than sending you span for complete import declaration rather than just the span for x as B.

[mjbvz]

1. Yes, we would like this for all declarations including: variables, classes, and interfaces.
2. I agree that definitionSpan may not be the clearest name. I find contextSpan is a little vague too but would be fine with that. Maybe declarationSpan?

Your proposal about the handling of the import also makes sense to me