add error for shadowing

Include/internal/pycore_moduleobject.h

@@ -12,6 +12,7 @@ extern void _PyModule_Clear(PyObject *);
 extern void _PyModule_ClearDict(PyObject *);
 extern int _PyModuleSpec_IsInitializing(PyObject *);
 extern int _PyModuleSpec_GetFileOrigin(PyObject *, PyObject **);
+extern int _PyModule_IsPossiblyShadowing(PyObject *);
 
 extern int _PyModule_IsExtension(PyObject *obj);
 

Lib/test/test_import/__init__.py

@@ -786,49 +786,61 @@ def test_issue105979(self):
  str(cm.exception))
 
  def test_script_shadowing_stdlib(self):
- with os_helper.temp_dir() as tmp:
- with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f:
- f.write("import fractions\nfractions.Fraction")
-
- expected_error = (
- rb"AttributeError: module 'fractions' has no attribute 'Fraction' "
- rb"\(consider renaming '.*fractions.py' since it has the "
- rb"same name as the standard library module named 'fractions' "
- rb"and the import system gives it precedence\)"
+ script_errors = [
+ (
+ "import fractions\nfractions.Fraction",
+ rb"AttributeError: module 'fractions' has no attribute 'Fraction'"
+ ),
+ (
+ "from fractions import Fraction",
+ rb"ImportError: cannot import name 'Fraction' from 'fractions'"
  )
+ ]
 
- popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp)
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
 
- popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp)
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
+ for script, error in script_errors:
+ with os_helper.temp_dir() as tmp:
+ with open(os.path.join(tmp, "fractions.py"), "w", encoding='utf-8') as f:
+ f.write(script)
 
- popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp)
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
+ expected_error = error + (
+ rb" \(consider renaming '.*fractions.py' since it has the "
+ rb"same name as the standard library module named 'fractions' "
+ rb"and the import system gives it precedence\)"
+ )
 
- # and there's no error at all when using -P
- popen = script_helper.spawn_python('-P', 'fractions.py', cwd=tmp)
- stdout, stderr = popen.communicate()
- self.assertEqual(stdout, b'')
+ popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp)
+ stdout, stderr = popen.communicate()
+ self.assertRegex(stdout, expected_error)
 
- tmp_child = os.path.join(tmp, "child")
- os.mkdir(tmp_child)
+ popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp)
+ stdout, stderr = popen.communicate()
+ self.assertRegex(stdout, expected_error)
 
- # test the logic with different cwd
- popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp_child)
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
+ popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp)
+ stdout, stderr = popen.communicate()
+ self.assertRegex(stdout, expected_error)
 
- popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp_child)
- stdout, stderr = popen.communicate()
- self.assertEqual(stdout, b'') # no error
+ # and there's no error at all when using -P
+ popen = script_helper.spawn_python('-P', 'fractions.py', cwd=tmp)
+ stdout, stderr = popen.communicate()
+ self.assertEqual(stdout, b'')
 
- popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp_child)
- stdout, stderr = popen.communicate()
- self.assertEqual(stdout, b'') # no error
+ tmp_child = os.path.join(tmp, "child")
+ os.mkdir(tmp_child)
+
+ # test the logic with different cwd
+ popen = script_helper.spawn_python(os.path.join(tmp, "fractions.py"), cwd=tmp_child)
+ stdout, stderr = popen.communicate()
+ self.assertRegex(stdout, expected_error)
+
+ popen = script_helper.spawn_python('-m', 'fractions', cwd=tmp_child)
+ stdout, stderr = popen.communicate()
+ self.assertEqual(stdout, b'') # no error
+
+ popen = script_helper.spawn_python('-c', 'import fractions', cwd=tmp_child)
+ stdout, stderr = popen.communicate()
+ self.assertEqual(stdout, b'') # no error
 
  def test_package_shadowing_stdlib_module(self):
  with os_helper.temp_dir() as tmp:
@@ -863,27 +875,37 @@ def test_package_shadowing_stdlib_module(self):
  self.assertRegex(stdout, b"module 'fractions' has no attribute 'shadowing_module'")
 
  def test_script_shadowing_third_party(self):
+ script_errors = [
+ (
+ "import numpy\nnumpy.array",
+ rb"AttributeError: module 'numpy' has no attribute 'array'"
+ ),
+ (
+ "from numpy import array",
+ rb"ImportError: cannot import name 'array' from 'numpy'"
+ )
+ ]
  with os_helper.temp_dir() as tmp:
- with open(os.path.join(tmp, "numpy.py"), "w", encoding='utf-8') as f:
- f.write("import numpy\nnumpy.array")
+ for script, error in script_errors:
+ with open(os.path.join(tmp, "numpy.py"), "w", encoding='utf-8') as f:
+ f.write(script)
 
- expected_error = (
- rb"AttributeError: module 'numpy' has no attribute 'array' "
- rb"\(consider renaming '.*numpy.py' if it has the "
- rb"same name as a third-party module you intended to import\)\s+\Z"
- )
+ expected_error = error + (
+ rb" \(consider renaming '.*numpy.py' if it has the "
+ rb"same name as a third-party module you intended to import\)\s+\Z"
+ )
 
- popen = script_helper.spawn_python(os.path.join(tmp, "numpy.py"))
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
+  popen = script_helper.spawn_python(os.path.join(tmp, "numpy.py"))
+  stdout, stderr = popen.communicate()
+  self.assertRegex(stdout, expected_error)
 
- popen = script_helper.spawn_python('-m', 'numpy', cwd=tmp)
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
+  popen = script_helper.spawn_python('-m', 'numpy', cwd=tmp)
+  stdout, stderr = popen.communicate()
+  self.assertRegex(stdout, expected_error)
 
