[BUG] sphinx.ext.viewcode incorrectly detects properties and class attributes.

Describe the bug

Currently, sphinx.ext.viewcode allows to jump to sourcecode of a class, method, or function but not to a property or a class attribute (assuming that the latter has a type comment or an annotation). Using the example below, the tags found by sphinx.pycode.ModuleAnalyzer are

 {'MyClass.bar': ('def', 9, 12), 
  'MyClass.baz': ('def', 15, 17), 
  'MyClass': ('class', 3, 17)}

In particular, the definition of the property bar is detected but it will not be shown when rendered. This can be avoided if viewcode_follow_imported_members = False (which is by default True) but I feel that this should not be the real purpose. Now, with the default value True, the following code is executed

https://github.com/sphinx-doc/sphinx/blob/669bcc0a190ce9921451bad28431886fbaad170b/sphinx/ext/viewcode.py#L113-L119

However, at the end of the block, modname is None if no handler is specified ! the reason is that _get_full_modname calls sphinx.util.get_full_modname which indeed finds the correct member. However,

https://github.com/sphinx-doc/sphinx/blob/669bcc0a190ce9921451bad28431886fbaad170b/sphinx/util/__init__.py#L196-L210

returns the __module__ field which is usually None for properties and class attributes. With the example below, we get:

Member	`value`	`fullname`	`modname`	`new_modname`
MyClass	`<class 'class.MyClass'>`	`'MyClass'`	`class`	`class`
MyClass.bar	`<property object at 0x...>`	`'MyClass.bar'`	`class`	`None`
MyClass.baz	`<function MyClass.baz at 0x...>`	`'MyClass.baz'`	`class`	`class`
MyClass.foo	`0`	`'MyClass.foo'`	`class`	`None`

As we can observe, because the module of MyClass.bar and MyClass.foo were not properly deduced, they are no more considered as entries with a possible tag. For MyClass.foo, this is expected since the definition is not even detected, however for MyClass.bar, this is unexpected.

Fix for properties

For properties, this report should be categorized as a "bug" since the definition is properly detected but never handled. One possible fix is to have an event handler for viewcode-follow-imported implemented as follows:

from sphinx.ext.viewcode import _get_full_modname

def on_viewcode_follow_imported(app, modname, fullname):
    ret = _get_full_modname(app, modname, fullname)
    return modname if ret is None else ret

def setup(app):
    app.connect('viewcode-follow-imported', on_viewcode_follow_imported)

One issue with the above fix is that it may conflict with other handlers, so it might not be a universal patch.

Feature for class attributes

Even if the tags dictionary contains the class attribute fullname, the issue that we had with properties would subsist since the module name is incorrectly detected. It could be patched using the same patch, but some conflicts with 3rd party extensions may arise as well.

In order to detect the definition location of a class attribute, one can easily extend the current sphinx.pycode.ModuleAnalyzer and sphinx.pycode.parser.VariableCommentPicker as follows:

import ast
import contextlib

from sphinx.errors import PycodeError
from sphinx.pycode import ModuleAnalyzer
from sphinx.pycode.parser import VariableCommentPicker

