From 6d7fb65b36bb61b3c7e91c45fbce8e00f323d1d5 Mon Sep 17 00:00:00 2001
From: Peter Cardenas <16930781+PeterCardenas@users.noreply.github.com>
Date: Sat, 13 Jan 2024 20:43:02 -0800
Subject: [PATCH] :sparkles: better hover docs

ripped from https://github.com/python-lsp/python-lsp-server/pull/452
---
 pylsp/plugins/hover.py     |  90 ++++++++++++++++++-------
 test/plugins/test_hover.py | 134 ++++++++++++++++++++++++++++++++-----
 2 files changed, 180 insertions(+), 44 deletions(-)

diff --git a/pylsp/plugins/hover.py b/pylsp/plugins/hover.py
index ae07b3dc..850f2709 100644
--- a/pylsp/plugins/hover.py
+++ b/pylsp/plugins/hover.py
@@ -8,43 +8,81 @@
 log = logging.getLogger(__name__)
 
 
+def _find_docstring(definitions):
+    if len(definitions) != 1:
+        # Either no definitions or multiple definitions
+        # If we have multiple definitions the element can be multiple things and we
+        # do not know which one
+
+        # TODO(Review)
+        # We could also concatenate all docstrings we find in the definitions
+        # I am against this because
+        # - If just one definition has a docstring, it gives a false impression of the hover element
+        # - If multiple definitions have a docstring, the user will probably not realize
+        #   that he can scroll to see the other options
+        return ""
+
+    # The single true definition
+    definition = definitions[0]
+    docstring = definition.docstring(
+        raw=True
+    )  # raw docstring returns only doc, without signature
+    if docstring != "":
+        return docstring
+
+    # If the definition has no docstring, try to infer the type
+    types = definition.infer()
+
+    if len(types) != 1:
+        # If we have multiple types the element can be multiple things and we
+        # do not know which one
+        return ""
+
+    # Use the docstring of the single true type (possibly empty)
+    return types[0].docstring(raw=True)
+
+
+def _find_signatures(definitions, word):
+    # Get the signatures of all definitions
+    signatures = [
+        signature.to_string()
+        for definition in definitions
+        for signature in definition.get_signatures()
+        if signature.type not in ["module"]
+    ]
+
+    if len(signatures) != 0:
+        return signatures
+
+    # If we did not find a signature, infer the possible types of all definitions
+    types = [
+        t.name
+        for d in sorted(definitions, key=lambda d: d.line)
+        for t in sorted(d.infer(), key=lambda t: t.line)
+    ]
+    if len(types) == 1:
+        return [types[0]]
+    elif len(types) > 1:
+        return [f"Union[{', '.join(types)}]"]
+
+
 @hookimpl
 def pylsp_hover(config, document, position):
     code_position = _utils.position_to_jedi_linecolumn(document, position)
-    definitions = document.jedi_script(use_document_path=True).infer(**code_position)
-    word = document.word_at_position(position)
 
-    # Find first exact matching definition
-    definition = next((x for x in definitions if x.name == word), None)
-
-    # Ensure a definition is used if only one is available
-    # even if the word doesn't match. An example of this case is 'np'
-    # where 'numpy' doesn't match with 'np'. Same for NumPy ufuncs
-    if len(definitions) == 1:
-        definition = definitions[0]
-
-    if not definition:
-        return {"contents": ""}
+    # TODO(Review)
+    # We could also use Script.help here. It would not resolve keywords
+    definitions = document.jedi_script(use_document_path=True).help(**code_position)
+    word = document.word_at_position(position)
 
     hover_capabilities = config.capabilities.get("textDocument", {}).get("hover", {})
     supported_markup_kinds = hover_capabilities.get("contentFormat", ["markdown"])
     preferred_markup_kind = _utils.choose_markup_kind(supported_markup_kinds)
 
-    # Find first exact matching signature
-    signature = next(
-        (
-            x.to_string()
-            for x in definition.get_signatures()
-            if (x.name == word and x.type not in ["module"])
-        ),
-        "",
-    )
-
     return {
         "contents": _utils.format_docstring(
-            # raw docstring returns only doc, without signature
-            definition.docstring(raw=True),
+            _find_docstring(definitions),
             preferred_markup_kind,
-            signatures=[signature] if signature else None,
+            signatures=_find_signatures(definitions, word),
         )
     }
