Semantic Tokens Support

[frankplow]

This PR implements semantic tokens from LSP 3.16. This will resolve #1239.

This is still very much WIP, some features which should be added before merging are:

• Support for the delta request type. This does the job the caching used to do, but on the server rather than the client. With this, I suspect semantic tokens may be faster than the old system. Done in f0b147d
• There is a standardised set of token types and so some sensible defaults could be provided for the semantic_highlight server option. Done in 7a53e6e.
• Token modifiers are also not yet supported. I don't think these are a priority however as the standard set of token types provides a good level of coverage and supporting modifiers isn't trivial. Done in 3ea6894.

Some feedback on how/when to make semanticTokens requests would be appreciated. This is how I'm doing it currently.

[frankplow]

Servers seem to have differing behaviour when tokenTypes and/or tokenModifiers are undefined - some interpret this as allowing any token types and some as an error. 4058c98 resolves the error case by explicitly supporting the standard set of token types. While logic could be added to detect types configured by the user, I think adopting a plug-and-play approach with only the standard set of types may be more useful for most users.

[prabirshrestha]

Thanks for taking up the semantic tokens support. Added few comments.

[frankplow]

I think this should be ready for merging, though f0b147d needs a look.

textDocument/semanticTokens/full/delta requests have definitely made it faster but there's still room for optimisation down the road. In particular, many of the delta responses just involve moving text around, which is accounted for by textprops, and so could be ignored. Also, rather than clearing all textprops before reinstating them all, prop_find could be used to only remove a textprop immediately before replacing it.

[prabirshrestha]

Can you let me know the repository/lang server you are using to test this feature?

I'm not getting much when using clangd.

3/30/2022 5:43:24 PM:["--->",2,"clangd",{"method":"textDocument/documentHighlight","on_notification":"---funcref---","params":{"textDocument":{"uri":"file:///d:/tmp/github/dwm-win32/src/grid.c"},"position":{"character":0,"line":2}}}]
3/30/2022 5:43:24 PM:["<---",2,"clangd",{"response":{"id":2,"jsonrpc":"2.0","result":[]},"request":{"id":2,"jsonrpc":"2.0","method":"textDocument/documentHighlight","params":{"textDocument":{"uri":"file:///d:/tmp/github/dwm-win32/src/grid.c"},"position":{"character":0,"line":2}}}}]
3/30/2022 5:43:25 PM:["<---(stderr)",2,"clangd","I[17:43:24.967] <-- textDocument/documentHighlight(2)\r\nI[17:43:24.967] --> reply:textDocument/documentHighlight(2) 0 ms\r\n"]

Clangd version:

I[17:43:24.738] clangd version 13.0.1\r\nI[17:43:24.740] Features: windows\r\nI[17:43:24.740] PID: 27864\r\nI[17:43:24.740] Working directory: d:\\tmp\\github\\dwm-win32\r\nI[17:43:24.740] argv[0]: C:\\Users\\prshrest\\scoop\\apps\\llvm\\current\\bin\\clangd.exe\r\n

repository:
https://github.com/prabirshrestha/dwm-win32

[frankplow]

Strange, I'm also primarilly using clangd 13.0.1. I'm on linux so could it maybe be an OS thing?

