Semantic highlighting doesn't work on language servers containing non-alphanumeric characters in token types

[JManch]

Neovim version (nvim -v)

NVIM v0.9.0-dev-496+g1c324cb19 commit 1c324cb

Language server name/version

omnisharp-roslyn v1.39.2 and any other language server with non-alphanumeric tokens

Operating system/version

Windows 11

Steps to reproduce using "nvim -u minimal_init.lua"

Start the omnisharp-rosyln language server and see the errors described below

Expected behavior

Semantic highlighting works

Actual behavior

Semantic highlighting doesn't work and produces the following error:

Error executing vim.schedule lua callback: ...ghtly\share\nvim\runtime/lua\vim\lsp\semantic_tokens.lua:342: Vim(redraw):E5248: Invalid character in group name
stack traceback:
	[C]: in function 'nvim_command'
	...ghtly\share\nvim\runtime/lua\vim\lsp\semantic_tokens.lua:342: in function 'process_response'
	...ghtly\share\nvim\runtime/lua\vim\lsp\semantic_tokens.lua:269: in function 'handler'
	...rograms\NeovimNightly\share\nvim\runtime/lua\vim\lsp.lua:1391: in function 'cb'
	vim/_editor.lua:250: in function <vim/_editor.lua:249>

This is the contents of the client.server_capabilities.semanticTokensProvider table where you can see the non-alphanumeric token types:

client.server_capabilities.semanticTokensProvider = {
   full = vim.empty_dict(),
   legend = {
      tokenModifiers = { "static symbol" },
      tokenTypes = {
         "comment",
         "excluded code",
         "identifier",
         "keyword",
         "keyword - control",
         "number",
         "operator",
         "operator - overloaded",
         "preprocessor keyword",
         "string",
         "whitespace",
         "text",
         "static symbol",
         "preprocessor text",
         "punctuation",
         "string - verbatim",
         "string - escape character",
         "class name",
         "delegate name",
         "enum name",
         "interface name",
         "module name",
         "struct name",
         "type parameter name",
         "field name",
         "enum member name",
         "constant name",
         "local name",
         "parameter name",
         "method name",
         "extension method name",
         "property name",
         "event name",
         "namespace name",
         "label name",
         "xml doc comment - attribute name",
         "xml doc comment - attribute quotes",
         "xml doc comment - attribute value",
         "xml doc comment - cdata section",
         "xml doc comment - comment",
         "xml doc comment - delimiter",
         "xml doc comment - entity reference",
         "xml doc comment - name",
         "xml doc comment - processing instruction",
         "xml doc comment - text",
         "xml literal - attribute name",
         "xml literal - attribute quotes",
         "xml literal - attribute value",
         "xml literal - cdata section",
         "xml literal - comment",
         "xml literal - delimiter",
         "xml literal - embedded expression",
         "xml literal - entity reference",
         "xml literal - name",
         "xml literal - processing instruction",
         "xml literal - text",
         "regex - comment",
         "regex - character class",
         "regex - anchor",
         "regex - quantifier",
         "regex - grouping",
         "regex - alternation",
         "regex - text",
         "regex - self escaped character",
         "regex - other escape",
      },
   },
   range = true,
}

Log file

No response

[clason]

That's some very illegal naming... That can't work with the current implementation at all. Best advice is to disable tokens for this server.

[theHamsta]

A workaround as a user could probably be to manipulate client.server_capabilities.semanticTokensProvider.legend on attach to replace the invalid characters.

[clason]

We could also sanitize or skip such invalid token highlights, but I worry about performance here @jdrouhard

[jdrouhard]

We could also sanitize or skip such invalid token highlights, but I worry about performance here @jdrouhard

If we sanitize the legend like @theHamsta suggested, we would only incur the performance cost once, at client start-up time.

This seems like a good idea regardless, we could replace any non alpha-numeric with an underscore. Easy to document, easy to implement.

However, for this particular language server, those are just flat out wrong. There should be a bug filed against it to make the legend conform to the spec.

[mfussenegger]

My interpretation of the spec is that a server isn't supposed to send token types which aren't part of the clients capabilities. Otherwise there wouldn't be any point in having the capability that lists the supported types.

I don't think we should do any kind of sanitation, but instead file bug reports to the language servers to get them fixed.

[JManch]

Thanks for the responses. I'll create an issue in the OmniSharp repo to let them know about the incorrect token naming.

[starteleport]

Just in case anyone needs quick and dirty fix, I'm sharing what I've put to my on_attach.

if client.name == "omnisharp" then
  client.server_capabilities.semanticTokensProvider.legend = {
    tokenModifiers = { "static" },
    tokenTypes = { "comment", "excluded", "identifier", "keyword", "keyword", "number", "operator", "operator", "preprocessor", "string", "whitespace", "text", "static", "preprocessor", "punctuation", "string", "string", "class", "delegate", "enum", "interface", "module", "struct", "typeParameter", "field", "enumMember", "constant", "local", "parameter", "method", "method", "property", "event", "namespace", "label", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "xml", "regexp", "regexp", "regexp", "regexp", "regexp", "regexp", "regexp", "regexp", "regexp" }
  }
end

UPD: replaced previous version because of errors on first buffer opening.