-
Notifications
You must be signed in to change notification settings - Fork 883
Description
I was reading through the code for Hover and noticed three oddities...
Missing newline
class Post:
"""Docstring-for-Post"""
def foo(item: type[Post]) -> None:
"""Args:
item: Docstring-for-item"""
...
Hover on the parameter "item" shows ---
but I think it should instead have been split into paragraphs

This is the LSP response:
{"jsonrpc":"2.0","id":2,"result":{"contents":{"kind":"markdown","value":"```python\n(parameter) item: type[Post]\n```\nitem: the item to bar---\nThis is what a Post is"},"range":{"start":{"line":21,"character":8},"end":{"line":21,"character":12}}}}
Should be union instead of concatenate
class D1(TypedDict):
age: int
"""doc-for-D1.age"""
class D2(TypedDict):
age: str
"""doc-for-D2.age"""
def bar(item: D1 | D2) -> None:
x = item["age"]
print(x)
reveal_type(item["age"])
bar({"age": 1})
Hover on the "age" argument in the callsite to bar. It shows the hover of D1.age, followed by the hover of D2.age. But I reckon it should instead be merging them (key) age: int | str
. That would be consistent with the reveal_type in the above code.

(The existing unit test hover.typedDict.key.fourslash.ts tests for the existing behavior; I just reckon the existing behavior feels odd...)
Should have hover for TypedDict member access
In the above example, in the line x = item["age"]
, hover over the string "age". I expect that this should show a hover tooltip, similar to how it does at the callsite to bar. But it doesn't show anything.