Disallow module cycles in autosummary (#6792)

Consider the following piece of reST::

  .. automodule:: sphinx.ext.autosummary
     :members:

     .. autosummary::

        sphinx.ext.autosummary.Autosummary

This inserts an autosummary after the module docstring, but before
the members of the module. Without the change in this commit, this
would fail because `import_by_name` would attempt to import::

    sphinx.ext.autosummary.sphinx.ext.autosummary.Autosumary

because the prefix (from the parent) is `sphinx.ext.autosummary`,
and the name is `sphinx.ext.autosummary.Autosummary`, which is able
to be imported from `sphinx.ext.autosummary`, but is not the way
that anyone would want to refer to it.

Co-authored-by: Adam Turner <[email protected]>

Author: tbekolay

CHANGES.rst

@@ -80,6 +80,8 @@ Features added
 * #7896, #11989: Add a :rst:dir:`py:type` directiv for documenting type aliases,
   and a :rst:role:`py:type` role for linking to them.
   Patch by Ashley Whetter.
+* #6792: Prohibit module import cycles in :mod:`sphinx.ext.autosummary`.
+  Patch by Trevor Bekolay.
 
 Bugs fixed
 ----------

doc/usage/configuration.rst

@@ -1383,6 +1383,7 @@ Options for warning control
    * ``autodoc.import_object``
    * ``autosectionlabel.<document name>``
    * ``autosummary``
+   * ``autosummary.import_cycle``
    * ``intersphinx.external``
 
    You can choose from these types.  You can also give only the first

sphinx/ext/autosummary/__init__.py

@@ -639,6 +639,13 @@ def import_by_name(
     tried = []
     errors: list[ImportExceptionGroup] = []
     for prefix in prefixes:
+        if prefix is not None and name.startswith(f'{prefix}.'):
+            # Catch and avoid module cycles (e.g., sphinx.ext.sphinx.ext...)
+            msg = __('Summarised items should not include the current module. '
+                     'Replace %r with %r.')
+            logger.warning(msg, name, name.removeprefix(f'{prefix}.'),
+                           type='autosummary', subtype='import_cycle')
+            continue
         try:
             if prefix:
                 prefixed_name = f'{prefix}.{name}'

tests/roots/test-ext-autosummary-import_cycle/conf.py

@@ -0,0 +1,7 @@
+import os
+import sys
+
+sys.path.insert(0, os.path.abspath('.'))
+
+extensions = ['sphinx.ext.autosummary']
+autosummary_generate = False

tests/roots/test-ext-autosummary-import_cycle/index.rst

@@ -0,0 +1,6 @@
+.. automodule:: spam.eggs
+   :members:
+
+   .. autosummary::
+
+      spam.eggs.Ham

tests/roots/test-ext-autosummary-import_cycle/spam/__init__.py

@@ -0,0 +1 @@
+"""``spam`` module docstring."""

tests/roots/test-ext-autosummary-import_cycle/spam/eggs.py

@@ -0,0 +1,10 @@
+"""``spam.eggs`` module docstring."""
+
+import spam  # Required for test.
+
+
+class Ham:
+    """``spam.eggs.Ham`` class docstring."""
+    a = 1
+    b = 2
+    c = 3

tests/test_extensions/test_ext_autosummary_imports.py

@@ -0,0 +1,40 @@
+"""Test autosummary for import cycles."""
+
+import pytest
+from docutils import nodes
+
+from sphinx import addnodes
+from sphinx.ext.autosummary import autosummary_table
+from sphinx.testing.util import assert_node
+
+
+@pytest.mark.sphinx('dummy', testroot='ext-autosummary-import_cycle')
+@pytest.mark.usefixtures("rollback_sysmodules")
+def test_autosummary_import_cycle(app, warning):
+    app.build()
+
+    doctree = app.env.get_doctree('index')
+    app.env.apply_post_transforms(doctree, 'index')
+
+    assert len(list(doctree.findall(nodes.reference))) == 1
+
+    assert_node(doctree,
+                (addnodes.index,  # [0]
+                 nodes.target,  # [1]
+                 nodes.paragraph,  # [2]
+                 addnodes.tabular_col_spec,  # [3]
+                 [autosummary_table, nodes.table, nodes.tgroup, (nodes.colspec,  # [4][0][0][0]
+                                                                 nodes.colspec,  # [4][0][0][1]
+                                                                 [nodes.tbody, nodes.row])],  # [4][0][0][2][1]
+                 addnodes.index,  # [5]
+                 addnodes.desc))  # [6]
+    assert_node(doctree[4][0][0][2][0],
+                ([nodes.entry, nodes.paragraph, (nodes.reference, nodes.Text)], nodes.entry))
+    assert_node(doctree[4][0][0][2][0][0][0][0], nodes.reference,
+                refid='spam.eggs.Ham', reftitle='spam.eggs.Ham')
+
+    expected = (
+        "Summarised items should not include the current module. "
+        "Replace 'spam.eggs.Ham' with 'Ham'."
+    )
+    assert expected in app.warning.getvalue()

tests/test_extensions/test_ext_viewcode.py

@@ -42,6 +42,7 @@ def check_viewcode_output(app, warning):
 
 @pytest.mark.sphinx(testroot='ext-viewcode', freshenv=True,
                     confoverrides={"viewcode_line_numbers": True})
+@pytest.mark.usefixtures("rollback_sysmodules")
 def test_viewcode_linenos(app, warning):
     shutil.rmtree(app.outdir / '_modules', ignore_errors=True)
     app.build(force_all=True)
@@ -52,6 +53,7 @@ def test_viewcode_linenos(app, warning):
 
 @pytest.mark.sphinx(testroot='ext-viewcode', freshenv=True,
                     confoverrides={"viewcode_line_numbers": False})
+@pytest.mark.usefixtures("rollback_sysmodules")
 def test_viewcode(app, warning):
     shutil.rmtree(app.outdir / '_modules', ignore_errors=True)
     app.build(force_all=True)
@@ -61,6 +63,7 @@ def test_viewcode(app, warning):
 
 
 @pytest.mark.sphinx('epub', testroot='ext-viewcode')
+@pytest.mark.usefixtures("rollback_sysmodules")
 def test_viewcode_epub_default(app, status, warning):
     shutil.rmtree(app.outdir)
     app.build(force_all=True)
@@ -73,6 +76,7 @@ def test_viewcode_epub_default(app, status, warning):
 
 @pytest.mark.sphinx('epub', testroot='ext-viewcode',
                     confoverrides={'viewcode_enable_epub': True})
+@pytest.mark.usefixtures("rollback_sysmodules")
 def test_viewcode_epub_enabled(app, status, warning):
     app.build(force_all=True)