Keep inline annotations over docstring generated ones (#61)

* first pass at keeping inline annotation over docstring

* Make override_docsring tests more independent

Keeping parameters is already tested in `override_docstring_param`
above – no need to do so again.

* Remove duplication in error message

The `underline` method of the `ErrorReporter` includes the underlined
line already.

* Simplify logic when ignoring docstring annotation

Also makes sure that imports of a doctype are not unnecessarily
included an inline annotation already exists.

* Keep existing inline annotation for assignments

even if a different docstring annotation exists. This is the same
behavior as for parameters and return types.

* Add more tests

* Re-enable commented out tests

Probably forgot to do so during debugging.

* Document inline annotation as overwrite solution

---------

Co-authored-by: Lars Grüter <lagru@mailbox.org>

Author: OriolAbril

docs/user_guide.md

@@ -172,9 +172,35 @@ Two command line options can help addressing these errors gradually:
 
 ## Dealing with typing problems
 
-Docstub may not fully or correctly implement a particular part of Python's typing system yet.
+For various reasons – missing features in docstub, or limitations of Python's typing system – it may not always be possible to correctly type something in a docstring.
+In those cases, you docstub provides a few approaches to dealing with this.
 
-In some cases, you can use a comment directive to selectively disable docstub for a specific block of lines:
+
+### Use inline type annotation
+
+Docstub will always preserve inline type annotations, regardless of what the docstring contains.
+This is useful for example, if you want to express something that isn't yet supported by Python's type system.
+
+E.g., consider the docstring type of `ord` parameter in [`numpy.linalg.matrix_norm`](https://numpy.org/doc/stable/reference/generated/numpy.linalg.matrix_norm.html)
+```rst
+ord : {1, -1, 2, -2, inf, -inf, ‘fro’, ‘nuc’}, optional
+```
+[Python's type system currently can't express floats as literal types](https://typing.python.org/en/latest/spec/literal.html#:~:text=Floats%3A%20e.g.%20Literal%5B3.14%5D) – such as `inf`.
+We don't want to make the type description here less specific to users, so instead, you could handle this with a less constrained inline type annotation like
+```python
+ord: Literal[1, -1, 2, -2, 'fro', 'nuc'] | float
+```
+Docstub will include the latter less constrained type in the stubs.
+This allows you to keep the information in the docstring while still having valid – if a bit less constrained – stubs.
+
+
+### Preserve code with comment directive
+
+At its heart, docstub transforms Python source files into stub files.
+You can tell docstub to temporarily stop that transformation for a specific area with a comment directive.
+Wrapping lines of code with `docstub: off` and `docstub: on` comments will preserve these lines completely.
+
+E.g., consider the following example:
 ```python
 class Foo:
     # docstub: off
@@ -184,7 +210,7 @@ class Foo:
     c: int = None
     d: str = ""
 ```
-will leave the parameters within the `# docstub` guards untouched in the resulting stub file:
+will leave the guarded parameters untouched in the resulting stub file:
 ```python
 class Foo:
     a: int = None
@@ -193,5 +219,7 @@ class Foo:
     d: str
 ```
 
-If that is not possible, you can – for now – fallback to writing a correct stub file by hand.
+### Write a manual stub file
+
+If all of the above does not solve your issue, you can fall back to writing a correct stub file by hand.
 Docstub will preserve this file and integrated it with other automatically generated stubs.

examples/example_pkg-stubs/_basic.pyi

@@ -29,6 +29,10 @@ def func_contains(
 def func_literals(
     a1: Literal[1, 3, "foo"], a2: Literal["uno", 2, "drei", "four"] = ...
 ) -> None: ...
+def override_docstring_param(
+    d1: dict[str, float], d2: dict[Literal["a", "b", "c"], int]
+) -> None: ...
+def override_docstring_return() -> list[Literal[-1, 0, 1] | float]: ...
 def func_use_from_elsewhere(
     a1: CustomException,
     a2: ExampleClass,
@@ -37,6 +41,9 @@ def func_use_from_elsewhere(
 ) -> tuple[CustomException, ExampleClass.NestedClass]: ...
 
 class ExampleClass:
+
+    b1: int
+
     class NestedClass:
         def method_in_nested_class(self, a1: complex) -> None: ...
 

examples/example_pkg/_basic.py

@@ -5,6 +5,7 @@
 
 # Existing imports are preserved
 import logging
+from typing import Literal
 
 # Assign-statements are preserved
 logger = logging.getLogger(__name__)  # Inline comments are stripped
@@ -51,6 +52,25 @@ def func_literals(a1, a2="uno"):
     """
 
 
+def override_docstring_param(d1, d2: dict[Literal["a", "b", "c"], int]):
+    """Check type hint is kept and overrides docstring.
+
+    Parameters
+    ----------
+    d1 : dict of {str : float}
+    d2 : dict of {str : int}
+    """
+
+
+def override_docstring_return() -> list[Literal[-1, 0, 1] | float]:
+    """Check type hint is kept and overrides docstring.
+
+    Returns
+    -------
+    {"-inf", 0, 1, "inf"}
+    """
+
+
 def func_use_from_elsewhere(a1, a2, a3, a4):
     """Check if types with full import names are matched.
 
@@ -75,10 +95,15 @@ class ExampleClass:
     ----------
     a1 : str
     a2 : float, default 0
+
+    Attributes
+    ----------
+    b1 : Sized
     """
 
-    class NestedClass:
+    b1: int
 
+    class NestedClass:
         def method_in_nested_class(self, a1):
             """
 

src/docstub/_stubs.py

@@ -571,25 +571,31 @@ def leave_FunctionDef(self, original_node, updated_node):
             assert ds_annotations.returns.value
             annotation_value = ds_annotations.returns.value
 
-            if original_node.returns is not None:
+            if original_node.returns is None:
+                annotation = cst.Annotation(cst.parse_expression(annotation_value))
+                node_changes["returns"] = annotation
+                # TODO: check imports
+                self._required_imports |= ds_annotations.returns.imports
+
+            else:
+                # Notify about ignored docstring annotation
+                # TODO: either remove message or print only in verbose mode
                 position = self.get_metadata(
                     cst.metadata.PositionProvider, original_node
                 ).start
                 reporter = self.reporter.copy_with(
                     path=self.current_source, line=position.line
                 )
-                replaced = _inline_node_as_code(original_node.returns.annotation)
+                to_keep = _inline_node_as_code(original_node.returns.annotation)
                 details = (
-                    f"{replaced}\n{reporter.underline(replaced)} -> {annotation_value}"
+                    f"{reporter.underline(to_keep)} "
+                    f"ignoring docstring: {annotation_value}"
                 )
                 reporter.message(
-                    short="Replacing existing inline return annotation",
+                    short="Keeping existing inline return annotation",
                     details=details,
                 )
 
-            annotation = cst.Annotation(cst.parse_expression(annotation_value))
-            node_changes["returns"] = annotation
-            self._required_imports |= ds_annotations.returns.imports
         elif original_node.returns is None:
             annotation = cst.Annotation(cst.parse_expression("None"))
             node_changes["returns"] = annotation
@@ -633,10 +639,35 @@ def leave_Param(self, original_node, updated_node):
             if pytype:
                 if defaults_to_none:
                     pytype = pytype.as_optional()
-                annotation = cst.Annotation(cst.parse_expression(pytype.value))
-                node_changes["annotation"] = annotation
-                if pytype.imports:
-                    self._required_imports |= pytype.imports
+                annotation_value = pytype.value
+
+                if original_node.annotation is None:
+                    annotation = cst.Annotation(cst.parse_expression(annotation_value))
+                    node_changes["annotation"] = annotation
+                    # TODO: check imports
+                    if pytype.imports:
+                        self._required_imports |= pytype.imports
+
+                else:
+                    # Notify about ignored docstring annotation
+                    # TODO: either remove message or print only in verbose mode
+                    position = self.get_metadata(
+                        cst.metadata.PositionProvider, original_node
+                    ).start
+                    reporter = self.reporter.copy_with(
+                        path=self.current_source, line=position.line
+                    )
+                    to_keep = cst.Module([]).code_for_node(
+                        original_node.annotation.annotation
+                    )
+                    details = (
+                        f"{reporter.underline(to_keep)} "
+                        f"ignoring docstring: {annotation_value}"
+                    )
+                    reporter.message(
+                        short="Keeping existing inline parameter annotation",
+                        details=details,
+                    )
 
         # Potentially use "Incomplete" except for first param in (class)methods
         elif not is_self_or_cls and updated_node.annotation is None:
@@ -764,31 +795,33 @@ def leave_AnnAssign(self, original_node, updated_node):
         if pytypes and name in pytypes.attributes:
             pytype = pytypes.attributes[name]
             expr = cst.parse_expression(pytype.value)
-            self._required_imports |= pytype.imports
 
-            if updated_node.annotation is not None:
-                # Turn original annotation into str and print with context
+            if updated_node.annotation is None:
+                self._required_imports |= pytype.imports
+                updated_node = updated_node.with_deep_changes(
+                    updated_node.annotation, annotation=expr
+                )
+
+            else:
+                # Notify about ignored docstring annotation
+                # TODO: either remove message or print only in verbose mode
                 position = self.get_metadata(
                     cst.metadata.PositionProvider, original_node
                 ).start
                 reporter = self.reporter.copy_with(
                     path=self.current_source, line=position.line
                 )
-                replaced = cst.Module([]).code_for_node(
+                to_keep = cst.Module([]).code_for_node(
                     updated_node.annotation.annotation
                 )
                 details = (
-                    f"{replaced}\n{reporter.underline(replaced)} -> {pytype.value}"
+                    f"{reporter.underline(to_keep)} ignoring docstring: {pytype.value}"
                 )
                 reporter.message(
-                    short="Replacing existing inline annotation",
+                    short="Keeping existing inline annotation for assignment",
                     details=details,
                 )
 
-            updated_node = updated_node.with_deep_changes(
-                updated_node.annotation, annotation=expr
-            )
-
         return updated_node
 
     def visit_Module(self, node):