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

Prefer stdlib modules over same-named modules on sys.path #2223

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 2 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 @@ -15,6 +15,11 @@ Release date: TBA
Closes #1780
Refs #2140

* Prefer standard library modules over same-named modules on sys.path. For example
``import copy`` now finds ``copy`` instead of ``copy.py``. Solves ``no-member`` issues.

Closes pylint-dev/pylint#6535

* Reduce file system access in ``ast_from_file()``.

* Reduce time to ``import astroid`` by delaying ``astroid_bootstrapping()`` until
Expand Down
15 changes: 14 additions & 1 deletion astroid/interpreter/_import/spec.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
from typing import Any, Literal, NamedTuple, Protocol

from astroid.const import PY310_PLUS
from astroid.modutils import EXT_LIB_DIRS
from astroid.modutils import EXT_LIB_DIRS, STD_LIB_DIRS

from . import util

Expand Down Expand Up @@ -157,6 +157,19 @@ def find_module(
location=getattr(spec.loader_state, "filename", None),
type=ModuleType.PY_FROZEN,
)
if (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could this clash with namespace modules?

Copy link
Member Author

Choose a reason for hiding this comment

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

Could you flesh this out a little more for me? (Sorry!)

Copy link
Collaborator

Choose a reason for hiding this comment

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

❯ tree .
.
β”œβ”€β”€ argparse.py
└── test.py

1 directory, 2 files
# test.py

import argparse

argparse.i_dont_exist()
# argparse.py

def i_dont_exist():
    print("1")
❯ python test.py
1

Won't this implementation default to stdlib.argparse instead of the local argparse?

This comment was marked as outdated.

Copy link
Member Author

@jacobtylerwalls jacobtylerwalls Jun 27, 2023

Choose a reason for hiding this comment

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

Oops, in the comment I just hid I hadn't run the test case against the correct copy of astroid. More soon.

spec
and isinstance(spec.loader, importlib.machinery.SourceFileLoader)
and any(spec.origin.startswith(std_lib) for std_lib in STD_LIB_DIRS)
and not spec.origin.endswith("__init__.py")
):
# Return standard library modules before local modules
# https://github.com/pylint-dev/pylint/issues/6535
return ModuleSpec(
name=modname,
location=spec.origin,
type=ModuleType.PY_SOURCE,
)
except ValueError:
pass
submodule_path = sys.path
Expand Down
17 changes: 16 additions & 1 deletion tests/test_modutils.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

import astroid
from astroid import modutils
from astroid.const import PY310_PLUS
from astroid.const import PY310_PLUS, PY311_PLUS, WIN32
from astroid.interpreter._import import spec

from . import resources
Expand Down Expand Up @@ -268,6 +268,21 @@ def test_std_lib(self) -> None:
os.path.realpath(os.path.__file__.replace(".pyc", ".py")),
)

@pytest.mark.skipif(
WIN32 and not PY311_PLUS,
reason="Fails on Windows below 3.11 for what seems like a test setup/isolation issue "
"rather than a functional issue. Possibly related: "
"https://github.com/python/cpython/pull/93653 (other surrounding tests add '.' to sys.path)",
)
def test_std_lib_found_before_same_named_package_on_path(self) -> None:
sys.path.insert(0, str(resources.RESOURCE_PATH))
self.addCleanup(sys.path.pop, 0)

file = modutils.file_from_modpath(["copy"])

self.assertNotIn("test", file) # tests/testdata/python3/data/copy.py
self.assertTrue(any(stdlib in file for stdlib in modutils.STD_LIB_DIRS))

def test_builtin(self) -> None:
self.assertIsNone(modutils.file_from_modpath(["sys"]))

Expand Down
1 change: 1 addition & 0 deletions tests/testdata/python3/data/copy.py
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
"""fake copy module (unlike email, we need one without __init__.py)"""