- popen = script_helper.spawn_python('-c', 'import numpy', cwd=tmp)
- stdout, stderr = popen.communicate()
- self.assertRegex(stdout, expected_error)
+  popen = script_helper.spawn_python('-c', 'import numpy', cwd=tmp)
+  stdout, stderr = popen.communicate()
+  self.assertRegex(stdout, expected_error)
 
  def test_script_maybe_not_shadowing_third_party(self):
  with os_helper.temp_dir() as tmp:

Objects/moduleobject.c

@@ -855,8 +855,8 @@ _PyModuleSpec_GetFileOrigin(PyObject *spec, PyObject **p_origin)
  return 1;
 }
 
-static int
-_is_module_possibly_shadowing(PyObject *origin)
+int
+_PyModule_IsPossiblyShadowing(PyObject *origin)
 {
  // origin must be a unicode subtype
  // Returns 1 if the module at origin could be shadowing a module of the
@@ -985,7 +985,7 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
  goto done;
  }
 
- int is_possibly_shadowing = _is_module_possibly_shadowing(origin);
+ int is_possibly_shadowing = _PyModule_IsPossiblyShadowing(origin);
  if (is_possibly_shadowing < 0) {
  goto done;
  }
@@ -1011,7 +1011,10 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
  }
  else {
  int rc = _PyModuleSpec_IsInitializing(spec);
- if (rc > 0) {
+ if (rc < 0) {
+ goto done;
+ }
+ else if (rc > 0) {
  if (is_possibly_shadowing) {
  assert(origin);
  // For third-party modules, only mention the possibility of
@@ -1037,7 +1040,8 @@ _Py_module_getattro_impl(PyModuleObject *m, PyObject *name, int suppress)
  mod_name, name);
  }
  }
- else if (rc == 0) {
+ else {
+ assert(rc == 0);
  rc = _PyModuleSpec_IsUninitializedSubmodule(spec, name);
  if (rc > 0) {
  PyErr_Format(PyExc_AttributeError,

Python/ceval.c

@@ -2801,6 +2801,7 @@ _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name)
  mod_name_or_unknown = mod_name;
  }
  // mod_name is no longer an owned reference
+ assert(mod_name_or_unknown);
  assert(mod_name == NULL || mod_name == mod_name_or_unknown);
 
  origin = NULL;
@@ -2818,27 +2819,80 @@ _PyEval_ImportFrom(PyThreadState *tstate, PyObject *v, PyObject *name)
  if (_PyModuleSpec_GetFileOrigin(spec, &origin) < 0) {
  goto done;
  }
- if (origin == NULL) {
+
+ int is_possibly_shadowing = _PyModule_IsPossiblyShadowing(origin);
+ if (is_possibly_shadowing < 0) {
+ goto done;
+ }
+ int is_possibly_shadowing_stdlib = 0;
+ if (is_possibly_shadowing) {
+ PyObject *stdlib_modules = PySys_GetObject("stdlib_module_names");
+ if (stdlib_modules && PyAnySet_Check(stdlib_modules)) {
+ is_possibly_shadowing_stdlib = PySet_Contains(stdlib_modules, mod_name);
+ if (is_possibly_shadowing_stdlib < 0) {
+ goto done;
+ }
+ }
+ }
+
+ if (is_possibly_shadowing_stdlib) {
+ assert(origin);
  errmsg = PyUnicode_FromFormat(
- "cannot import name %R from %R (unknown location)",
- name, mod_name_or_unknown
+ "cannot import name %R from %R "
+ "(consider renaming %R since it has the same "
+ "name as the standard library module named %R "
+ "and the import system gives it precedence)",
+ name, mod_name_or_unknown, origin, mod_name_or_unknown
  );
- goto done_with_errmsg;
  }
-
- int rc = _PyModuleSpec_IsInitializing(spec);
- if (rc < 0) {
- Py_DECREF(mod_name_or_unknown);
- Py_DECREF(origin);
- return NULL;
+ else {
+ int rc = _PyModuleSpec_IsInitializing(spec);
+ if (rc < 0) {
+ goto done;
+ }
+ else if (rc > 0) {
+ if (is_possibly_shadowing) {
+ assert(origin);
+ // For third-party modules, only mention the possibility of
+ // shadowing if the module is being initialized.
+ errmsg = PyUnicode_FromFormat(
+ "cannot import name %R from %R "
+ "(consider renaming %R if it has the same name "
+ "as a third-party module you intended to import)",
+ name, mod_name_or_unknown, origin
+ );
+ }
+ else if (origin) {
+ errmsg = PyUnicode_FromFormat(
+ "cannot import name %R from partially initialized module %R "
+ "(most likely due to a circular import) (%S)",
+ name, mod_name_or_unknown, origin
+ );
+ }
+ else {
+ errmsg = PyUnicode_FromFormat(
+ "cannot import name %R from partially initialized module %R "
+ "(most likely due to a circular import)",
+ name, mod_name_or_unknown
+ );
+ }
+ }
+ else {
+ assert(rc == 0);
+ if (origin) {
+ errmsg = PyUnicode_FromFormat(
+ "cannot import name %R from %R (%S)",
+ name, mod_name_or_unknown, origin
+ );
+ }
+ else {
+ errmsg = PyUnicode_FromFormat(
+ "cannot import name %R from %R (unknown location)",
+ name, mod_name_or_unknown
+ );
+ }
+ }
  }
- const char *fmt =
- rc ?
- "cannot import name %R from partially initialized module %R "
- "(most likely due to a circular import) (%S)" :
- "cannot import name %R from %R (%S)";
-
- errmsg = PyUnicode_FromFormat(fmt, name, mod_name_or_unknown, origin);
 
 done_with_errmsg:
  /* NULL checks for errmsg and mod_name done by PyErr_SetImportError. */