diff --git a/test/plugins/test_hover.py b/test/plugins/test_hover.py
index 7049becc..696aa1c3 100644
--- a/test/plugins/test_hover.py
+++ b/test/plugins/test_hover.py
@@ -9,10 +9,44 @@
 
 DOC_URI = uris.from_fs_path(__file__)
 DOC = """
+from random import randint
+from typing import overload
 
-def main():
-    \"\"\"hello world\"\"\"
+class A:
+    \"\"\"Docstring for class A\"\"\"
+
+    b = 42
+    \"\"\"Docstring for the class property A.b\"\"\"
+
+    def foo(self):
+        \"\"\"Docstring for A.foo\"\"\"
+        pass
+
+if randint(0, 1) == 0:
+    int_or_string_value = 10
+else:
+    int_or_string_value = "10"
+
+@overload
+def overload_function(s: int) -> int:
+    ...
+
+@overload
+def overload_function(s: str) -> str:
+    ...
+
+def overload_function(s):
+    \"\"\"Docstring of overload function\"\"\"
     pass
+
+int_value = 10
+string_value = "foo"
+instance_of_a = A()
+copy_of_class_a = A
+copy_of_property_b = A.b
+int_or_string_value
+overload_function
+
 """
 
 NUMPY_DOC = """
@@ -23,6 +57,83 @@ def main():
 """
 
 
+def _hover_result_in_doc(workspace, position):
+    doc = Document(DOC_URI, workspace, DOC)
+    return pylsp_hover(
+        doc._config, doc, {"line": position[0], "character": position[1]}
+    )["contents"]["value"]
+
+
+def test_hover_over_nothing(workspace):
+    # Over blank line
+    assert "" == _hover_result_in_doc(workspace, (3, 0))
+
+
+def test_hover_on_keyword(workspace):
+    # Over "class" in "class A:"
+    res = _hover_result_in_doc(workspace, (4, 1))
+    assert "Class definitions" in res
+
+
+def test_hover_on_variables(workspace):
+    # Over "int_value" in "int_value = 10"
+    res = _hover_result_in_doc(workspace, (31, 2))
+    assert "int" in res  # type
+
+    # Over "string_value" in "string_value = "foo""
+    res = _hover_result_in_doc(workspace, (32, 2))
+    assert "string" in res  # type
+
+
+def test_hover_on_class(workspace):
+    # Over "A" in "class A:"
+    res = _hover_result_in_doc(workspace, (4, 7))
+    assert "A()" in res  # signature
+    assert "Docstring for class A" in res  # docstring
+
+    # Over "A" in "instance_of_a = A()"
+    res = _hover_result_in_doc(workspace, (33, 17))
+    assert "A()" in res  # signature
+    assert "Docstring for class A" in res  # docstring
+
+    # Over "copy_of_class_a" in "copy_of_class_a = A" - needs infer
+    res = _hover_result_in_doc(workspace, (34, 4))
+    assert "A()" in res  # signature
+    assert "Docstring for class A" in res  # docstring
+
+
+def test_hover_on_property(workspace):
+    # Over "b" in "b = 42"
+    res = _hover_result_in_doc(workspace, (7, 5))
+    assert "int" in res  # type
+    assert "Docstring for the class property A.b" in res  # docstring
+
+    # Over "b" in "A.b"
+    res = _hover_result_in_doc(workspace, (35, 24))
+    assert "int" in res  # type
+    assert "Docstring for the class property A.b" in res  # docstring
+
+
+def test_hover_on_method(workspace):
+    # Over "foo" in "def foo(self):"
+    res = _hover_result_in_doc(workspace, (10, 10))
+    assert "foo(self)" in res  # signature
+    assert "Docstring for A.foo" in res  # docstring
+
+
+def test_hover_multiple_definitions(workspace):
+    # Over "int_or_string_value"
+    res = _hover_result_in_doc(workspace, (36, 5))
+    assert "```python\nUnion[int, str]\n```" == res.strip()  # only type
+
+    # Over "overload_function"
+    res = _hover_result_in_doc(workspace, (37, 5))
+    assert (
+        "overload_function(s: int) -> int\noverload_function(s: str) -> str" in res
+    )  # signature
+    assert "Docstring of overload function" in res  # docstring
+
+
 def test_numpy_hover(workspace):
     # Over the blank line
     no_hov_position = {"line": 1, "character": 0}
@@ -38,7 +149,9 @@ def test_numpy_hover(workspace):
     doc = Document(DOC_URI, workspace, NUMPY_DOC)
 
     contents = ""
-    assert contents in pylsp_hover(doc._config, doc, no_hov_position)["contents"]
+    assert (
+        contents in pylsp_hover(doc._config, doc, no_hov_position)["contents"]["value"]
+    )
 
     contents = "NumPy\n=====\n\nProvides\n"
     assert (
@@ -72,21 +185,6 @@ def test_numpy_hover(workspace):
         )
 
 
-def test_hover(workspace):
-    # Over 'main' in def main():
-    hov_position = {"line": 2, "character": 6}
-    # Over the blank second line
-    no_hov_position = {"line": 1, "character": 0}
-
-    doc = Document(DOC_URI, workspace, DOC)
-
-    contents = {"kind": "markdown", "value": "```python\nmain()\n```\n\n\nhello world"}
-
-    assert {"contents": contents} == pylsp_hover(doc._config, doc, hov_position)
-
-    assert {"contents": ""} == pylsp_hover(doc._config, doc, no_hov_position)
-
-
 def test_document_path_hover(workspace_other_root_path, tmpdir):
     # Create a dummy module out of the workspace's root_path and try to get
     # a definition on it in another file placed next to it.