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

Alter fallback for source-roots #9967

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
24 changes: 16 additions & 8 deletions pylint/lint/expand_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@
)


def discover_package_path(modulepath: str, source_roots: Sequence[str]) -> str:
def discover_package_path(
modulepath: str, source_roots: Sequence[str]
) -> Sequence[str]:
"""Discover package path from one its modules and source roots."""
dirname = os.path.realpath(os.path.expanduser(modulepath))
if not os.path.isdir(dirname):
Expand All @@ -33,18 +35,24 @@
# Look for a source root that contains the module directory
for source_root in source_roots:
source_root = os.path.realpath(os.path.expanduser(source_root))
if os.path.commonpath([source_root, dirname]) == source_root:
return source_root
try:
if os.path.commonpath([source_root, dirname]) == source_root:
return [source_root]
except ValueError:
continue

Check warning on line 42 in pylint/lint/expand_modules.py

View check run for this annotation

Codecov / codecov/patch

pylint/lint/expand_modules.py#L41-L42

Added lines #L41 - L42 were not covered by tests

if len(source_roots) != 0:
return source_roots

# Fall back to legacy discovery by looking for __init__.py upwards as
# it's the only way given that source root was not found or was not provided
# source_roots was not provided
while True:
if not os.path.exists(os.path.join(dirname, "__init__.py")):
return dirname
return [dirname]
old_dirname = dirname
dirname = os.path.dirname(dirname)
if old_dirname == dirname:
return os.getcwd()
return [os.getcwd()]


