diff --git a/pylint/lint/expand_modules.py b/pylint/lint/expand_modules.py index f40bdeea5b..ebfa030c68 100644 --- a/pylint/lint/expand_modules.py +++ b/pylint/lint/expand_modules.py @@ -24,7 +24,9 @@ def _is_package_cb(inner_path: str, parts: list[str]) -> bool: ) -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): @@ -33,18 +35,24 @@ def discover_package_path(modulepath: str, source_roots: Sequence[str]) -> str: # 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 + + 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: @@ -88,8 +96,8 @@ def expand_modules( 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: diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index cc4605b96a..2df0d09fa1 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -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() ) diff --git a/pylint/pyreverse/main.py b/pylint/pyreverse/main.py index 3ba0b6c77d..166bca7434 100644 --- a/pylint/pyreverse/main.py +++ b/pylint/pyreverse/main.py @@ -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( diff --git a/tests/checkers/unittest_imports.py b/tests/checkers/unittest_imports.py index 92477ac2ea..d89dad5a11 100644 --- a/tests/checkers/unittest_imports.py +++ b/tests/checkers/unittest_imports.py @@ -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] @@ -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( diff --git a/tests/lint/unittest_expand_modules.py b/tests/lint/unittest_expand_modules.py index e8d38e6a57..b9ae0631fa 100644 --- a/tests/lint/unittest_expand_modules.py +++ b/tests/lint/unittest_expand_modules.py @@ -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 @@ -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 diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 7ba8879e9a..fa2077fa1b 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -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 @@ -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 @@ -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 @@ -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): @@ -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" diff --git a/tests/pyreverse/conftest.py b/tests/pyreverse/conftest.py index ac63b2aeaa..35a267f810 100644 --- a/tests/pyreverse/conftest.py +++ b/tests/pyreverse/conftest.py @@ -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 diff --git a/tests/pyreverse/test_main.py b/tests/pyreverse/test_main.py index e8e46df2c1..63f177ece8 100644 --- a/tests/pyreverse/test_main.py +++ b/tests/pyreverse/test_main.py @@ -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] diff --git a/tests/regrtest_data/source_roots_implicit_namespace_pkg/src/namespacepkg/pkg/__init__.py b/tests/regrtest_data/source_roots_implicit_namespace_pkg/src/namespacepkg/pkg/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regrtest_data/source_roots_implicit_namespace_pkg/src/namespacepkg/pkg/app.py b/tests/regrtest_data/source_roots_implicit_namespace_pkg/src/namespacepkg/pkg/app.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regrtest_data/source_roots_implicit_namespace_pkg/tests/testapp.py b/tests/regrtest_data/source_roots_implicit_namespace_pkg/tests/testapp.py new file mode 100644 index 0000000000..0ffaa00df3 --- /dev/null +++ b/tests/regrtest_data/source_roots_implicit_namespace_pkg/tests/testapp.py @@ -0,0 +1,3 @@ +""" Test for source-roots import for implicit namespace package. The following + should succeed.""" +import namespacepkg.pkg.app diff --git a/tests/regrtest_data/source_roots_src_layout/src/mypkg/__init__.py b/tests/regrtest_data/source_roots_src_layout/src/mypkg/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regrtest_data/source_roots_src_layout/src/mypkg/app.py b/tests/regrtest_data/source_roots_src_layout/src/mypkg/app.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regrtest_data/source_roots_src_layout/tests/testapp.py b/tests/regrtest_data/source_roots_src_layout/tests/testapp.py new file mode 100644 index 0000000000..c028b81c23 --- /dev/null +++ b/tests/regrtest_data/source_roots_src_layout/tests/testapp.py @@ -0,0 +1,2 @@ +""" Test for import. The following import should work if source-roots is setup correctly """ +import mypkg.app diff --git a/tests/test_self.py b/tests/test_self.py index d01821c34c..f45361080d 100644 --- a/tests/test_self.py +++ b/tests/test_self.py @@ -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)