bpo-44717: improve AttributeError on circular imports of submodules (p…

…ythonGH-27299) Signed-off-by: Filipe Laíns <[email protected]> Co-authored-by: Łukasz Langa <[email protected]>
akulakov · Jul 24, 2021 · 8072a11 · 8072a11
1 parent 5370f0a
commit 8072a11
Show file tree

Hide file tree

Showing 8 changed files with 1,807 additions and 1,734 deletions.
diff --git a/Lib/importlib/_bootstrap.py b/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:

diff --git a/Lib/test/test_import/__init__.py b/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")

diff --git a/Lib/test/test_import/data/circular_imports/subpkg2/__init__.py b/Lib/test/test_import/data/circular_imports/subpkg2/__init__.py
diff --git a/Lib/test/test_import/data/circular_imports/subpkg2/parent/__init__.py b/Lib/test/test_import/data/circular_imports/subpkg2/parent/__init__.py
@@ -0,0 +1 @@
+import test.test_import.data.circular_imports.subpkg2.parent.child
diff --git a/Lib/test/test_import/data/circular_imports/subpkg2/parent/child.py b/Lib/test/test_import/data/circular_imports/subpkg2/parent/child.py
@@ -0,0 +1,3 @@
+import test.test_import.data.circular_imports.subpkg2.parent
+
+test.test_import.data.circular_imports.subpkg2.parent
diff --git a/Misc/NEWS.d/next/Core and Builtins/2021-07-23-01-52-13.bpo-44717.-vVmAh.rst b/Misc/NEWS.d/next/Core and Builtins/2021-07-23-01-52-13.bpo-44717.-vVmAh.rst
@@ -0,0 +1 @@
+Improve AttributeError on circular imports of submodules.
diff --git a/Objects/moduleobject.c b/Objects/moduleobject.c
@@ -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'",