Issue #25791: Raise an ImportWarning when __spec__ or __package__ are

brettcannon · brettcannon · commit 63b8505281f9 · 2016-01-15T13:33:03.000-08:00
not defined for a relative import.

This is the start of work to try and clean up import semantics to rely
more on a module's spec than on the myriad attributes that get set on
a module. Thanks to Rose Ames for the patch.
diff --git a/Doc/reference/import.rst b/Doc/reference/import.rst
@@ -554,7 +554,8 @@ the module.
    details.
 
    This attribute is used instead of ``__name__`` to calculate explicit
-   relative imports for main modules, as defined in :pep:`366`.
+   relative imports for main modules -- as defined in :pep:`366` --
+   when ``__spec__`` is not defined.
 
 .. attribute:: __spec__
 
diff --git a/Doc/whatsnew/3.6.rst b/Doc/whatsnew/3.6.rst
@@ -209,7 +209,12 @@ Deprecated features
 * The ``pyvenv`` script has been deprecated in favour of ``python3 -m venv``.
   This prevents confusion as to what Python interpreter ``pyvenv`` is
   connected to and thus what Python interpreter will be used by the virtual
-  environment. See :issue:`25154`.
+  environment. (Contributed by Brett Cannon in :issue:`25154`.)
+
+* When performing a relative import, falling back on ``__name__`` and
+  ``__path__`` from the calling module when ``__spec__`` or
+  ``__package__`` are not defined now raises an :exc:`ImportWarning`.
+  (Contributed by Rose Ames in :issue:`25791`.)
 
 
 Removed
@@ -251,6 +256,10 @@ Changes in the Python API
   :mod:`wave`.  This means they will export new symbols when ``import *``
   is used.  See :issue:`23883`.
 
+* When performing a relative import, ``__spec__.parent`` is used
+  is ``__spec__`` is defined instead of ``__package__``.
+  (Contributed by Rose Ames in :issue:`25791`.)
+
 
 Changes in the C API
 --------------------
diff --git a/Lib/importlib/_bootstrap.py b/Lib/importlib/_bootstrap.py
@@ -1032,8 +1032,14 @@ def _calc___package__(globals):
     to represent that its proper value is unknown.
 
     """
+    spec = globals.get('__spec__')
+    if spec is not None:
+        return spec.parent
     package = globals.get('__package__')
     if package is None:
+        _warnings.warn("can't resolve package from __spec__ or __package__, "
+                       "falling back on __name__ and __path__",
+                       ImportWarning, stacklevel=3)
         package = globals['__name__']
         if '__path__' not in globals:
             package = package.rpartition('.')[0]
diff --git a/Lib/test/test_importlib/import_/test___package__.py b/Lib/test/test_importlib/import_/test___package__.py
@@ -33,31 +33,39 @@ def calc_package(caller_name, has___path__):
 
     """
 
-    def test_using___package__(self):
-        # [__package__]
+    def import_module(self, globals_):
         with self.mock_modules('pkg.__init__', 'pkg.fake') as importer:
             with util.import_state(meta_path=[importer]):
                 self.__import__('pkg.fake')
                 module = self.__import__('',
-                                            globals={'__package__': 'pkg.fake'},
+                                            globals=globals_,
                                             fromlist=['attr'], level=2)
+        return module
+
+    def test_using___package__(self):
+        # [__package__]
+        module = self.import_module({'__package__': 'pkg.fake'})
         self.assertEqual(module.__name__, 'pkg')
 
-    def test_using___name__(self, package_as_None=False):
+    def test_using___name__(self):
         # [__name__]
-        globals_ = {'__name__': 'pkg.fake', '__path__': []}
-        if package_as_None:
-            globals_['__package__'] = None
-        with self.mock_modules('pkg.__init__', 'pkg.fake') as importer:
-            with util.import_state(meta_path=[importer]):
-                self.__import__('pkg.fake')
-                module = self.__import__('', globals= globals_,
-                                                fromlist=['attr'], level=2)
-            self.assertEqual(module.__name__, 'pkg')
+        module = self.import_module({'__name__': 'pkg.fake', '__path__': []})
+        self.assertEqual(module.__name__, 'pkg')
+
+    def test_warn_when_using___name__(self):
+        with self.assertWarns(ImportWarning):
+            self.import_module({'__name__': 'pkg.fake', '__path__': []})
 
     def test_None_as___package__(self):
         # [None]
-        self.test_using___name__(package_as_None=True)
+        module = self.import_module({
+            '__name__': 'pkg.fake', '__path__': [], '__package__': None })
+        self.assertEqual(module.__name__, 'pkg')
+
+    def test_prefers___spec__(self):
+        globals = {'__spec__': FakeSpec()}
+        with self.assertRaises(SystemError):
+            self.__import__('', globals, {}, ['relimport'], 1)
 
     def test_bad__package__(self):
         globals = {'__package__': '<not real>'}
