You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In a language like Python, there are scenarios where you end up with unused variables that are still very much necessary. For example, if you're implementing a method signature, you can end up defining parameters that are unused, but still must be be there in order to preserve the arity of the function. (Furthermore, you often are not even free to rename them to something obviously unused because Python supports invoking functions with named parameters.)
It's nice for IDEs to be able to give a visual indicator that these variables are unused without telling the user that there's something actionable here.
There seems to be disagreement in the ecosystem about if the Language Server Protocol is capable of expressing this. I've read everything I could on this, and my opinion is that it does not.
Some tools (Pyright, neodim, VS Code, maybe others) treat the pair (DiagnosticSeverity.Hint, DiagnosticTag.Unnecessary) as "non-actionable unused code".
Other tools (notably Neovim, I haven't checked other popular IDEs) choose to treat all diagnostics (including the (DiagnosticSeverity.Hint, DiagnosticTag.Unnecessary) pair) as actionable.
One of Neovim's maintainers sought clarity on this in #1696, and the issue was closed with this message:
I will close the issue since I really don't want to enforce UI rendering in the LSP specification.
I totally get that UI rendering is not in scope for the Language Server Protocol. But perhaps the concept of "actionability" is? Is there some way we can clarify or change the Language Server Protocol to support this?
A few proposals, in no particular order:
Update the docstring on DiagnosticTag.Unnecessary to clarify that is it non-actionable.
This would be consistent with what Pyright does: it emits this diagnostic at (DiagnosticSeverity.Hint, DiagnosticTag.Unnecessary). Relevant code here.
This would be inconsistent with what rust-analyzer does: it emits this diagnostic at (DiagnosticSeverity.Warning, DiagnosticTag.Unnecessary). Relevant code: here's the DiagnosticTag, the severity ultimately comes from cargo check, which emits this as a "warning".
Update the docstring on DiagnosticTag.Unnecessary to clarify that it is non-actionable with DiagnosticSeverity.Hint, but is actionable at other severities.
This would be consistent with both Pyright and rust-analyzer. I haven't explored other LSP servers, but would be happy to do so if there's interest in moving this forward.
Add a new DiagnosticTag.Unused (or perhaps DiagnosticTag.Unreferenced) that is clearly documented as non-actionable.
If we made this change, ideally we'd also clarify that DiagnosticTag.Unnecessary is actionable. Or we deprecate DiagnosticTag.Unnecessary and add an alternative that is clearly documented as actionable.
Afaik eclipse.jdt.ls and luals also use DiagnosticTag.Unnecessary for actionable diagnostic. There are probably others too. Seems to me that pyright is the odd duck here.
I'd also question a bit why there'd need to be a diagnostic at all if something is legitimately/intentionally unused. What'd be the point of a DiagnosticTag.Unused over just not emitting any diagnostic?
If it isn't actionable it would require lots of special handling on the client side - for what?
there are scenarios where you end up with unused variables that are still very much necessary. For example, if you're implementing a method signature, you can end up defining parameters that are unused, but still must be be there in order to preserve the arity of the function.
This sounds a bit as a more general ambiguity problem. How is a server supposed to know if something is intentionally unused or not, without some additional hint from the person who wrote the code?
E.g. some languages allow to prefix intentionally unused variables with _.
If it isn't actionable it would require lots of special handling on the client side - for what?
Currently, in neovim, unused variables get grayed out and they get this message next to them about being unused, and they show up in the diagnostics list. I think the ask here is "I like it being grayed out, but I don't like it being in the list". Given that clients already do (part of) this, I'm not sure what special handing would be required for the graying out of the variables; and regarding the list I'm sure it's easy enough to filter out that particular type? That said, I'm firmly in the camp that would appreciate if my editor let me store a setting to determine if I see these "unactionable items" in my diagnostic list (such as - maybe I expect some to not be actionable but I also want to ensure I didn't accidentally leave some dangling around. I suppose CI could catch this as well but I suppose I should stay on topic).
E.g. some languages allow to prefix intentionally unused variables with _.
As stated in the initial post, you can't do this with Python if named parameters are used:
(Furthermore, you often are not even free to rename them to something obviously unused because Python supports invoking functions with named parameters.)
In a language like Python, there are scenarios where you end up with unused variables that are still very much necessary. For example, if you're implementing a method signature, you can end up defining parameters that are unused, but still must be be there in order to preserve the arity of the function. (Furthermore, you often are not even free to rename them to something obviously unused because Python supports invoking functions with named parameters.)
It's nice for IDEs to be able to give a visual indicator that these variables are unused without telling the user that there's something actionable here.
There seems to be disagreement in the ecosystem about if the Language Server Protocol is capable of expressing this. I've read everything I could on this, and my opinion is that it does not.
(DiagnosticSeverity.Hint, DiagnosticTag.Unnecessary)
as "non-actionable unused code".(DiagnosticSeverity.Hint, DiagnosticTag.Unnecessary)
pair) as actionable.One of Neovim's maintainers sought clarity on this in #1696, and the issue was closed with this message:
I totally get that UI rendering is not in scope for the Language Server Protocol. But perhaps the concept of "actionability" is? Is there some way we can clarify or change the Language Server Protocol to support this?
A few proposals, in no particular order:
DiagnosticTag.Unnecessary
to clarify that is it non-actionable.(DiagnosticSeverity.Hint, DiagnosticTag.Unnecessary)
. Relevant code here.(DiagnosticSeverity.Warning, DiagnosticTag.Unnecessary)
. Relevant code: here's theDiagnosticTag
, the severity ultimately comes fromcargo check
, which emits this as a "warning".DiagnosticTag.Unnecessary
to clarify that it is non-actionable withDiagnosticSeverity.Hint
, but is actionable at other severities.DiagnosticTag.Unused
(or perhapsDiagnosticTag.Unreferenced
) that is clearly documented as non-actionable.DiagnosticTag.Unnecessary
is actionable. Or we deprecateDiagnosticTag.Unnecessary
and add an alternative that is clearly documented as actionable.DiagnosticTag.Unnecessary
, replace with a newDiagnosticTag.Unused
#2025 before I fully understood the issue.DiagnosticTag
entirely, instead do this with semantic tokens.DiagnosticTag
s deprecated in favor of semantic tokens? #2024 to seek clarity on this.Does this feel like something that could be in scope for the Language Server Protocol? If so, I'd be happy to help move this forward.
The text was updated successfully, but these errors were encountered: