Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't rely on the module cache when "importing self" #1747

Merged
merged 8 commits into from
Aug 23, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ What's New in astroid 2.13.0?
=============================
Release date: TBA

* Fixed importing of modules that have the same name as the file that is importing.
A file called ``math.py`` will now correctly ``import math`` from the standard library
instead of trying to import itself.

Refs PyCQA/pylint#5151


What's New in astroid 2.12.3?
Expand Down
15 changes: 12 additions & 3 deletions astroid/manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -147,9 +147,18 @@ def _can_load_extension(self, modname: str) -> bool:
modname, self.extension_package_whitelist
)

def ast_from_module_name(self, modname, context_file=None):
"""given a module name, return the astroid object"""
if modname in self.astroid_cache:
def ast_from_module_name(
self,
modname: str | None,
context_file: str | None = None,
use_cache: bool = True,
) -> nodes.Module:
"""Given a module name, return the astroid object."""
# Sometimes we don't want to use the cache. For example, when we're
# importing a module with the same name as the file that is importing
# we want to fallback on the import system to make sure we get the correct
# module.
if modname in self.astroid_cache and use_cache:
return self.astroid_cache[modname]
if modname == "__main__":
return self._build_stub_module(modname)
Expand Down
23 changes: 12 additions & 11 deletions astroid/nodes/_base_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,25 +128,26 @@ def _infer_name(self, frame, name):
return name

def do_import_module(self, modname: str | None = None) -> nodes.Module:
"""return the ast for a module whose name is <modname> imported by <self>"""
# handle special case where we are on a package node importing a module
# using the same name as the package, which may end in an infinite loop
# on relative imports
# XXX: no more needed ?
"""Return the ast for a module whose name is <modname> imported by <self>."""
mymodule = self.root()
level = getattr(self, "level", None) # Import as no level
level: int | None = getattr(self, "level", None) # Import has no level
if modname is None:
modname = self.modname
# XXX we should investigate deeper if we really want to check
# importing itself: modname and mymodule.name be relative or absolute
# If the module ImportNode is importing is a module with the same name
# as the file that contains the ImportNode we don't want to use the cache
# to make sure we use the import system to get the correct module.
# pylint: disable-next=no-member # pylint doesn't recognize type of mymodule
if mymodule.relative_to_absolute_name(modname, level) == mymodule.name:
# FIXME: we used to raise InferenceError here, but why ?
return mymodule
use_cache = False
else:
use_cache = True

# pylint: disable-next=no-member # pylint doesn't recognize type of mymodule
return mymodule.import_module(
modname, level=level, relative_only=level and level >= 1
modname,
level=level,
relative_only=bool(level and level >= 1),
use_cache=use_cache,
)

def real_name(self, asname):
Expand Down
18 changes: 12 additions & 6 deletions astroid/nodes/scoped_nodes/scoped_nodes.py
Original file line number Diff line number Diff line change
Expand Up @@ -498,27 +498,33 @@ def absolute_import_activated(self):
"""
return self._absolute_import_activated

def import_module(self, modname, relative_only=False, level=None):
def import_module(
self,
modname: str | None,
relative_only: bool = False,
level: int | None = None,
use_cache: bool = True,
) -> Module:
"""Get the ast for a given module as if imported from this module.

:param modname: The name of the module to "import".
:type modname: str

:param relative_only: Whether to only consider relative imports.
:type relative_only: bool

:param level: The level of relative import.
:type level: int or None

:param use_cache: Whether to use the astroid_cache of modules.

:returns: The imported module ast.
:rtype: NodeNG
"""
if relative_only and level is None:
level = 0
absmodname = self.relative_to_absolute_name(modname, level)

try:
return AstroidManager().ast_from_module_name(absmodname)
return AstroidManager().ast_from_module_name(
absmodname, use_cache=use_cache
)
except AstroidBuildingError:
# we only want to import a sub module or package of this module,
# skip here
Expand Down
3 changes: 3 additions & 0 deletions tests/testdata/python3/data/import_conflicting_names/math.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import math

print(math)
17 changes: 17 additions & 0 deletions tests/unittest_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -325,6 +325,23 @@ def hook(modname: str):
with self.assertRaises(AstroidBuildingError):
self.manager.ast_from_module_name("foo.bar.baz")

def test_same_name_import_module(self) -> None:
"""Test inference of an import statement with the same name as the module.

See https://github.com/PyCQA/pylint/issues/5151.
"""
math_file = resources.find("data/import_conflicting_names/math.py")
module = self.manager.ast_from_file(math_file)

# Change the cache key and module name to mimick importing the test file
# from the root/top level. This creates a clash between math.py and stdlib math.
self.manager.astroid_cache["math"] = self.manager.astroid_cache.pop(module.name)
module.name = "math"

# Infer the 'import math' statement
stdlib_math = next(module.body[1].value.args[0].infer())
assert self.manager.astroid_cache["math"] != stdlib_math
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the old math stays in place in the cache. This is how we handle all cache clashes so I think this is also good in this case?



class BorgAstroidManagerTC(unittest.TestCase):
def test_borg(self) -> None:
Expand Down