[red-knot] Add basic on-hover to playground and LSP #17057
Conversation
|
a950f23 to
debd0c8
Compare
ca67635 to
ce5a01f
Compare
b9b659c to
68d506e
Compare
|
I'm not sure if we need to do anything here as I think it's the client that will do the rendering, the server just need to make sure to use the Edit: I think I know what you mean - it's more related to how we generate the markdown content similar to how currently we're using horizontal line to split between signature and (in the future) documentation? |
| ); | ||
|
|
||
| // TODO: Add documentation of the symbol (not the type's definition). | ||
| // TODO: Render the symbol's signature instead of just its type. |
There was a problem hiding this comment.
I like what Pyright is doing here as it's format is (<node type>) <name>: type or (<node type>) <signature>. I also like what rust-analyzer does which provides a hierarchy of where the symbol is coming from:
<import location>
<impl block>
<signature>
---
<documentation>
These are just something that I've noticed and there are no action items here except to take inspiration and iterate on the format of the hover content.
| impl<'db> IntoIterator for Hover<'db> { | ||
| type Item = HoverContent<'db>; | ||
| type IntoIter = std::vec::IntoIter<Self::Item>; | ||
|
|
||
| fn into_iter(self) -> Self::IntoIter { | ||
| self.contents.into_iter() | ||
| } | ||
| } | ||
|
|
||
| impl<'a, 'db> IntoIterator for &'a Hover<'db> { | ||
| type Item = &'a HoverContent<'db>; | ||
| type IntoIter = std::slice::Iter<'a, HoverContent<'db>>; | ||
|
|
||
| fn into_iter(self) -> Self::IntoIter { | ||
| self.iter() | ||
| } | ||
| } |
There was a problem hiding this comment.
These two looks the same except for only the IntoIter type, what's the reason for having these two implementations?
There was a problem hiding this comment.
The difference is that one allows iterating when you have self and the other when you only have a &self
| match self.content { | ||
| HoverContent::Type(ty) => self | ||
| .kind | ||
| .fenced_code_block(ty.display(self.db.upcast()), "text") |
There was a problem hiding this comment.
Should we use "python" as the code block language here?
There was a problem hiding this comment.
I first used python but the notation used by our type rendering isn't valid Python. For example, we have some types of the form <some text here>. This resulted in text here to be highlighted in red because it isn't valid python.
I think the long term solution here is to implement a type renderer that more closely matches valid Python syntax (or even renders a full signature).
| let token = parsed.tokens().at_offset(offset).find(|token| { | ||
| matches!( | ||
| token.kind(), | ||
| let token = parsed | ||
| .tokens() | ||
| .at_offset(offset) | ||
| .max_by_key(|token| match token.kind() { |
There was a problem hiding this comment.
Is this change to get the last matched token instead of the first matched token?
There was a problem hiding this comment.
The change is to get the token with the highest priority and it was necessary to support hovering over the operator in a binary expression. Before, the logic filtered out any non Name or literal token, meaning that hovering + showed no type.
The logic now ranks the tokens, preferring names and literals. The "preferring" is important for if you have call<CURSOR>(; in this case, we want to get the call name and not ( (or .<CURSOR>arg) because it's more likely what the user wanted to hover / goto.
There was a problem hiding this comment.
The change is to get the token with the highest priority and it was necessary to support hovering over the operator in a binary expression. Before, the logic filtered out any non
Nameor literal token, meaning that hovering+showed no type.
Interesting, so I think this would affect goto implementations as well. For example, in an addition expression like a + b, would we go to the type of a or b? Regardless, it's not a major issue.
* main: [red-knot] Empty tuple is always-falsy (#17213) Run fuzzer with `--preview` (#17210) Bump 0.11.4 (#17212) [syntax-errors] Allow `yield` in base classes and annotations (#17206) Don't skip visiting non-tuple slice in `typing.Annotated` subscripts (#17201) [red-knot] mypy_primer: do not specify Python version (#17200) [red-knot] Add `Type.definition` method (#17153) Implement `Invalid rule provided` as rule RUF102 with `--fix` (#17138) [red-knot] Add basic on-hover to playground and LSP (#17057) [red-knot] don't remove negations when simplifying constrained typevars (#17189) [minor] Fix extra semicolon for clippy (#17188) [syntax-errors] Invalid syntax in annotations (#17101) [syntax-errors] Duplicate attributes in match class pattern (#17186) [syntax-errors] Fix multiple assignment for class keyword argument (#17184) use astral-sh/cargo-dist instead (#17187) Enable overindented docs lint (#17182)
Summary
Implement a very basic hover in the playground and LSP.
It's basic, because it only shows the type on-hover. Most other LSPs also show:
class Test(a:int, b:int)(we want something like https://github.com/microsoft/pyright/blob/54f7da25f9c2b6253803602048b04fe0ccb13430/packages/pyright-internal/src/analyzer/typeEvaluator.ts#L21929-L22129)I decided to defer these features for now because it requires new semantic APIs (similar to goto definition), and investing in fancy rendering only makes sense once we have the relevant data.
Closes #16826
Test Plan
Screen.Recording.2025-04-03.at.11.28.59.mov
Screen.Recording.2025-04-03.at.11.43.50.mov