Add visibility to SymbolInformation

[felixfbecker]

I propose an addition to SymbolInformation that allows an LS to report the visibility (private, protected, public) to the client:

interface SymbolInformation {
  visibility?: Visibility;
}
enum Visibility {
  Private = 1,
  Protected = 2,
  Public = 3
}

A client can choose how he uses that information:

• it could display an icon that indicates a symbol is private
• after getting a list of symbols, it could allow the user to hide/show only public symbols

The same property could get added to CompletionItem.

I think private, protected and public should cover all languages, or are there more modifiers in some languages?

The enum should be structured to allow something like

if (symbol.visibility >= Visibility.Protected) {
  // symbol is at least protected or public
}

[kaloyan-raev]

I think private, protected and public should cover all languages, or are there more modifiers in some languages?

In Java there is package visibility, when there is no access modifier used.

[felixfbecker]

@kaloyan-raev what do you think would be best to represent package visibility in the proposal?

[kaloyan-raev]

Like this:

enum Visibility {
  Private = 1,
  Package = 2,
  Protected = 3,
  Public = 4
}

[dpwiz]

Better put "public" first, as it's the base case - other cases are kinds of restricted.

[felixfbecker]

Why does this have the 4.0 milestone? This is not a breaking change

[felixfbecker]

@wiz public is the highest visibility, so it should be the highest number.

[dpwiz]

So, the visibility set is closed as no further levels can be added below "Private" and above "Public"? That would be unfortunate for non-Javy languages...

[felixfbecker]

I'm confused, below private and after public is exactly where you could still add further levels. And you can add more levels in major protocol versions. We can also decide to leave more slots open in between. But if you have more visibilities in mind, the time to propose them is now :)

[dpwiz]

I'm confused, below private and after public is exactly where you could still add further levels.

That would ruin the pretty ordering we have here. Adding Whatnot = 5 would break that "public is the highest visibility" rule. Adding Something = -10 would add confusion to enum-styled constants.

But if you have more visibilities in mind, the time to propose them is now :)

I propose leaving some wiggle room for the future additions while keeping confusion to the minimum.

[alexet]

C# has 5 levels of accessibility: https://msdn.microsoft.com/en-us/library/wxh6fsc7.aspx, also in this case the accessibility lattice is not a sub lattice of the integer lattice so you can't do simple integer comparisons to check accessibility.

[ljw1004]

C# either has or is going to get a 6th level of accessibility, 'proternal', accessible only to things that derive from the type AND are in the same assembly. (compare with 'protected internal' which is accessible to things that either derive from the type OR are in the same assembly.)

[ljw1004]

One concern I have is to avoid clutter. Imagine you're writing in a language where almost everything is private, apart from the few methods that are explicitly annotated with the modifier export. What would an Outline view look like then? Would it include a 'private' glyph next to every single entry? Or would it leave the entries unadorned by default, and only stick a 'public glyph next to the ones that have the export modifier?

[felixfbecker]

Up to the client to decide. It could grey it out if it's private, it could show an icon, it could just have checkboxes to filter like "only public". The protocol shouldn't make any UI decisions.

[ljw1004]

My point is, it probably shouldn't be up to the client to decide. Only the language knows what is the default (and hence which one should be shown with unadorned icons).

For instance: in Flow, if you write function foo() then it is private. If you adorn it with a keyword export function foo() then it it is public. I therefore expect my editor to show things unadorned by default, and only adorn them (with a glyph for public) when the developer has adorned their code with the export keyword.

In C# the accessibility of things is confusing and there are lots of common options, so I expect every single declaration in C# to have a glyph indicating accessibility.

[mickaelistria]

I think @felixfbecker says it's up to the client to decide how to render once we know the visibility, and @ljw1004 says is that the default visibility is not desired. Both are fully compatible.
@ljw1004 In the example you give, then the foo symbol should have a private visibility flag when returned from LS. In general, if we go for it, the spec should mention that setting visibility is highly recommended and not allow a "default" visibility that doesn't mean much to the client as "default" is a language-specific concept.

[vladdu]

I'm with @ljw1004 here. How would a generic client know that for language X, it should not decorate private modifiers, while for language Y it is the public ones? Only the LS knows.

I agree that the symbol information should contain all available data. I believe that the problem arises from conflating the symbol information with selecting a glyph - the "icon" value can't be computed reliably from the SymbolInformation, so an optional field should be provided by the server when needed.

The "icon" set can be more open than the "kind" one, so maybe this will address even #129.

[KamasamaK]

This could be addressed more generically with #889. The discussion in microsoft/vscode#23927 indicates any modifier, including visibility/access, could be accommodated.

[mitonize]

As discussed and concluded at microsoft/vscode#23927, a namespace SymbolTag was merged.
Though SymbolTag has ability to indicate modifiers, I don't know how to add language specific definitions.

https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/

[travkin79]

It seems the LSP spec proposes a SymbolTag for DocumentSymbol details like deprecation, visibility and more. Shouldn't the spec contain more SymbolTag values than just deprecated? I thought of values like visibility tags common for most languages (private, package-private, protected, public,...) and maybe even more pre-defined tags (e.g. synchronized, volatile, transient).

My original request comes from a requirement on LSP4E, CDT LSP, and clangd discussed here.

[travkin79]

I've prepared a proposal for additional SymbolTag values in PR #2003. Could someone please do a review?