Fix two relative link bugs in inheritance diagrams (#11634)

Co-authored-by: Adam Turner <9087854+aa-turner@users.noreply.github.com>

CHANGES

@@ -20,6 +20,9 @@ Bugs fixed
   packages that make use of ``if typing.TYPE_CHECKING:`` to guard circular
   imports needed by type checkers.
   Patch by Matt Wozniski.
+* #11634: Fixed inheritance diagram relative link resolution
+  for sibling files in a subdirectory.
+  Patch by Albert Shih.
 
 Testing
 -------

sphinx/ext/graphviz.py

@@ -21,7 +21,7 @@ import sphinx
 from sphinx.errors import SphinxError
 from sphinx.locale import _, __
 from sphinx.util import logging
-from sphinx.util.docutils import SphinxDirective, SphinxTranslator
+from sphinx.util.docutils import SphinxDirective
 from sphinx.util.i18n import search_image_for_language
 from sphinx.util.nodes import set_source_info
 from sphinx.util.osutil import ensuredir
@@ -218,7 +218,8 @@ class GraphvizSimple(SphinxDirective):
             return [figure]
 
 
-def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None:
+def fix_svg_relative_paths(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator,
+                           filepath: str) -> None:
     """Change relative links in generated svg files to be relative to imgpath."""
     tree = ET.parse(filepath)  # NoQA: S314
     root = tree.getroot()
@@ -230,16 +231,20 @@ def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None:
         root.findall('.//svg:image[@xlink:href]', ns),
         root.findall('.//svg:a[@xlink:href]', ns),
     ):
-        scheme, hostname, url, query, fragment = urlsplit(element.attrib[href_name])
+        scheme, hostname, rel_uri, query, fragment = urlsplit(element.attrib[href_name])
         if hostname:
             # not a relative link
             continue
 
-        old_path = path.join(self.builder.outdir, url)
+        docname = self.builder.env.path2doc(self.document["source"])
-        new_path = path.relpath(
+        if docname is None:
-            old_path,
+            # This shouldn't happen!
-            start=path.join(self.builder.outdir, self.builder.imgpath),
+            continue
-        )
+        doc_dir = self.builder.app.outdir.joinpath(docname).resolve().parent
+
+        old_path = doc_dir / rel_uri
+        img_path = doc_dir / self.builder.imgpath
+        new_path = path.relpath(old_path, start=img_path)
         modified_url = urlunsplit((scheme, hostname, new_path, query, fragment))
 
         element.set(href_name, modified_url)
@@ -249,7 +254,8 @@ def fix_svg_relative_paths(self: SphinxTranslator, filepath: str) -> None:
         tree.write(filepath)
 
 
