Repository: sphinx-doc/sphinx

Issue:
autodoc maybe should ignore return annotations on classes
Subject: autodoc adds "→ None" to classes

### Problem
We noticed on Trio (https://github.com/python-trio/trio/issues/529) that some classes that have a return annotation on `__init__` are rendered by autodoc with "→ None".

Example:
http://trio.readthedocs.io/en/latest/reference-core.html#trio.Event
`class trio.Event → None`

This class uses `attrs`, which [adds a `-> None` annotation to the generated `__init__` method](https://github.com/python-attrs/attrs/commit/fc033b41a11c9ef3e5fd7bee7f936427d4bb5f3d#diff-21756223332b04173cec8910fda6655bR1264).

A minimal example could be:

```python
class Foo:
    """Class docstring"""
    def __init__(self) -> None:
        pass
```

And `.. autoclass:: Foo` will generate `class Foo → None`.

I would expect autodoc to ignore the return annotation for the `__init__` method when it is used to build the signature for the class itself. A return annotation of `None` is legal for `__init__` but it doesn't make sense to pick it up to document the class signature.

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=h1) Report
> Merging [#4972](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=desc) into [master](https://codecov.io/gh/sphinx-doc/sphinx/commit/1a4a728816946d104fd70cc8484137e677c3c1b7?src=pr&el=desc) will **increase** coverage by `0.01%`.
> The diff coverage is `66.66%`.

[![Impacted file tree graph](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/graphs/tree.svg?height=150&token=VFlFSELbkw&width=650&src=pr)](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master    #4972      +/-   ##
==========================================
+ Coverage    82.2%   82.21%   +0.01%     
==========================================
  Files         289      294       +5     
  Lines       38402    38730     +328     
  Branches     5915     5970      +55     
==========================================
+ Hits        31569    31843     +274     
- Misses       5521     5567      +46     
- Partials     1312     1320       +8
```


| [Impacted Files](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [sphinx/util/inspect.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L3V0aWwvaW5zcGVjdC5weQ==) | `73.61% <100%> (+0.18%)` | :arrow_up: |
| [tests/test\_util\_inspect.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-dGVzdHMvdGVzdF91dGlsX2luc3BlY3QucHk=) | `90.78% <100%> (+0.08%)` | :arrow_up: |
| [sphinx/ext/autodoc/\_\_init\_\_.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L2V4dC9hdXRvZG9jL19faW5pdF9fLnB5) | `85% <20%> (-0.23%)` | :arrow_down: |
| [sphinx/builders/html.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L2J1aWxkZXJzL2h0bWwucHk=) | `82.51% <0%> (-0.12%)` | :arrow_down: |
| [sphinx/errors.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L2Vycm9ycy5weQ==) | `70% <0%> (ø)` | |
| [sphinx/apidoc.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L2FwaWRvYy5weQ==) | `0% <0%> (ø)` | |
| [sphinx/quickstart.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L3F1aWNrc3RhcnQucHk=) | `0% <0%> (ø)` | |
| [sphinx/search/\_\_init\_\_.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L3NlYXJjaC9fX2luaXRfXy5weQ==) | `96.88% <0%> (ø)` | |
| [sphinx/\_\_init\_\_.py](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972/diff?src=pr&el=tree#diff-c3BoaW54L19faW5pdF9fLnB5) | `60% <0%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=footer). Last update [1a4a728...8607322](https://codecov.io/gh/sphinx-doc/sphinx/pull/4972?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/CHANGES b/CHANGES
index 87b134488..7a06cef75 100644
--- a/CHANGES
+++ b/CHANGES
@@ -27,6 +27,7 @@ Bugs fixed
   (escaped) line breaks are present.
 * C++, properly use ``desc_addname`` nodes for prefixes of names.
 * #4915, #4916: links on search page are broken when using dirhtml builder
+* #4969: autodoc: constructor method should not have return annotation
 
 Testing
 --------
diff --git a/sphinx/ext/autodoc/__init__.py b/sphinx/ext/autodoc/__init__.py
index d516146bf..eabe487e3 100644
--- a/sphinx/ext/autodoc/__init__.py
+++ b/sphinx/ext/autodoc/__init__.py
@@ -1035,9 +1035,11 @@ def format_args(self):
             # typing) we try to use the constructor signature as function
             # signature without the first argument.
             try:
-                args = Signature(self.object.__new__, bound_method=True).format_args()
+                sig = Signature(self.object.__new__, bound_method=True, has_retval=False)
+                args = sig.format_args()
             except TypeError:
-                args = Signature(self.object.__init__, bound_method=True).format_args()
+                sig = Signature(self.object.__init__, bound_method=True, has_retval=False)
+                args = sig.format_args()
 
         # escape backslashes for reST
         args = args.replace('\\', '\\\\')
@@ -1090,7 +1092,7 @@ def format_args(self):
                 not(inspect.ismethod(initmeth) or inspect.isfunction(initmeth)):
             return None
         try:
-            return Signature(initmeth, bound_method=True).format_args()
+            return Signature(initmeth, bound_method=True, has_retval=False).format_args()
         except TypeError:
             # still not possible: happens e.g. for old-style classes
             # with __init__ in C
diff --git a/sphinx/util/inspect.py b/sphinx/util/inspect.py
index 492db8700..72c3065bc 100644
--- a/sphinx/util/inspect.py
+++ b/sphinx/util/inspect.py
@@ -298,8 +298,8 @@ class Signature(object):
     its return annotation.
     """
 
-    def __init__(self, subject, bound_method=False):
-        # type: (Callable, bool) -> None
+    def __init__(self, subject, bound_method=False, has_retval=True):
+        # type: (Callable, bool, bool) -> None
         # check subject is not a built-in class (ex. int, str)
         if (isinstance(subject, type) and
                 is_builtin_class_method(subject, "__new__") and
@@ -307,6 +307,7 @@ def __init__(self, subject, bound_method=False):
             raise TypeError("can't compute signature for built-in type {}".format(subject))
 
         self.subject = subject
+        self.has_retval = has_retval
         self.partialmethod_with_noargs = False
 
         if PY3:
@@ -375,7 +376,10 @@ def parameters(self):
     def return_annotation(self):
         # type: () -> Any
         if PY3 and self.signature:
-            return self.signature.return_annotation
+            if self.has_retval:
+                return self.signature.return_annotation
+            else:
+                return inspect.Parameter.empty
         else:
             return None
 


Test patch:
diff --git a/tests/test_util_inspect.py b/tests/test_util_inspect.py
index 6a3e25b6e..76f21a5e1 100644
--- a/tests/test_util_inspect.py
+++ b/tests/test_util_inspect.py
@@ -280,6 +280,10 @@ def test_Signature_annotations():
     sig = inspect.Signature(f11).format_args()
     assert sig == '(x: CustomAnnotation, y: 123) -> None'
 
+    # has_retval=False
+    sig = inspect.Signature(f11, has_retval=False).format_args()
+    assert sig == '(x: CustomAnnotation, y: 123)'
+
 
 def test_safe_getattr_with_default():
     class Foo(object):