class CustomCodeAnalyzer(VariableCommentPicker):
    def __init__(self, modname, *args, **kwargs):
        self.modname = modname
        kwargs.setdefault('encoding', 'utf-8')
        super().__init__(*args, **kwargs)
        #: Mapping from identifiers to definition blocks ('def', start, end).
        self.definitions: dict[str, tuple[str, int, int]] = {}

    def get_identifier(self, name):
        # type: (str) -> str
        qualname = self.get_qualname_for(name)
        return '.'.join(qualname) if qualname else name

    def get_identifiers(self):
        # type: () -> frozenset[str]
        ret = set(self.deforders.keys())

        def get_identifier(entry):
            return self.get_identifier(entry[1])

        # Adding variable names with type comments allows to pick-up those
        # that do not have an annotation or that are not in :attr:`deforders`.
        ret.update(map(get_identifier, self.comments))

        # Adding variable names with annotations allows to pick-up those
        # that do not have type comments or that are not in :attr:`deforders`.
        ret.update(map(get_identifier, self.annotations))

        # depending on the implementation of `VariableCommentPicker`, more
        # identifier-like names could be added
        return frozenset(ret)

    def add_definition(self, identifier, node):
        # type: (str, ast.AST) -> None
        if not hasattr(node, 'lineno'): 
            return
        start = node.lineno
        end = getattr(node, 'end_lineno', start)
        self.definitions[identifier] = ('def', start, end)

    def visit_Assign(self, node):
        # type: (ast.Assign) -> None
        with self._track_definitions(node):
            super().visit_Assign(node)

    def visit_AnnAssign(self, node):
        # type: (ast.AnnAssign) -> None
        with self._track_definitions(node):
            super().visit_AnnAssign(node)

    @contextlib.contextmanager
    def _track_definitions(self, node):
        # type: (ast.AST) -> None

        # current known identifiers
        identifiers_set = self.get_identifiers()

        try:
            yield
        finally:
            # the identifiers that were added
            new_identifiers = self.get_identifiers() - identifiers_set
            for identifier in new_identifiers:
                self.add_definition(identifier, node)

class CustomModuleAnalyzer(ModuleAnalyzer):
    # required for objects being cached to be of CustomModuleAnalyzer type
    cache: dict[tuple[str, str], object] = {}

    def analyze(self):
        if self._analyzed:
            return None

        super().analyze()

        try:
            tree = ast.parse(self.code, type_comments=True)
            buffers = self.code.splitlines(True)
            picker = CustomCodeAnalyzer(self.modname, buffers)
            picker.visit(tree)
            self.tags.update(picker.definitions)
        except Exception as e:
            raise PycodeError(f'parsing {self.srcname!r} failed: {e!r}') from e

Then, we would implement the following event handlers:

import sphinx.ext.viewcode
from sphinx.ext.viewcode import _get_full_modname
# from [...] import CustomModuleAnalyzer

def viewcode_find_source(app, modname):
    try:
        # use CustomModuleAnalyzer for analyzing the module
        analyzer = CustomModuleAnalyzer.for_module(modname)
        analyzer.find_tags()
    except:  # pylint: disable=W0702
        return None

    code = analyzer.code
    tags = analyzer.tags
    return code, tags

def viewcode_follow_imported(app, modname, fullname):
    ret = _get_full_modname(app, modname, fullname)
    return modname if ret is None else ret

def setup(app):
    app.setup_extension('sphinx.ext.viewcode')
    app.connect('viewcode-find-source', viewcode_find_source)
    app.connect('viewcode-follow-imported', viewcode_follow_imported)
    return {'parallel_read_safe': True}

How to Reproduce

class.py

class MyClass:
    """My class."""

    #: The foo attribute.
    foo = 0

    @property
    def bar(self):
        """The bar property."""
        pass

    def baz(self):
        """The baz method."""
        pass

index.rst

Foo
===

.. autoclass:: class.MyClass
   :members:

conf.py

import os
import sys

sys.path.insert(0, os.path.abspath('.'))

project = 'Foo'
copyright = '2023, picnixz'
author = 'picnixz'

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

templates_path = ['_templates']

master_doc = 'index'

include_patterns = ['index.rst', 'class.py']
exclude_patterns = ['_build', 'Thumbs.db', '.DS_Store']

html_theme = 'alabaster'

html_static_path = ['_static']

Environment Information

Platform:              linux; (Linux-5.3.18-lp152.106-default-x86_64-with-glibc2.26)
Python version:        3.10.3 (main, Jan 31 2023, 10:47:25) [GCC 7.5.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

['sphinx.ext.autodoc', 'sphinx.ext.viewcode']

Additional context

No response

sphinx-doc / sphinx