-def render_dot(self: SphinxTranslator, code: str, options: dict, format: str,
+def render_dot(self: HTML5Translator | LaTeXTranslator | TexinfoTranslator,
+               code: str, options: dict, format: str,
                prefix: str = 'graphviz', filename: str | None = None,
                ) -> tuple[str | None, str | None]:
     """Render graphviz code into a PNG or PDF output file."""
@@ -294,8 +300,8 @@ def render_dot(self: SphinxTranslator, code: str, options: dict, format: str,
         logger.warning(__('dot command %r cannot be run (needed for graphviz '
                           'output), check the graphviz_dot setting'), graphviz_dot)
         if not hasattr(self.builder, '_graphviz_warned_dot'):
-            self.builder._graphviz_warned_dot = {}  # type: ignore[attr-defined]
+            self.builder._graphviz_warned_dot = {}  # type: ignore[union-attr]
-        self.builder._graphviz_warned_dot[graphviz_dot] = True  # type: ignore[attr-defined]
+        self.builder._graphviz_warned_dot[graphviz_dot] = True
         return None, None
     except CalledProcessError as exc:
         raise GraphvizError(__('dot exited with error:\n[stderr]\n%r\n'

sphinx/ext/inheritance_diagram.py

@@ -36,6 +36,7 @@ import inspect
 import re
 from collections.abc import Iterable, Sequence
 from importlib import import_module
+from os import path
 from typing import TYPE_CHECKING, Any, cast
 
 from docutils import nodes
@@ -417,7 +418,7 @@ def html_visit_inheritance_diagram(self: HTML5Translator, node: inheritance_diag
 
     # Create a mapping from fully-qualified class names to URLs.
     graphviz_output_format = self.builder.env.config.graphviz_output_format.upper()
-    current_filename = self.builder.current_docname + self.builder.out_suffix
+    current_filename = path.basename(self.builder.current_docname + self.builder.out_suffix)
     urls = {}
     pending_xrefs = cast(Iterable[addnodes.pending_xref], node)
     for child in pending_xrefs:

tests/roots/test-ext-inheritance_diagram/index.rst

@@ -7,12 +7,12 @@ test-ext-inheritance_diagram
 .. inheritance-diagram:: test.Foo
    :caption: Test Foo!
 
-.. inheritance-diagram:: test.DocLowerLevel
+.. inheritance-diagram:: test.DocSubDir2
 
 .. py:class:: test.DocHere
 
 .. py:class:: test.DocMainLevel
 
-.. inheritance-diagram:: subdir.other.Bob
+.. inheritance-diagram:: external.other.Bob
 
 .. py:class:: test.Alice

tests/roots/test-ext-inheritance_diagram/subdir/index.rst

@@ -1,7 +0,0 @@
-=========================================
-test-ext-inheritance_diagram subdirectory
-=========================================
-
-.. inheritance-diagram:: test.DocMainLevel
-
-.. py:class:: test.DocLowerLevel

tests/roots/test-ext-inheritance_diagram/subdir/page1.rst

@@ -0,0 +1,9 @@
+================================================
+test-ext-inheritance_diagram subdirectory page 1
+================================================
+
+.. inheritance-diagram:: test.DocMainLevel
+
+.. inheritance-diagram:: test.DocSubDir2
+
+.. py:class:: test.DocSubDir1

tests/roots/test-ext-inheritance_diagram/subdir/page2.rst

@@ -0,0 +1,5 @@
+================================================
+test-ext-inheritance_diagram subdirectory page 2
+================================================
+
+.. py:class:: test.DocSubDir2

tests/roots/test-ext-inheritance_diagram/test.py

@@ -6,7 +6,11 @@ class DocHere(Foo):
     pass
 
 
-class DocLowerLevel(DocHere):
+class DocSubDir1(DocHere):
+    pass
+
+
+class DocSubDir2(DocSubDir1):
     pass
 
 

tests/test_ext_inheritance_diagram.py

@@ -138,13 +138,13 @@ def test_inheritance_diagram(app, status, warning):
 
 
 # An external inventory to test intersphinx links in inheritance diagrams
-subdir_inventory = b'''\
+external_inventory = b'''\
 # Sphinx inventory version 2
-# Project: subdir
+# Project: external
 # Version: 1.0
 # The remainder of this file is compressed using zlib.
 ''' + zlib.compress(b'''\
-subdir.other.Bob py:class 1 foo.html#subdir.other.Bob -
+external.other.Bob py:class 1 foo.html#external.other.Bob -
 ''')
 
 
@@ -152,7 +152,7 @@ subdir.other.Bob py:class 1 foo.html#subdir.other.Bob -
 @pytest.mark.usefixtures('if_graphviz_found')
 def test_inheritance_diagram_png_html(tmp_path, app):
     inv_file = tmp_path / 'inventory'
-    inv_file.write_bytes(subdir_inventory)
+    inv_file.write_bytes(external_inventory)
     app.config.intersphinx_mapping = {
         'https://example.org': str(inv_file),
     }
@@ -173,7 +173,7 @@ def test_inheritance_diagram_png_html(tmp_path, app):
                'title="Link to this image">\xb6</a></p>\n</figcaption>\n</figure>\n')
     assert re.search(pattern, content, re.M)
 
-    subdir_content = (app.outdir / 'subdir/index.html').read_text(encoding='utf8')
+    subdir_content = (app.outdir / 'subdir/page1.html').read_text(encoding='utf8')
     subdir_maps = re.findall('<map .+\n.+\n</map>', subdir_content)
     subdir_maps = [re.sub('href="(\\S+)"', 'href="subdir/\\g<1>"', s) for s in subdir_maps]
 
@@ -199,7 +199,7 @@ def test_inheritance_diagram_png_html(tmp_path, app):
 @pytest.mark.usefixtures('if_graphviz_found')
 def test_inheritance_diagram_svg_html(tmp_path, app):
     inv_file = tmp_path / 'inventory'
-    inv_file.write_bytes(subdir_inventory)
+    inv_file.write_bytes(external_inventory)
     app.config.intersphinx_mapping = {
         "subdir": ('https://example.org', str(inv_file)),
     }
@@ -223,7 +223,7 @@ def test_inheritance_diagram_svg_html(tmp_path, app):
 
     assert re.search(pattern, content, re.M)
 
-    subdir_content = (app.outdir / 'subdir/index.html').read_text(encoding='utf8')
+    subdir_content = (app.outdir / 'subdir/page1.html').read_text(encoding='utf8')
     subdir_svgs = re.findall('<object data="../(_images/inheritance-\\w+.svg?)"', subdir_content)
 
     # Go through every SVG inheritance diagram
@@ -268,8 +268,9 @@ def test_inheritance_diagram_latex_alias(app, status, warning):
 
     doc = app.env.get_and_resolve_doctree('index', app)
     aliased_graph = doc.children[0].children[3]['graph'].class_info
-    assert len(aliased_graph) == 3
+    assert len(aliased_graph) == 4
-    assert ('test.DocLowerLevel', 'test.DocLowerLevel', ['test.DocHere'], None) in aliased_graph
+    assert ('test.DocSubDir2', 'test.DocSubDir2', ['test.DocSubDir1'], None) in aliased_graph
+    assert ('test.DocSubDir1', 'test.DocSubDir1', ['test.DocHere'], None) in aliased_graph
     assert ('test.DocHere', 'test.DocHere', ['alias.Foo'], None) in aliased_graph
     assert ('alias.Foo', 'alias.Foo', [], None) in aliased_graph