Show deprecated message in hover
When hovering over a propery or method that is labeled with the @Deprecated('MESSAGE'); annotation.
The message is not shown in the hoverbox.
The message should be visible.
Current:
What it should be:
@DanTup This maybe could be extented to include all text for all annotation's that have text ?
I'm not sure if it makes sense to add all of them. The deprecation message I can see as useful, but if we include the text from others we'd also need to include the annotation (so it's clear what the text relates to). Then it might seem odd if we don't show annotations without text, and if we do that, the hover would be much bigger (and may just be full of things like @override or @visibleForTesting that I don't think are that useful).
That said, if you hold down Ctrl when you hover over a symbol, VS Code shows a preview of code for that symbol. Currently it does not include annotations, but I wonder if we should set the range such that it does 🤔
I wonder if we should set the range such that it does
I'd say this is a great idea and would work most of the time. We should just be aware of cases like (this test example):
abstract class A {
int get foo;
}
class B extends A {
@deprecated
@override
@deprecated
int foo = 0;
}
There is also nothing that prevents the user from adding the annotation above Dart docs so this may be a case to be aware of:
class MyClass {
@Deprecated('Message here')
/// This is a deprecated method.
///
/// This does nothing useful.
void deprecated() {}
}
final _ = MyClass().deprecated;
I took a quick look at this and realised that generally we do show the deprecation messages in the hovers, because they are usually diagnostics when you reference an item:
The place where we don't show it is when you hover the declaration itself:
I think it would be nice to show here, but since it's usually not far from where you've hovering, it's probably less important. It will require some refactoring of hovers in the server because currently LSP uses the legacy protocol classes and they contain only a boolean for isDeprecated and not the message.
(I'm also not sure the current behaviour of putting "(deprecated)" inside a code block makes sense, so it might make sense to move this to its own line outside of the block if/when we do this.
@DanTup When will the LSP legacy protocol be updated/refactored. Or is the something that is not on the current planning?