def _is_in_ignore_list_re(element: str, ignore_list_re: list[Pattern[str]]) -> bool:
Expand Down Expand Up @@ -88,8 +96,8 @@
something, ignore_list, ignore_list_re, ignore_list_paths_re
):
continue
module_package_path = discover_package_path(something, source_roots)
additional_search_path = [".", module_package_path, *path]
module_package_paths = discover_package_path(something, source_roots)
additional_search_path = [".", *module_package_paths, *path]
if os.path.exists(something):
# this is a file or a directory
try:
Expand Down
5 changes: 4 additions & 1 deletion pylint/lint/pylinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -667,8 +667,11 @@ def check(self, files_or_modules: Sequence[str]) -> None:
extra_packages_paths = list(
dict.fromkeys(
[
discover_package_path(file_or_module, self.config.source_roots)
path
for file_or_module in files_or_modules
for path in discover_package_path(
file_or_module, self.config.source_roots
)
]
).keys()
)
Expand Down
6 changes: 5 additions & 1 deletion pylint/pyreverse/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,11 @@ def run(self, args: list[str]) -> int:
print(self.help())
return 1
extra_packages_paths = list(
{discover_package_path(arg, self.config.source_roots) for arg in args}
{
path
for arg in args
for path in discover_package_path(arg, self.config.source_roots)
}
)
with augmented_sys_path(extra_packages_paths):
project = project_from_files(
Expand Down
4 changes: 2 additions & 2 deletions tests/checkers/unittest_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ def test_relative_beyond_top_level_four(capsys: CaptureFixture[str]) -> None:
def test_wildcard_import_init(self) -> None:
context_file = os.path.join(REGR_DATA, "dummy_wildcard.py")

with augmented_sys_path([discover_package_path(context_file, [])]):
with augmented_sys_path(discover_package_path(context_file, [])):
module = astroid.MANAGER.ast_from_module_name("init_wildcard", context_file)
import_from = module.body[0]

Expand All @@ -105,7 +105,7 @@ def test_wildcard_import_init(self) -> None:
def test_wildcard_import_non_init(self) -> None:
context_file = os.path.join(REGR_DATA, "dummy_wildcard.py")

with augmented_sys_path([discover_package_path(context_file, [])]):
with augmented_sys_path(discover_package_path(context_file, [])):
module = astroid.MANAGER.ast_from_module_name("wildcard", context_file)
import_from = module.body[0]
msg = MessageTest(
Expand Down
46 changes: 45 additions & 1 deletion tests/lint/unittest_expand_modules.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,11 @@
import pytest

from pylint.checkers import BaseChecker
from pylint.lint.expand_modules import _is_in_ignore_list_re, expand_modules
from pylint.lint.expand_modules import (
_is_in_ignore_list_re,
discover_package_path,
expand_modules,
)
from pylint.testutils import CheckerTestCase, set_config
from pylint.typing import MessageDefinitionTuple, ModuleDescriptionDict

Expand Down Expand Up @@ -306,3 +310,43 @@ def test_expand_modules_with_ignore(
)
assert modules == expected
assert not errors


def test_discover_package_path_no_source_root_overlap(tmp_path: Path) -> None:
"""Test whether source_roots is returned even if module doesn't overlap
with source_roots
"""
source_roots = [str(tmp_path)]
package_paths = discover_package_path(__file__, source_roots)

expected = source_roots
assert package_paths == expected


def test_discover_package_path_legacy() -> None:
"""Test for legacy path discovery when source_roots is empty"""
source_roots: list[str] = []
package_paths = discover_package_path(__file__, source_roots)

# First ancestor directory without __init__.py
expected = [str(Path(__file__).parent.parent.absolute())]

assert package_paths == expected


def test_discover_package_path_legacy_no_parent_without_init_py(
tmp_path: Path, monkeypatch: pytest.MonkeyPatch
) -> None:
"""Test to return current directory if no parent directory without
__init__.py is found
"""
source_roots: list[str] = []

monkeypatch.setattr(os.path, "exists", lambda path: True)
monkeypatch.setattr(os.path, "dirname", lambda path: path)

package_paths = discover_package_path(str(tmp_path), source_roots)

expected = [os.getcwd()]

assert package_paths == expected
16 changes: 11 additions & 5 deletions tests/lint/unittest_lint.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,9 @@ def test_one_arg(fake_path: list[str], case: list[str]) -> None:
expected = [join(chroot, "a"), *fake_path]

extra_sys_paths = [
expand_modules.discover_package_path(arg, []) for arg in case
path
for arg in case
for path in expand_modules.discover_package_path(arg, [])
]

assert sys.path == fake_path
Expand All @@ -157,7 +159,9 @@ def test_two_similar_args(fake_path: list[str], case: list[str]) -> None:
expected = [join(chroot, "a"), *fake_path]

extra_sys_paths = [
expand_modules.discover_package_path(arg, []) for arg in case
path
for arg in case
for path in expand_modules.discover_package_path(arg, [])
]

assert sys.path == fake_path
Expand All @@ -183,7 +187,9 @@ def test_more_args(fake_path: list[str], case: list[str]) -> None:
] + fake_path

extra_sys_paths = [
expand_modules.discover_package_path(arg, []) for arg in case
path
for arg in case
for path in expand_modules.discover_package_path(arg, [])
]

assert sys.path == fake_path
Expand Down Expand Up @@ -1242,7 +1248,7 @@ def test_import_sibling_module_from_namespace(initialized_linter: PyLinter) -> N
"""
)
os.chdir("namespace")
extra_sys_paths = [expand_modules.discover_package_path(tmpdir, [])]
extra_sys_paths = expand_modules.discover_package_path(tmpdir, [])

# Add the parent directory to sys.path
with lint.augmented_sys_path(extra_sys_paths):
Expand All @@ -1267,7 +1273,7 @@ def test_lint_namespace_package_under_dir_on_path(initialized_linter: PyLinter)
with tempdir() as tmpdir:
create_files(["namespace_on_path/submodule1.py"])
os.chdir(tmpdir)
extra_sys_paths = [expand_modules.discover_package_path(tmpdir, [])]
extra_sys_paths = expand_modules.discover_package_path(tmpdir, [])
with lint.augmented_sys_path(extra_sys_paths):
linter.check(["namespace_on_path"])
assert linter.file_state.base_name == "namespace_on_path"
2 changes: 1 addition & 1 deletion tests/pyreverse/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ def _astroid_wrapper(
) -> Module:
return func(modname)

with augmented_sys_path([discover_package_path(module, [])]):
with augmented_sys_path(discover_package_path(module, [])):
project = project_from_files([module], _astroid_wrapper, project_name=name)
return project

Expand Down
2 changes: 1 addition & 1 deletion tests/pyreverse/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ def test_project_root_in_sys_path() -> None:
"""Test the context manager adds the project root directory to sys.path.
This should happen when pyreverse is run from any directory.
"""
with augmented_sys_path([discover_package_path(TEST_DATA_DIR, [])]):
with augmented_sys_path(discover_package_path(TEST_DATA_DIR, [])):
assert sys.path == [PROJECT_ROOT_DIR]


Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
""" Test for source-roots import for implicit namespace package. The following
should succeed."""
import namespacepkg.pkg.app
Empty file.
Empty file.
2 changes: 2 additions & 0 deletions tests/regrtest_data/source_roots_src_layout/tests/testapp.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
""" Test for import. The following import should work if source-roots is setup correctly """
import mypkg.app
10 changes: 10 additions & 0 deletions tests/test_self.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,16 @@ def test_exit_zero(self) -> None:
["--exit-zero", join(HERE, "regrtest_data", "syntax_error.py")], code=0
)

@pytest.mark.parametrize(
"repo", ["source_roots_src_layout", "source_roots_implicit_namespace_pkg"]
)
def test_source_roots_src_layout(self, repo: str) -> None:
repo = join(HERE, "regrtest_data", repo)
src_path = join(repo, "src")
self._runtest(
["-d", "unused-import", f"--source-roots={src_path}", repo], code=0
)

def test_nonexistent_config_file(self) -> None:
self._runtest(["--rcfile=/tmp/this_file_does_not_exist"], code=32)

Expand Down
Loading