autodoc confuses constructor parameter typehint with class attribute typehint

[jmandreoli]

Describe the bug

The following script

class MyClass:
  "An instance of this class holds a list.\n\n:param r: the list as a string"
  l: list
  def __init__(self,r:str): self.l = list(r)

produces the following doc, as expected:

An instance of this class holds a list.
Parameters * r (str): the list as a string

Now, if I just change the name of the class attribute (and name it the same as the parameter), then Sphinx gets confused.

class MyClass:
  "An instance of this class holds a list.\n\n:param r: the list as a string"
  r: list
  def __init__(self,r:str): self.r = list(r)

produces the following doc, where the parameter is shown as having the type of the attribute with same name:

An instance of this class holds a list.
Parameters * r (list): the list as a string

Having a class attribute and a constructor parameter with the same name but not exactly the same type is quite common. For example, the parameter could be of type Optional[T] and the attribute of type T, with a default value being assigned in the constructor.

How to Reproduce

In index.rst:

.. toctree::
   :maxdepth: 2
   :caption: Contents:

.. automodule:: TEST
   :members:
   :member-order: bysource
   :show-inheritance:

In conf.py:

extensions = ['sphinx.ext.autodoc','sphinx.ext.viewcode']
autodoc_typehints = 'description'

Environment Information

Platform:              linux; (Linux-6.1.12-100.fc36.x86_64-x86_64-with-glibc2.35)
Python version:        3.10.9 | packaged by conda-forge | (main, Feb  2 2023, 20:20:04) [GCC 11.3.0])
Python implementation: CPython
Sphinx version:        6.1.3
Docutils version:      0.18.1
Jinja2 version:        3.1.2
Pygments version:      2.14.0

Sphinx extensions

No response

Additional context

No response

[greschd]

A full reproducing example for this issue can be found here: https://github.com/greschd/sphinx-autodoc-typehints-confusion

[greschd]

This immediate issue could be resolved with the following patch

diff --git a/sphinx/util/inspect.py b/sphinx/util/inspect.py
index c925485e1..8c90ccb10 100644
--- a/sphinx/util/inspect.py
+++ b/sphinx/util/inspect.py
@@ -666,6 +666,11 @@ def signature(
         localns = TypeAliasNamespace(type_aliases)
         annotations = typing.get_type_hints(subject, None, localns, include_extras=True)
         for i, param in enumerate(parameters):
+            # Skip parameters which have annotations already. This is needed to avoid
+            # overwriting annotations of a class __init__ with annotations of class
+            # attributes.
+            if param.annotation is not inspect.Parameter.empty:
+                continue
             if param.name in annotations:
                 annotation = annotations[param.name]
                 if isinstance(annotation, TypeAliasForwardRef):

This skips overwriting the annotation in sphinx.util.inspect.signature if there is already an annotation given by inspect.signature.

I'd be happy to create a PR and add some tests, but don't have enough context to know whether this change might break things elsewhere. The most obvious case would be if a project relies on the current overwriting behavior. Maybe this should be behind an option?

@AA-Turner, maybe you could comment on this?

[greschd]

In a more complex example, I have observed that napoleon_attr_annotations = True also has the effect of overwriting the annotations; this may need a similar treatment.