The textDocument/documentHighlight requests are to do with the highlighting of the object you are hovering over, not the semantic highlighting. Could you send any mention of any textDocument/semanticTokens/* requests please?

[prabirshrestha]

Doesn't seem to work quite yet when using dwm-win32 on windows. If you have an open source repository that works either on win/mac/linux let me know and I can give it a try.

[
  [
    "--->",
    2,
    "clangd",
    {
      "method": "textDocument/semanticTokens/full/delta",
      "on_notification": "---funcref---",
      "params": {
        "previousResultId": 0,
        "textDocument": {
          "uri": "file:///d:/tmp/github/dwm-win32/src/mods/dwm.h"
        }
      }
    }
  ],
  [
    "<---",
    2,
    "clangd",
    {
      "response": {
        "id": 4,
        "jsonrpc": "2.0",
        "error": {
          "code": -32602,
          "message": "failed to decode textDocument/semanticTokens/full/delta request: expected string at (root).previousResultId"
        }
      },
      "request": {
        "id": 4,
        "jsonrpc": "2.0",
        "method": "textDocument/semanticTokens/full/delta",
        "params": {
          "previousResultId": 0,
          "textDocument": {
            "uri": "file:///d:/tmp/github/dwm-win32/src/mods/dwm.h"
          }
        }
      }
    }
  ],
  [
    "Skipping semantic highlight: response is invalid"
  ],
  [
    "<---(stderr)",
    2,
    "clangd",
    "I[09:51:21.323] <-- textDocument/semanticTokens/full/delta(4)\r\nE[09:51:21.323] Failed to decode textDocument/semanticTokens/full/delta request: expected string at (root).previousResultId\r\nI[09:51:21.323] --> reply:textDocument/semanticTokens/full/delta(4) 0 ms, error: -32602: failed to decode textDocument/semanticTokens/full/delta request: expected string at (root).previousResultId\r\n"
  ]
]

We also don't seem to run the highlighting when buffer get loaded hence i wasn't seeing any changes. Seems like you need to listen to User lsp_buffer_enabled event too. We can probably send an event stream such as the $/vimlsp/lsp_server_exit one.

[frankplow]

Yes, looks to me like your error is resulting from the initial semanticTokens/full request never being made.

b12fe4d uses lsp_buffer_enabled rather than FileType to dispatch this request, does this fix it?

Adding a system where delta requests are not sent unless a full request has already been processed is not as easy as it should be unfortunately - the different lifetimes of textprops and bufvars throws a bit of a spanner in the works there. It could be implemented by introducing a buffer-local autocommand for every buffer which clears the relevant bufvars, but that's a bit of a hack-job.

[prabirshrestha]

Seems like there is a merge conflict due to #1298. Can you make sure to add the priorities too and merge/rebase with latest master?

I was able to get it working with rust. At one point I did get the following error but have not been able to constantly repro it.

[
  "<---",
  1,
  "rust-analyzer",
  {
    "response": {
      "id": 9,
      "jsonrpc": "2.0",
      "error": {
        "code": -32602,
        "message": "Failed to deserialize textDocument/semanticTokens/full/delta: invalid type: integer `0`, expected a string; {\"previousResultId\":0,\"textDocument\":{\"uri\":\"file:///home/prabirshrestha/code/tmp/helloworld/src/main.rs\"}}"
      }
    },
    "request": {
      "id": 9,
      "jsonrpc": "2.0",
      "method": "textDocument/semanticTokens/full/delta",
      "params": {
        "previousResultId": 0,
        "textDocument": {
          "uri": "file:///home/prabirshrestha/code/tmp/helloworld/src/main.rs"
        }
      }
    }
  }
]

Probably some race conditions somewhere?

When it works the previousResultId is a string.

[
  "<---",
  1,
  "rust-analyzer",
  {
    "response": {
      "id": 9,
      "jsonrpc": "2.0",
      "result": {
        "edits": [
          {
            "data": [
              2,
              0,
              1,
              26,
              0
            ],
            "deleteCount": 5,
            "start": 115
          }
        ],
        "resultId": "2"
      }
    },
    "request": {
      "id": 9,
      "jsonrpc": "2.0",
      "method": "textDocument/semanticTokens/full/delta",
      "params": {
        "previousResultId": "1",
        "textDocument": {
          "uri": "file:///home/prabirshrestha/code/tmp/helloworld/src/main.rs"
        }
      }
    }
  }
]

Spec does say it needs to be a string.

/**
 * The result id of a previous response. The result Id can either point to
 * a full response or a delta response depending on what was received last.
 */
 previousResultId: string;

[afranchuk]

Hey, just wanted to chime in and say that I tried this branch out but I'm struggling to get it to work. First I couldn't find any docs indicating that g:lsp_semantic_enabled has to be set (though I vaguely recalled having read that somewhere which led me to search through the variables until I found it). But once I had that set, an error occurs:

