Document Symbols Request, clarify location of Symbol.

[bruno-medeiros]

Suppose you have this code:

class Foo {
  // ...
} 

Then for the Document Symbols Request what is the location range for the symbol Foo?

• Is it just Foo (length of 3)? (that being just the range of the identifier)
• Or is it the range of the whole element? (the 3 lines from class Foo { ... to }.

This has implications for: #112

[ljw1004]

Here are some more examples to consider...

Q1. If range includes the entire declaration, does it include the doc-block before? In other words, does it start at the beginning of line 10 and end at the end of line 60? Or should it merely start at the start of line 40?

10: /// <summary>
20: /// This is a doc-block for the following class
30: /// </summary>
40: class Foo {
50:   ...
60: }

Q2. If range includes the entire declaration, what does it do about trailing trivia? In the following, does it end after the } on line 90 or after the comment?

70: class Bar {
80:   ...
90: } // comment

Q3. If range includes the entire declaration, where is the end-point in a language that doesn't use closing delimiters? Would the range of Baz go up to the number 42 on line 120? or up to the end of comment on line 120? or up to the end of discussion on line 130? or up to the end of discourse on line 150?

100: define class Baz
110:    method Fred = 15
120:    method Jones = 42   // comment
130:    // discussion
140:
150: // discourse

[dbaeumer]

Actually I am leaning towards not to strictly spec this, however we can give recommendations. The LSP is meant to help with integrating smartness into a tool. The idea of the LSP is not do support program analysis for which this has to be spec clearly.

[ljw1004]

@dbaeumer I agree with not giving a strict spec. I hope the recommendations will be enough to give guidance to LSP-server-implementors and LSP-client-implementors on the following areas of smartness:

1. Can you reconstruct an Outline View (as used in VS, IntelliJ, Eclipse, Nuclide) out of the results of documentSymbols? If Symbol covers the entire range, then you can do so unambiguously.

2. The editor would like to highlight the symbol in its OutlineView which contains the current caret location. If Symbol covers the entire range, then you can do that.

3. The editor would like to do a "preview peek" of the line code at the symbol's location. But how many and which lines should be preview-peeked? ... If the range is the entire span, then the editor can use that as a guide about how many lines to preview peek. If the range includes docblock above the method, then very likely the editor would not want to truncate to just a few lines without even showing the symbol. If the range is the solely the span of the symbol itself, then the editor is assured that previewing just the one single line that includes the range is a good idea.

[rcjsuen]

I too would like this to be clarified.

I had assumed that the range of the symbol was for the string (so that clients could select the symbol's name when you selected the symbol in an 'Outline' view) but it seems that I was wrong and the range should be for the entire scope of the symbol...?

[eamodio]

As I commented here: microsoft/vscode#11587 (comment) -- I think it would be great for the LSP to define a range for both usages. You wouldn't have to completely spec what exactly each encompasses but just having them gives a better idea of usage and expectation.

[mickaelistria]

The current state with both ranges (this issue) and containerName (#112) not being strictly enough specified makes unclear how to properly support a tree-based navigation of the code (outline).
Both are too weak in term of specification to be fully reliable, the state of building an outline with LSP isn't that good and clients have to deal with multiple variations of LS understanding differently how to provide data for good outline.
I believe the LSP should take a decision here, by encouraging usage of a specific data to build an outline.

I would suggest that:

• we clarify the range is the whole node. This can be used for many things: outline, direct edition (deletion, move after/before...) in a tree-based or list-based UI, resolution of the other way round: from a selected text, highlight the node...
• we introduce an optional "caret: Position" that tool can use to place the caret at a given position inside the range. For example in case of a 1500 lines class, it would place the caret just before the class name.
• we drop the container name which would be redundant with the range.

cc @JanKoehnlein

[ljw1004]

I discovered another problem with containerName in the ocaml-language-server. cc @arxanas @freebroccolo

Consider the following ocaml:

module Env = struct
  type t = {
    debug: bool;
  }
end

The ocaml-language-server reports three symbols:

• Symbol name "Env", whose range spans the from the start of the word module to the end of the word end, with no containerName.
• Symbol name "t", whose range spans from the start of the word type to the end of }, with containerName Env
• Symbol name "debug", whose range spans from the start of the word debug to the end of ;, with containerName Env.t

Here the containerName Env.t is a name which is good for human-readability inside the VSCode symbol search dialog, even though it's a symbol which doesn't appear in the list of document symbols. Therefore any name-based lookup algorithm will fail on it; it requires the client to (1) use a range-based lookup, (2) trust that for all languages, ranges include the entire contents.

PROPOSAL

I think we should add an optional "containerRange" field to each element that already has a "containerName". It will unambiguously identify which documentSymbol is the parent.

I think this will be the easiest incremental proposal to implement. It doesn't require anyone to define what precisely is meant by "range". And I'm certain that every LSP server which currently emits a "containerName" will already know how to print the range it associates with that container symbol.

[mickaelistria]

t doesn't require anyone to define what precisely is meant by "range".

Then it doesn't really solve the initial issue ;)

[dbaeumer]

IMO we should do the following:

• spec the purpose of the request which is to get a flat list of document symbols. Neither the container name nor the positions should be used to re-infer a hierarchy. The container name's purpose was to render it in the UI as a qualification.

• have a new request that returns the document symbols in a hierarchy. Here again the range should be speced from a tools side (reveal, highlight, cursor position). I would not enforce in the spec that a parent node's range contains all child node ranges as it would be the case with an AST.

In the last milestone I added means to propose new protocol more easily. It is described here: https://github.com/Microsoft/language-server-protocol/blob/master/contributing.md

So may be someone wants to provide this as a PR.

[dbaeumer]

Here is an example about what I mean:

export interface SymbolInformation {
	/**
	 * The name of this symbol.
	 */
	name: string;

	/**
	 * The kind of this symbol.
	 */
	kind: SymbolKind;

	/**
	 * The location of this symbol. The location's range is used by a tool
	 * to reveal the location in the editor. If the symbol is selected in the
	 * tool the range's start information is used to position the cursor. So
	 * the range usually spwans more then the actual symbol's name and does
	 * normally include thinks like visibility modifiers.
	 *
	 * The range doesn't have to denote a node range in the sense of a abstract
	 * syntax tree. It can therefore not be used to re-construct a hierarchy of
	 * the symbols.
	 */
	location: Location;

	/**
	 * The name of the symbol containing this symbol. This information is for
	 * user interface purposes (e.g. to render a qaulifier in the user interface
	 * if necessary). It can't be used to re-infer a hierarchy for the document
	 * symbols.
	 */
	containerName?: string;
}

[dbaeumer]

I clarified the indented behavior in the spec and the implementation.