bpo-44717: improve AttributeError on circular imports of submodules (GH-27299)

Signed-off-by: Filipe Laíns <lains@riseup.net>

Co-authored-by: Łukasz Langa <lukasz@langa.pl>

Author: FFY00

Lib/importlib/_bootstrap.py

@@ -361,6 +361,7 @@ def __init__(self, name, loader, *, origin=None, loader_state=None,
         self.origin = origin
         self.loader_state = loader_state
         self.submodule_search_locations = [] if is_package else None
+        self._uninitialized_submodules = []
 
         # file-location attributes
         self._set_fileattr = False
@@ -987,6 +988,7 @@ def _sanity_check(name, package, level):
 def _find_and_load_unlocked(name, import_):
     path = None
     parent = name.rpartition('.')[0]
+    parent_spec = None
     if parent:
         if parent not in sys.modules:
             _call_with_frames_removed(import_, parent)
@@ -999,15 +1001,24 @@ def _find_and_load_unlocked(name, import_):
         except AttributeError:
             msg = (_ERR_MSG + '; {!r} is not a package').format(name, parent)
             raise ModuleNotFoundError(msg, name=name) from None
+        parent_spec = parent_module.__spec__
+        child = name.rpartition('.')[2]
     spec = _find_spec(name, path)
     if spec is None:
         raise ModuleNotFoundError(_ERR_MSG.format(name), name=name)
     else:
-        module = _load_unlocked(spec)
+        if parent_spec:
+            # Temporarily add child we are currently importing to parent's
+            # _uninitialized_submodules for circular import tracking.
+            parent_spec._uninitialized_submodules.append(child)
+        try:
+            module = _load_unlocked(spec)
+        finally:
+            if parent_spec:
+                parent_spec._uninitialized_submodules.pop()
     if parent:
         # Set the module as an attribute on its parent.
         parent_module = sys.modules[parent]
-        child = name.rpartition('.')[2]
         try:
             setattr(parent_module, child, module)
         except AttributeError:

Lib/test/test_import/__init__.py

@@ -1350,6 +1350,16 @@ def test_circular_from_import(self):
             str(cm.exception),
         )
 
+    def test_absolute_circular_submodule(self):
+        with self.assertRaises(AttributeError) as cm:
+            import test.test_import.data.circular_imports.subpkg2.parent
+        self.assertIn(
+            "cannot access submodule 'parent' of module "
+            "'test.test_import.data.circular_imports.subpkg2' "
+            "(most likely due to a circular import)",
+            str(cm.exception),
+        )
+
     def test_unwritable_module(self):
         self.addCleanup(unload, "test.test_import.data.unwritable")
         self.addCleanup(unload, "test.test_import.data.unwritable.x")

Lib/test/test_import/data/circular_imports/subpkg2/__init__.py

@@ -0,0 +1 @@
+import test.test_import.data.circular_imports.subpkg2.parent.child

Lib/test/test_import/data/circular_imports/subpkg2/parent/__init__.py

@@ -0,0 +1,3 @@
+import test.test_import.data.circular_imports.subpkg2.parent
+
+test.test_import.data.circular_imports.subpkg2.parent

Lib/test/test_import/data/circular_imports/subpkg2/parent/child.py

@@ -0,0 +1 @@
+Improve AttributeError on circular imports of submodules.

Misc/NEWS.d/next/Core and Builtins/2021-07-23-01-52-13.bpo-44717.-vVmAh.rst

@@ -739,6 +739,30 @@ _PyModuleSpec_IsInitializing(PyObject *spec)
     return 0;
 }
 
+/* Check if the submodule name is in the "_uninitialized_submodules" attribute
+   of the module spec.
+ */
+int
+_PyModuleSpec_IsUninitializedSubmodule(PyObject *spec, PyObject *name)
+{
+    if (spec == NULL) {
+         return 0;
+    }
+
+    _Py_IDENTIFIER(_uninitialized_submodules);
+    PyObject *value = _PyObject_GetAttrId(spec, &PyId__uninitialized_submodules);
+    if (value == NULL) {
+        return 0;
+    }
+
+    int is_uninitialized = PySequence_Contains(value, name);
+    Py_DECREF(value);
+    if (is_uninitialized == -1) {
+        return 0;
+    }
+    return is_uninitialized;
+}
+
 static PyObject*
 module_getattro(PyModuleObject *m, PyObject *name)
 {
@@ -773,6 +797,12 @@ module_getattro(PyModuleObject *m, PyObject *name)
                             "(most likely due to a circular import)",
                             mod_name, name);
         }
+        else if (_PyModuleSpec_IsUninitializedSubmodule(spec, name)) {
+            PyErr_Format(PyExc_AttributeError,
+                            "cannot access submodule '%U' of module '%U' "
+                            "(most likely due to a circular import)",
+                            name, mod_name);
+        }
         else {
             PyErr_Format(PyExc_AttributeError,
                             "module '%U' has no attribute '%U'",