Improve hover results

pylsp/plugins/hover.py

@@ -1,50 +1,100 @@
 # Copyright 2017-2020 Palantir Technologies, Inc.
 # Copyright 2021- Python Language Server Contributors.
 
+import itertools
 import logging
 
 from pylsp import _utils, hookimpl
 
 log = logging.getLogger(__name__)
 
 
-@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)
+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_and_types(definitions):
+    def _line_number(definition):
+        """Helper for sorting definitions by line number (which might be None)."""
+        return definition.line if definition.line is not None else 0
 
-    # Find first exact matching definition
-    definition = next((x for x in definitions if x.name == word), None)
+    def _get_signatures(definition):
+        """Get the signatures of functions and classes."""
+        return [
+            signature.to_string()
+            for signature in definition.get_signatures()
+            if signature.type in ["class", "function"]
+        ]
 
-    # 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]
+    definitions = sorted(definitions, key=_line_number)
+    signatures_per_def = [_get_signatures(d) for d in definitions]
+    types_per_def = [d.infer() for d in definitions]
 
-    if not definition:
-        return {"contents": ""}
+    # a flat list with all signatures
+    signatures = list(itertools.chain(*signatures_per_def))
+
+    # We want to show the type if there is at least one type that does not
+    # correspond to a signature
+    if any(
+        len(s) == 0 and len(t) > 0 for s, t in zip(signatures_per_def, types_per_def)
+    ):
+        # Get all types (also the ones that correspond to a signature)
+        types = set(itertools.chain(*types_per_def))
+        type_names = [t.name for t in sorted(types, key=_line_number)]
+
+        if len(type_names) == 1:
+            return [*signatures, type_names[0]]
+        elif len(type_names) > 1:
+            return [*signatures, f"Union[{', '.join(type_names)}]"]
+
+    else:
+        # The type does not add any information because it is already in the signatures
+        return signatures
+
+
+@hookimpl
+def pylsp_hover(config, document, position):
+    code_position = _utils.position_to_jedi_linecolumn(document, position)
+    definitions = document.jedi_script(use_document_path=True).help(**code_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_and_types(definitions),
         )
     }

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 (
@@ -71,21 +184,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.