sphinx-doc / sphinx-doc/sphinx

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

Open
#11,279 1 comment 1 reaction 0 assignees View on GitHub

Nobody has claimed this yet.

extensions:viewcode
Dominant language
Python
Stars
8k
Forks
2.6k
PR merge metrics
No merged PRs in 30d

Description

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

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Reproduce the issue with class.py, index.rst, and conf.py, then inspect sphinx/ext/viewcode.py and sphinx/util/init.py to trace how properties and class attributes are resolved. Review sphinx/pycode/ModuleAnalyzer and sphinx/pycode/parser.py for tag detection. Done means viewcode links to the source definitions of both the property and annotated or type-commented class attributes.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
documentation
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.