Semantic Highlighting of class members

[Fingolfin1196]

When using clangd semantic highlighting in VS Code, member variables and member functions are coloured the same way. As an example, consider the following code snippet:

struct Test {
  int integer;

  int function(int other) {
    int local;
    return other;
  }
};

Here integer and function are formatted the same, irrespective of the colour theme used.
After some digging and experimenting with a custom theme, it has emerged that both types of members have different SymbolKind values as their kind in the reply of the textDocument/documentSymbol operation of the LSP protocol (8 vs. 6 i.e. Field vs. Method), which can be seen in the log attached below. This does not appear to be the issue.

However, both kinds seem to be mapped to the same semantic token type in VS Code, that is member (see tokenClassificationRegistry.ts).
While this may sound sensible, member is rather clearly meant for functions, given its mapping to the TextMate scope entity.name.function.member, i.e. the styling of member variables in themes may be closer to functions than to variables.

As a solution, I propose keeping the mapping from the SymbolKind Method to the semantic token type member, as this seems to be intended, but to map Field to the semantic token type property, if that is at all possible.
I am not an expert on this issue at all and have found all of that out through a few hours of research; as such I have not been able to find a suitable place to fix this.
If pointed sufficiently in the right direction, I would be willing to try and fix this issue.

Logs

<-- textDocument/documentSymbol(1)
ASTWorker running documentSymbols on version 1 of […]/test.cpp
<<< {"id":2,"jsonrpc":"2.0","method":"textDocument/codeAction","params":{"context":{"diagnostics":[]},"range":{"end":{"character":13,"line":4},"start":{"character":13,"line":4}},"textDocument":{"uri":"file://[…]/test.cpp"}}}

<-- textDocument/codeAction(2)
--> reply:textDocument/documentSymbol(1) 0 ms
<<< {"id":3,"jsonrpc":"2.0","method":"textDocument/documentLink","params":{"textDocument":{"uri":"file://[…]/test.cpp"}}}

<-- textDocument/documentLink(3)
>>> {"id":1,"jsonrpc":"2.0","result":[{"children":[{"kind":8,"name":"integer","range":{"end":{"character":13,"line":1},"start":{"character":2,"line":1}},"selectionRange":{"end":{"character":13,"line":1},"start":{"character":6,"line":1}}},{"kind":6,"name":"function","range":{"end":{"character":3,"line":6},"start":{"character":2,"line":3}},"selectionRange":{"end":{"character":14,"line":3},"start":{"character":6,"line":3}}}],"kind":23,"name":"Test","range":{"end":{"character":1,"line":7},"start":{"character":0,"line":0}},"selectionRange":{"end":{"character":11,"line":0},"start":{"character":7,"line":0}}}]}

System information
Clangd version (from the log, or clangd --version): 11.0.0
clangd extension version: 0.1.7
Operating system: Manjaro Linux

[DrasLorus]

Same problem here and same system informations

System
Clangd version: 11.0.0
clangd extension version: 0.1.7
Operating system: Manjaro Linux
VS Codium version: 1.51.1

[sam-mccall]

While this may sound sensible, member is rather clearly meant for functions, given its mapping to the TextMate scope entity.name.function.member, i.e. the styling of member variables in themes may be closer to functions than to variables.

Thanks! This wasn't obvious to me, I implemented the protocol from the LSP spec and the TM scope mapping wasn't mentioned there. I agree with your fix.

The relevant code is here: https://github.com/llvm/llvm-project/blob/c964f308141578f24932c68a03af5fae7f876011/clang-tools-extra/clangd/SemanticHighlighting.cpp#L564, I'll fix it shortly.

[sam-mccall]

I've also sent microsoft/language-server-protocol#1146 to clarify the LSP spec here.

[sam-mccall]

(This is a bug in clangd rather than the vscode extension, and so it won't be released until clangd 12 I'm afraid)

[Fingolfin1196]

I did not expect this mapping to happen on the clangd side — thanks a lot for the quick fix! Now that I know where to look, I might propose some other changes (now potentially with pull requests) in the future over there, so thank you for pointing me to the right place in the implementation, too!