Add warning for implicit relative imports issues

Lib/test/test_py3kwarn.py

@@ -1,9 +1,11 @@
 import unittest
 import sys
+import os
 from test.test_support import check_py3k_warnings, CleanImport, run_unittest
 import warnings
 import base64
 from test import test_support
+from test import script_helper
 
 if not sys.py3kwarning:
     raise unittest.SkipTest('%s must be run with the -3 flag' % __name__)
@@ -41,6 +43,43 @@ def assertWarning(self, _, warning, expected_message):
     def assertNoWarning(self, _, recorder):
         self.assertEqual(len(recorder.warnings), 0)
 
+    def _write_file(self, path, contents):
+        with open(path, 'w') as f:
+            f.write(contents)
+
+    def _check_import_order_warning(self, importer_source, expected_message=None,
+                                    imported_name='foo',
+                                    top_source="WHO = 'TOP_LEVEL_FOO'\n",
+                                    sibling_source="WHO = 'PKG_FOO'\n",
+                                    package=True):
+        with test_support.temp_dir() as tmp:
+            if top_source is not None:
+                self._write_file(os.path.join(tmp, imported_name + '.py'),
+                                 top_source)
+
+            if package:
+                pkg_dir = os.path.join(tmp, 'pkg')
+                os.mkdir(pkg_dir)
+                self._write_file(os.path.join(pkg_dir, '__init__.py'), '')
+                if sibling_source is not None:
+                    self._write_file(os.path.join(pkg_dir, imported_name + '.py'),
+                                     sibling_source)
+                import_target = 'pkg.importer'
+                importer_path = os.path.join(pkg_dir, 'importer.py')
+            else:
+                import_target = 'importer'
+                importer_path = os.path.join(tmp, 'importer.py')
+
+            self._write_file(importer_path, importer_source)
+            rc, out, err = script_helper.assert_python_ok(
+                '-S', '-3', '-c', 'import %s' % import_target, PYTHONPATH=tmp)
+            self.assertEqual(rc, 0)
+            self.assertEqual(out, '')
+            if expected_message is None:
+                self.assertNotIn('implicit relative import', err)
+            else:
+                self.assertEqual(err.count(expected_message), 1)
+
     def test_backquote(self):
         expected = 'backquote not supported in 3.x; use repr()'
         with check_py3k_warnings((expected, SyntaxWarning)):
@@ -429,6 +468,48 @@ def test_b16encode_warns(self):
         expected = "base64.b16encode returns str in Python 2 (bytes in 3.x)"
         base64.b16encode(b'test')
         check_py3k_warnings(expected, UserWarning)
+
+    def test_import_order_implicit_import_local_sibling(self):
+        expected = ("implicit relative import of 'foo' resolved to package "
+                    "sibling 'pkg.foo'; in 3.x imports are absolute by "
+                    "default and this will resolve differently or fail: "
+                    "use 'from . import foo' if the package sibling is "
+                    "intended")
+        self._check_import_order_warning("import foo\n", expected)
+
+    def test_import_order_implicit_from_import_local_sibling(self):
+        expected = ("implicit relative import from 'foo' resolved to package "
+                    "sibling 'pkg.foo'; in 3.x imports are absolute by "
+                    "default and this will resolve differently or fail: "
+                    "use 'from .foo import ...' if the package sibling is "
+                    "intended")
+        self._check_import_order_warning("from foo import WHO\n", expected)
+
+    def test_import_order_implicit_import_stdlib_name_conflict(self):
+        expected = ("implicit relative import of 'string' resolved to package "
+                    "sibling 'pkg.string'; in 3.x imports are absolute by "
+                    "default and this will resolve differently or fail: "
+                    "use 'from . import string' if the package sibling is "
+                    "intended")
+        self._check_import_order_warning("import string\n",
+                                         expected_message=expected,
+                                         imported_name='string',
+                                         top_source=None,
+                                         sibling_source="WHO = 'PKG_STRING'\n")
+
+    def test_import_order_future_absolute_import_is_not_warned(self):
+        self._check_import_order_warning(
+            "from __future__ import absolute_import\nimport foo\n")
+
+    def test_import_order_explicit_relative_import_is_not_warned(self):
+        self._check_import_order_warning(
+            "from __future__ import absolute_import\nfrom . import foo\n")
+
+    def test_import_order_top_level_script_is_not_warned(self):
+        self._check_import_order_warning("import foo\n", package=False)
+
+    def test_import_order_absolute_import_without_sibling_is_not_warned(self):
+        self._check_import_order_warning("import foo\n", sibling_source=None)
 
 
 class TestStdlibRemovals(unittest.TestCase):