Error detected while processing function <SNR>44_debounceTimeTimerCallback[1]..<SNR>44_filterSourceCallback[6]..<SNR>44_filterSourceCallback[6]..<SNR>44_switchMapInputSourceCallback[7]..<lambda>40[1]..<SNR>58_semantic_request[1]..<SNR>58_get_server[2]..<SNR>58_supports_delta_semantic_request:
line   10:
E715: Dictionary required

Note that the server I'm trying to use doesn't yet support deltas.

[afranchuk]

Oh and I think token modifiers (at least a handful of them) should be a higher priority. Off the top of my head, the documentation modifier is often used with comment, and default_library is often used to indicate builtins. CoC handles this by simply making additional highlight classes that prepend each modifier to the name, e.g. LspDocumentationComment.

[frankplow]

@afranchuk does 9b2f5cd fix your error? What LSP server are you using - I haven't been able to find any which support semanticTokens/full but not semanticTokens/full/delta to test.

g:lsp_semantic_enabled was documented, but yes as it's 0 by default it's worth referencing in the vim-lsp-semantic section. @prabirshrestha Bit of an aside to this PR, but I think the documentation needs changing slightly with how these options work. All the subsystem _enable() functions are only called on VimEnter, yet all the basic configuration examples in the readme and top of the vim help set configuration variables inside a lsp_buffer_enabled autocommand. It means if a user was to copy-paste an example configuration and add, for example, let g:lsp_semantic_enabled=1 or let g:lsp_diagnostic_disabled=0 after the other options from one of the examples then these won't take effect.

[afranchuk]

Hey look at that, it works! Awesome, thank you. This is an internal LSP server so it is not publicly accessible for you to test. But your blind changes did the trick :)

[afranchuk]

BTW another common highlight you might want to add by default is LspMacro -> Macro.

[afranchuk]

Another highlighting suggestion (as I'm noticing this in both C++ and Rust codebases with clangd/rust-analyzer): the default highlight links for LspVariable and LspParameter tend to color everything; it might be better for those to default to Normal (though maybe my colorscheme is to blame; if no one else is bothered by it then it's not a big deal).

[frankplow]

3ea6894 adds support for modifiers. Right now this has been done as suggested by prepending the token type with each of the names of the modifers. The fact that any subset of the modifiers can be applied leads to some quirks:

• I don't think there's any guarantee as to the ordering of the modifiers, i.e. for for one server a static function definition may become LspSemanticStaticDefinitionFunction whereas for another it may be LspSemanticDefinitionStaticFunction.
• Linked to the first point, the lack of ordering in the spec means that there's no sensible way to do modifier precedence - right now all modified highlight groups link back to the unmodified case. This makes it difficult for the user to configure the highlighting - if, say, they wanted all tokens with the Deprecated modifier to be underlined, they'd have to do this for each of the combinations have modifiers including Deprecated (of which there could be thousands).

Any ideas regarding this would be much appreciated, I can't see any easy solutions. The first point could be remedied by sorting the modifiers alphabetically, but that has ramifications for the second point.

I've had to change the system for initialising and clearing the highlight groups to do it dynamically, the consequence of which is the highlight groups have to begin with a unique prefix LspSemantic rather than Lsp now, as Lsp was shared with some other non-semantic groups.

@afranchuk the LspSemanticVariable and LspSemanticParameter highlight groups link back to the Identifier group by default, I think that makes most sense semantically.

[afranchuk]

Thanks for the modifier support; with it I was able to get to parity with the prior CoC support in my extension.

[frankplow]

b3535bf sorts the modifiers alphabetically to eliminate one of the two issues outlined above.

It's still difficult to change the highlighting of a token based on its modifiers. I think the best solution to this would be to provide the user with some kind of custom highlight function with a weight argument to manage modifer precedence. This could then be taken into account when the highlight groups are first created.

I think for the time being though it's best to just merge this PR as-is, provided there are no issues.

[prabirshrestha]

@frankplow I will be using this branch for the next few days and hopefully by end of week will be able to merge. Appreciate the effort you have put into this. Thanks!

[prabirshrestha]

Merged. Thanks for the large contribution as well as your patience.

[prabirshrestha]

I’m the next few weeks I’m more than happy to make it enabled by defaults if there aren’t issues or bug reports.

[frankplow]

README.md should probably be updated, neglected that sorry