Replace `mutable` modifier with `readonly`

[sagg0t]

ZLS 0.15 added non-standard mutable modifier for variables which is a great feature. However for better integration with various tools it's better to use the standard readonly modifier on constants. It will allow for the same results (logic is just inverted) and will be standard compatible.

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

I'd be happy to work on it if the issue gets a green light.

[Techatrix]

It was a deliberate to use mutable over readonly. The reasoning can be found in https://github.com/zigtools/zls/pull/2251. If you still think that using readonly is the better choice, you are welcome to present your argument here.

[sagg0t]

Sure. I did my best to understand your arguments in #2251 and https://github.com/ziglang/vscode-zig/issues/213 so I'll refer to your quotes from there. I tried to address the facts only, so if by any chance you feel like it addresses you personally - it's not my intention and I apologize for it.

TLDR highlighting const vars in Zig is very noisy.

The fact that it makes it look noisy only highlights the fact that the theme highlights constants too aggressively. As you said yourself, "LSP should not decide how to emit semantic token information based on how it affects various editors." And if some editor/colortheme produces too much highlight - it's a highlight problem, not a semantics problem.

With mutable modifier now it works such that constants in code get variable token type which makes zero sense. So now LSP sacrifices semantics (which is its core feature) in favor of highlight (which is beyond LSP's scope.) I understand the desire to provide the best user experience, but in this case it looks like a wrong priority over correctness or compatibility.

adding the readonly token modifier would be a bad addition for semantic highlighting in Zig.

As you already know it's a standard modifier. Various tools, not only colorschemes, can rely on standard tokens, not providing it limits options for customization/usage. I can't come up with an example of the tool off the top of my head, but I'm sure you understand the idea.

LSP is a standard for the very reason to provide generic information about language. Terms like mutable are specific to certain languages and this knowledge should remain within the boundaries of those languages. LSP should be a generalized bridge between a language (with its internals,) and an editor that works with more generic constructs.

rust-anlyzer uses mutable over readonly

For the record I'm not using Rust. In Rust, syntax implies mutability as a modifier let mut x = 5;. But event then, syntax != semantics since let mut a declares a variable a and let b declares a constant. Even though in Rust's terms they are called differently, that doesn't change what they are. that's why I think it's not the best option even for Rust, but at the very least it mirrors the syntax.

In the case of Zig, there's nothing like that. Even if we use Rust as an argument here, Zig's syntax doesn't have modifiers like that, it uses different keywords. So adopting Rust's model not only goes orthogonal to the standard, but also is a bad fit.

Zig code is usually heavy on const variables.

True, but many expression types, that use const don't produce a constant variable, they often return a type, or a namespace, etc. As for the parameters and captures (which are also parameters IMO) - there's a separate token type for them. So pretty much the only use case is constants.

[Techatrix]

To avoid any confusion, I have to clarify that my usage of the term "standard" and "non-standard" token types/modifers can be misleading. I should rather call them "non-predefined" or "custom" token type/modifier so that it can't be misunderstood to imply they are somewhat discouraged or disallowed by the LSP spec. Please correct me if I am wrong but in your response it sometimes read like you had understood custom token types/modifiers like this.

The fact that it makes it look noisy only highlights the fact that the theme highlights constants too aggressively. [...] And if some editor/colortheme produces too much highlight - it's a highlight problem, not a semantics problem.

Let say we use readonly and this problem is meant to be fixed by the editor or colorscheme. How exactly are they supposed to do that? This seems to suggest that editors/colorsschemes should either ignore readonly or highlight the absence of the readonly token modifier for a subset of languages/servers which is something I have yet to encounter. Let me know if have a better solution for this.

This is why I see a use case in both readonly and mutable. If a server wants "immutable" things to be highlighted, they use readonly. If a server wants "mutable" things to be highlighted, they use mutable. If editors treat both modifiers the same then they automatically handle this difference without needing language specific knowledge. Of course the default behaviour can be customized by users in editor that support it.

As you said yourself, "LSP should not decide how to emit semantic token information based on how it affects various editors."

I have to admit that I failed to here to express what I actually intended to say. Of course you have take into account the impact on editors when making decisions that aren't answered by the spec. Here is what I intended to say:

A language server should not decide how to emit semantic token information solely based on how it affects a subset of editors or colorschemes.

The intent was to clarify that I did not reach my decision about mutable/readonly just because of how it affected VS Code with some of the builtin themes.

With mutable modifier now it works such that constants in code get variable token type which makes zero sense.

How is this any different when using a readonly token modifier? There is no predefined constant token type nor have I ever seen a custom one being used.

So now LSP sacrifices semantics (which is its core feature) in favor of highlight (which is beyond LSP's scope.) I understand the desire to provide the best user experience, but in this case it looks like a wrong priority over correctness or compatibility.

Could you explain what you mean by sacrifice semantics over highlight and why former is a core feature and the latter is beyond scope? The entire point of LSP semantic tokens has always been to be used for code highlighting. And how exactly is not using readonly incorrect or incompatible? My guess is that this comes from the interpretation of the term "non-standard" tokens like I mentioned at the start.

And if some editor/colortheme produces too much highlight - it's a highlight problem, not a semantics problem.

As you already know it's a standard modifier. Various tools, not only colorschemes, can rely on standard tokens, not providing it limits options for customization/usage. I can't come up with an example of the tool off the top of my head, but I'm sure you understand the idea.

LSP is a standard for the very reason to provide generic information about language. Terms like mutable are specific to certain languages and this knowledge should remain within the boundaries of those languages. LSP should be a generalized bridge between a language (with its internals,) and an editor that works with more generic constructs.

In Rust, syntax implies mutability as a modifier let mut x = 5;. But event then, syntax != semantics since let mut a declares a variable a and let b declares a constant. Even though in Rust's terms they are called differently, that doesn't change what they are. that's why I think it's not the best option even for Rust, but at the very least it mirrors the syntax. In the case of Zig, there's nothing like that. Even if we use Rust as an argument here, Zig's syntax doesn't have modifiers like that, it uses different keywords. So adopting Rust's model not only goes orthogonal to the standard, but also is a bad fit.

Instead of making vague statements about what is good for colorscheme customization, what LSP is or isn't, syntax vs. semantics or highlight vs. semantics, I try to make decisions based on their real-world impact on editors and their users. If there is a benefit in having more custom tokens in ZLS than so be it. If not, then ZLS will prefer the predefines ones.

True, but many expression types, that use const don't produce a constant variable, they often return a type, or a namespace, etc. As for the parameters and captures (which are also parameters IMO) - there's a separate token type for them. So pretty much the only use case is constants.

What is the point you are trying to make here? Even ignoring declarations of type type since neither readonly nor mutable applies to them, Zig is heavy on consts compared to vars so using a token modifier that highlights the minority is the more reasonable choice. If we were to use readonly then we would be highlighting parameters, captures and consts. I do not want colorschemes that make use of readonly to have this as their default behaviour.