@@ -70,6 +78,10 @@ def test_bunk__package__(self):
             self.__import__('', globals, {}, ['relimport'], 1)
 
 
+class FakeSpec:
+    parent = '<fake>'
+
+
 class Using__package__PEP302(Using__package__):
     mock_modules = util.mock_modules
 
diff --git a/Misc/NEWS b/Misc/NEWS
@@ -10,6 +10,9 @@ Release date: tba
 Core and Builtins
 -----------------
 
+- Issue #25791: Trying to resolve a relative import without __spec__ or
+  __package__ defined now raises an ImportWarning
+
 - Issue #25961: Disallowed null characters in the type name.
 
 - Issue #25973: Fix segfault when an invalid nonlocal statement binds a name
diff --git a/Python/import.c b/Python/import.c
@@ -1359,6 +1359,7 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals,
     PyObject *final_mod = NULL;
     PyObject *mod = NULL;
     PyObject *package = NULL;
+    PyObject *spec = NULL;
     PyObject *globals = NULL;
     PyObject *fromlist = NULL;
     PyInterpreterState *interp = PyThreadState_GET()->interp;
@@ -1414,39 +1415,60 @@ PyImport_ImportModuleLevelObject(PyObject *name, PyObject *given_globals,
         goto error;
     }
     else if (level > 0) {
-        package = _PyDict_GetItemId(globals, &PyId___package__);
+        spec = _PyDict_GetItemId(globals, &PyId___spec__);
+        if (spec != NULL) {
+            package = PyObject_GetAttrString(spec, "parent");
+        }
         if (package != NULL && package != Py_None) {
-            Py_INCREF(package);
             if (!PyUnicode_Check(package)) {
-                PyErr_SetString(PyExc_TypeError, "package must be a string");
+                PyErr_SetString(PyExc_TypeError,
+                        "__spec__.parent must be a string");
                 goto error;
             }
         }
         else {
-            package = _PyDict_GetItemId(globals, &PyId___name__);
-            if (package == NULL) {
-                PyErr_SetString(PyExc_KeyError, "'__name__' not in globals");
-                goto error;
-            }
-            else if (!PyUnicode_Check(package)) {
-                PyErr_SetString(PyExc_TypeError, "__name__ must be a string");
+            package = _PyDict_GetItemId(globals, &PyId___package__);
+            if (package != NULL && package != Py_None) {
+                Py_INCREF(package);
+                if (!PyUnicode_Check(package)) {
+                    PyErr_SetString(PyExc_TypeError, "package must be a string");
+                    goto error;
+                }
             }
-            Py_INCREF(package);
-
-            if (_PyDict_GetItemId(globals, &PyId___path__) == NULL) {
-                PyObject *partition = NULL;
-                PyObject *borrowed_dot = _PyUnicode_FromId(&single_dot);
-                if (borrowed_dot == NULL) {
+            else {
+                if (PyErr_WarnEx(PyExc_ImportWarning,
+                            "can't resolve package from __spec__ or __package__, "
+                            "falling back on __name__ and __path__", 1) < 0) {
                     goto error;
                 }
-                partition = PyUnicode_RPartition(package, borrowed_dot);
-                Py_DECREF(package);
-                if (partition == NULL) {
+
+                package = _PyDict_GetItemId(globals, &PyId___name__);
+                if (package == NULL) {
+                    PyErr_SetString(PyExc_KeyError, "'__name__' not in globals");
                     goto error;
                 }
-                package = PyTuple_GET_ITEM(partition, 0);
+
                 Py_INCREF(package);
-                Py_DECREF(partition);
+                if (!PyUnicode_Check(package)) {
+                    PyErr_SetString(PyExc_TypeError, "__name__ must be a string");
+                    goto error;
+                }
+
+                if (_PyDict_GetItemId(globals, &PyId___path__) == NULL) {
+                    PyObject *partition = NULL;
+                    PyObject *borrowed_dot = _PyUnicode_FromId(&single_dot);
+                    if (borrowed_dot == NULL) {
+                        goto error;
+                    }
+                    partition = PyUnicode_RPartition(package, borrowed_dot);
+                    Py_DECREF(package);
+                    if (partition == NULL) {
+                        goto error;
+                    }
+                    package = PyTuple_GET_ITEM(partition, 0);
+                    Py_INCREF(package);
+                    Py_DECREF(partition);
+                }
             }
         }
 
diff --git a/Python/importlib.h b/Python/importlib.h
