booleans not highlighted as Boolean

[ptwales]

Describe the bug

Call :Inspect on true or false

Treesitter
  - @boolean.scala links to Boolean scala

Semantic Tokens
  - @lsp.type.keyword.scala links to Statement priority: 125

I know how to link a semantic token to a different highlight group but because booleans are marked as keywords like def and for I can't change the color of booleans without changing the color of other keywords.

Expected behavior

true and false should be linked to the Boolean highlight group or at least in a different token group than other keywords so it can be configurable by users.

Operating system

Linux

Version of Metals

v1.3.1

Commit of nvim-metals

1b87e6bfa4174b5fbaee9ca7ec79d8eae8df7f18

[ckipp01]

Thanks for the issue @ptwales, although I've transferred this over to the main Metals repo. In this scenario nvim-metals isn't really doing anything here apart from giving what Metals returns to treesitter to highlight. So I believe this semantic token information regarding boolean would need to change here.

[ptwales]

Digging around in the metals source it seems like the issue comes from lsp4j not having a "boolean" SemanticTokenType.

scalameta correctly (imo) types booleans as constants, although it does then use a Kw prefix for the True and False instances.

@branch
abstract class BooleanConstant(val value: Boolean) extends Constant[Boolean]
//...
@fixed("true")
class KwTrue extends BooleanConstant(true)
//...
@fixed("true")
class KwTrue extends BooleanConstant(true)

metals doesn't have a boolean type to map that too, so it maps it to a keyword.

  case _: Token.KwTrue => getTypeId(SemanticTokenTypes.Keyword)
  case _: Token.KwFalse => getTypeId(SemanticTokenTypes.Keyword)

IDK if the metals team wants to override this or kick it upstream to lsp4j. I imagine they have some reason for including "String", "Number" and "Regexp" as token types but not "Boolean". I haven't been able to figure how they map their Booleans, maybe "Enum"?

Either way, I'd like to avoid an inherently semantic debate over what "true" and "false" ought be tagged as. If metals doesn't want to do anything about this then I suggest punting back to nvim-metals so a user-level workaround could be documented. ie messing with priority and a after/syntax file.

[tgodzik]

We can actually add additional token types on the client side and metals can then use them. So this would be perfectly ok to have neovim announce more.

[tgodzik]

So this is actually a nice feature to work on, we need to change the file here: https://github.com/scalameta/metals/blob/78ded80dcc4a8af60e3dd6751fed783560c75bb1/mtags-shared/src/main/scala/scala/meta/internal/pc/SemanticTokens.scala#L5
to have dynamic information based on the capability that is announced by the client. We can try to recognize additional token types such as bool or boolean and use that if it's available. We would fallback to keyword otherwise.

This would however need to be also added on the client side, but this should perfectly doable without breaking anything for existing clients that do not support the new token types.

Since it's outside of the LSP spec I don't think this is a bug, so I will move it to the feature requests repo. If anyone wants to take a look at it, this should be fairly nice to implement