sphinx-doc / sphinx

The Sphinx documentation generator
https://www.sphinx-doc.org/
Other
6.39k stars 2.09k forks source link

docstrings of inherited members from separate project fail if they reference other docs in that project #10640

Open davidism opened 2 years ago

davidism commented 2 years ago

Describe the bug

Flask's Response class inherits from Werkzeug's Response class. It inherits two attributes, max_content_length and max_form_memory_size that have docstrings in Werkzeug. The docstrings reference :doc:`/request_data`, which is a valid reference in Werkzeug but not in Flask. With warnings treated as errors, we can no longer render Flask's docs successfully with Sphinx 5.0.2. This does not happen in Sphinx 4.5.0.

How to Reproduce

$ git clone https://github.com/pallets/flask
$ cd flask
$ pip install -r requirements/dev.txt
$ cd docs
$ make clean html O=-W

Warning, treated as error:
docstring of flask.Request.max_form_memory_size:7:unknown document: /request_data

Expected behavior

References in other libraries should point to those libraries, not the current project.

Your project

https://github.com/pallets/flask

Screenshots

No response

OS

Any

Python version

3.10

Sphinx version

5.0.2

Sphinx extensions

intersphinx

Extra tools

No response

Additional context

No response

davidism commented 2 years ago

Anything I can do to help identify the issue?

I think I can work around this by removing :doc: references from docstrings, but it's not clear why that limitation was introduced in 5.0, there's no reference to the change.

AA-Turner commented 2 years ago

Another workaround is to add the errors to the whitelist in conf.py.

At a first guess, maybe this PR is related (https://github.com/sphinx-doc/sphinx/pull/10539) -- apologies in advance if the issue turns out to be my fault. If 5.0.0/5.0.1 work, this is a good indicator that said PR is to blame.

In terms of identification otherwise -- it's hard beyond pure git bisection. You could look at the PRs labelled with autodoc and against 5.0.0, but I'm not sure how much help it'd be.

A

davidism commented 2 years ago

Git bisect says that efa6197ffcdd1d4763079d42fb4338f101587685 (https://github.com/sphinx-doc/sphinx/pull/10447) is where this issue was introduced.

$ git bisect run sphinx-build -b dirhtml -a -E -W ../flask/docs ../flask/docs/_build/html

efa6197ffcdd1d4763079d42fb4338f101587685 is the first bad commit
commit efa6197ffcdd1d4763079d42fb4338f101587685
Date:   Wed May 11 13:30:02 2022 +0200

    Set the docstring attribute of class members. Fixes #8180.

    Change ext.autodoc.importer.get_class_members to set
    ObjectMember.docstring the docstring found by the source code
    analyzer.

 sphinx/ext/autodoc/importer.py | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
bisect found first bad commit

It still fails with Sphinx 5.0.2.

davidism commented 2 years ago

That makes sense why it's failing now, prior to 5.0 Sphinx did not include inherited class attributes, which meant that it didn't encounter the reference in the docstring.

I don't mind using the ignore list or modifying the docstring, but it seems like this should be something that's addressed anyway so I'll leave this open.