Python/import.c

@@ -2236,6 +2236,10 @@ static int mark_miss(char *name);
 static int ensure_fromlist(PyObject *mod, PyObject *fromlist,
                            char *buf, Py_ssize_t buflen, int recursive);
 static PyObject * import_submodule(PyObject *mod, char *name, char *fullname);
+static int warn_implicit_relative_sibling(PyObject *module,
+                                          const char *imported_name,
+                                          const char *package_name,
+                                          int from_import);
 
 /* The Magnum Opus of dotted-name import :-) */
 
@@ -2246,6 +2250,10 @@ import_module_level(char *name, PyObject *globals, PyObject *locals,
     char *buf;
     Py_ssize_t buflen = 0;
     PyObject *parent, *head, *next, *tail;
+    int from_import = 0;
+    int warn_if_relative_sibling = 0;
+    char package_name[MAXPATHLEN + 1];
+    char imported_name[MAXPATHLEN + 1];
 
     if (strchr(name, '/') != NULL
 #ifdef MS_WINDOWS
@@ -2265,6 +2273,13 @@ import_module_level(char *name, PyObject *globals, PyObject *locals,
     if (parent == NULL)
         goto error_exit;
 
+    if (level < 0 && parent != Py_None && strchr(name, '.') == NULL &&
+        buflen < sizeof(package_name) && strlen(name) < sizeof(imported_name)) {
+        strcpy(package_name, buf);
+        strcpy(imported_name, name);
+        warn_if_relative_sibling = 1;
+    }
+
     Py_INCREF(parent);
     head = load_next(parent, level < 0 ? Py_None : parent, &name, buf,
                         &buflen);
@@ -2303,6 +2318,17 @@ import_module_level(char *name, PyObject *globals, PyObject *locals,
         }
         if (!b)
             fromlist = NULL;
+        else
+            from_import = 1;
+    }
+
+    if (warn_if_relative_sibling) {
+        if (warn_implicit_relative_sibling(head, imported_name,
+                                           package_name, from_import) < 0) {
+            Py_DECREF(tail);
+            Py_DECREF(head);
+            goto error_exit;
+        }
     }
 
     if (fromlist == NULL) {
@@ -2325,6 +2351,65 @@ import_module_level(char *name, PyObject *globals, PyObject *locals,
     return NULL;
 }
 
+static int
+warn_implicit_relative_sibling(PyObject *module, const char *imported_name,
+                               const char *package_name, int from_import)
+{
+    PyObject *msg = NULL;
+    PyObject *fix = NULL;
+    const char *resolved_name;
+    size_t package_name_len;
+    int result;
+
+    if (!Py_Py3kWarningFlag || module == NULL || !PyModule_Check(module))
+        return 0;
+    if (imported_name == NULL || package_name == NULL)
+        return 0;
+
+    resolved_name = PyModule_GetName(module);
+    if (resolved_name == NULL) {
+        PyErr_Clear();
+        return 0;
+    }
+
+    package_name_len = strlen(package_name);
+    if (strncmp(resolved_name, package_name, package_name_len) != 0 ||
+        resolved_name[package_name_len] != '.' ||
+        strcmp(resolved_name + package_name_len + 1, imported_name) != 0)
+        return 0;
+
+    if (from_import) {
+        msg = PyString_FromFormat(
+            "implicit relative import from '%.200s' resolved to package sibling '%.200s'; "
+            "in 3.x imports are absolute by default and this will resolve differently or fail",
+            imported_name, resolved_name);
+        fix = PyString_FromFormat(
+            "use 'from .%.200s import ...' if the package sibling is intended",
+            imported_name);
+    }
+    else {
+        msg = PyString_FromFormat(
+            "implicit relative import of '%.200s' resolved to package sibling '%.200s'; "
+            "in 3.x imports are absolute by default and this will resolve differently or fail",
+            imported_name, resolved_name);
+        fix = PyString_FromFormat(
+            "use 'from . import %.200s' if the package sibling is intended",
+            imported_name);
+    }
+    if (msg == NULL || fix == NULL) {
+        Py_XDECREF(msg);
+        Py_XDECREF(fix);
+        return -1;
+    }
+
+    result = PyErr_WarnEx_WithFix(PyExc_DeprecationWarning,
+                                  PyString_AsString(msg),
+                                  PyString_AsString(fix), 1);
+    Py_DECREF(msg);
+    Py_DECREF(fix);
+    return result;
+}
+
 PyObject *
 PyImport_ImportModuleLevel(char *name, PyObject *globals, PyObject *locals,
                          PyObject *fromlist, int level)