From b8c5c56481426352afe2434be520bdf59c2eb428 Mon Sep 17 00:00:00 2001 From: Alexey Pelykh Date: Wed, 1 Feb 2023 15:38:19 +0100 Subject: [PATCH 1/3] Support Implicit Namespace Packages (PEP 420) --- doc/user_guide/configuration/all-options.rst | 7 ++ doc/user_guide/usage/run.rst | 7 ++ doc/whatsnew/fragments/8154.feature | 3 + examples/pylintrc | 5 ++ pylint/config/argument.py | 11 +++ pylint/config/option.py | 3 + pylint/lint/__init__.py | 11 ++- pylint/lint/base_options.py | 11 +++ pylint/lint/expand_modules.py | 33 +++++++-- pylint/lint/parallel.py | 16 +++-- pylint/lint/pylinter.py | 28 ++++++-- pylint/lint/utils.py | 43 +++++++++-- pylint/pyreverse/main.py | 19 ++++- pylint/testutils/pyreverse.py | 9 ++- tests/lint/unittest_expand_modules.py | 3 + tests/lint/unittest_lint.py | 72 ++++++++++++++++--- tests/primer/packages_to_prime.json | 5 +- tests/pyreverse/conftest.py | 4 +- .../namespaces/pep420/pep420.dot | 5 ++ .../namespaces/pep420/pep420.mmd | 3 + .../namespaces/pep420/pep420.puml | 5 ++ .../namespaces/pep420/pep420.py | 2 + .../namespaces/pep420/pep420.rc | 3 + tests/pyreverse/test_main.py | 4 +- tests/pyreverse/test_pyreverse_functional.py | 19 ++++- .../project/namespace/package/__init__.py | 0 .../namespace/package/logging/__init__.py | 5 ++ .../package/logging/wrapper/__init__.py | 5 ++ tests/test_check_parallel.py | 20 ++++-- tests/test_deprecation.py | 14 ++++ 30 files changed, 321 insertions(+), 54 deletions(-) create mode 100644 doc/whatsnew/fragments/8154.feature create mode 100644 tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.dot create mode 100644 tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.mmd create mode 100644 tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.puml create mode 100644 tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.py create mode 100644 tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.rc create mode 100644 tests/regrtest_data/pep420/basic/project/namespace/package/__init__.py create mode 100644 tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/__init__.py create mode 100644 tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/wrapper/__init__.py diff --git a/doc/user_guide/configuration/all-options.rst b/doc/user_guide/configuration/all-options.rst index b83a2ca083..dcc8e39b22 100644 --- a/doc/user_guide/configuration/all-options.rst +++ b/doc/user_guide/configuration/all-options.rst @@ -174,6 +174,13 @@ Standard Checkers **Default:** ``(3, 10)`` +--source-roots +"""""""""""""" +*Add paths to the list of the source roots. The source root is an absolute path or a path relative to the current working directory used to determine a package namespace for modules located under the source root.* + +**Default:** ``()`` + + --recursive """"""""""" *Discover python modules and packages in the file system subtree.* diff --git a/doc/user_guide/usage/run.rst b/doc/user_guide/usage/run.rst index 84e1a8e2f7..fb298dae9a 100644 --- a/doc/user_guide/usage/run.rst +++ b/doc/user_guide/usage/run.rst @@ -45,6 +45,13 @@ directory is automatically added on top of the python path package (i.e. has an ``__init__.py`` file), an implicit namespace package or if ``directory`` is in the python path. +With implicit namespace packages +-------------------------------- + +If the analyzed sources use the Implicit Namespace Packages (PEP 420), the source root(s) should +be specified to Pylint using the ``--source-roots`` option. Otherwise, the package names are +detected incorrectly, since the Implicit Namespace Packages don't contain the ``__init__.py``. + Command line options -------------------- diff --git a/doc/whatsnew/fragments/8154.feature b/doc/whatsnew/fragments/8154.feature new file mode 100644 index 0000000000..840540de29 --- /dev/null +++ b/doc/whatsnew/fragments/8154.feature @@ -0,0 +1,3 @@ +Support Implicit Namespace Packages (PEP 420). + +Closes #8154 diff --git a/examples/pylintrc b/examples/pylintrc index 1cf9639a13..879c1d4f13 100644 --- a/examples/pylintrc +++ b/examples/pylintrc @@ -90,6 +90,11 @@ persistent=yes # the version used to run pylint. py-version=3.10 +# Add paths to the list of the source roots. The source root is an absolute +# path or a path relative to the current working directory used to +# determine a package namespace for modules located under the source root. +source-roots=src,tests + # Discover python modules and packages in the file system subtree. recursive=no diff --git a/pylint/config/argument.py b/pylint/config/argument.py index b0ff4d5de2..fd01a9b5f9 100644 --- a/pylint/config/argument.py +++ b/pylint/config/argument.py @@ -88,6 +88,16 @@ def _path_transformer(value: str) -> str: return os.path.expandvars(os.path.expanduser(value)) +def _paths_csv_transformer(value: str) -> Sequence[str]: + """Transforms a comma separated list of paths while expanding user and + variables. + """ + paths: list[str] = [] + for path in _csv_transformer(value): + paths.append(os.path.expandvars(os.path.expanduser(path))) + return paths + + def _py_version_transformer(value: str) -> tuple[int, ...]: """Transforms a version string into a version tuple.""" try: @@ -138,6 +148,7 @@ def _regexp_paths_csv_transfomer(value: str) -> Sequence[Pattern[str]]: "confidence": _confidence_transformer, "non_empty_string": _non_empty_string_transformer, "path": _path_transformer, + "paths_csv": _paths_csv_transformer, "py_version": _py_version_transformer, "regexp": _regex_transformer, "regexp_csv": _regexp_csv_transfomer, diff --git a/pylint/config/option.py b/pylint/config/option.py index 95248d6b1e..5a425f34d9 100644 --- a/pylint/config/option.py +++ b/pylint/config/option.py @@ -117,6 +117,7 @@ def _py_version_validator(_: Any, name: str, value: Any) -> tuple[int, int, int] "string": utils._unquote, "int": int, "float": float, + "paths_csv": _csv_validator, "regexp": lambda pattern: re.compile(pattern or ""), "regexp_csv": _regexp_csv_validator, "regexp_paths_csv": _regexp_paths_csv_validator, @@ -163,6 +164,7 @@ def _validate(value: Any, optdict: Any, name: str = "") -> Any: # pylint: disable=no-member class Option(optparse.Option): TYPES = optparse.Option.TYPES + ( + "paths_csv", "regexp", "regexp_csv", "regexp_paths_csv", @@ -175,6 +177,7 @@ class Option(optparse.Option): ) ATTRS = optparse.Option.ATTRS + ["hide", "level"] TYPE_CHECKER = copy.copy(optparse.Option.TYPE_CHECKER) + TYPE_CHECKER["paths_csv"] = _csv_validator TYPE_CHECKER["regexp"] = _regexp_validator TYPE_CHECKER["regexp_csv"] = _regexp_csv_validator TYPE_CHECKER["regexp_paths_csv"] = _regexp_paths_csv_validator diff --git a/pylint/lint/__init__.py b/pylint/lint/__init__.py index 86186ebd40..573d9c2624 100644 --- a/pylint/lint/__init__.py +++ b/pylint/lint/__init__.py @@ -18,6 +18,7 @@ from pylint.config.exceptions import ArgumentPreprocessingError from pylint.lint.caching import load_results, save_results +from pylint.lint.expand_modules import discover_package_path from pylint.lint.parallel import check_parallel from pylint.lint.pylinter import PyLinter from pylint.lint.report_functions import ( @@ -26,7 +27,12 @@ report_total_messages_stats, ) from pylint.lint.run import Run -from pylint.lint.utils import _patch_sys_path, fix_import_path +from pylint.lint.utils import ( + _augment_sys_path, + _patch_sys_path, + augmented_sys_path, + fix_import_path, +) __all__ = [ "check_parallel", @@ -38,6 +44,9 @@ "ArgumentPreprocessingError", "_patch_sys_path", "fix_import_path", + "_augment_sys_path", + "augmented_sys_path", + "discover_package_path", "save_results", "load_results", ] diff --git a/pylint/lint/base_options.py b/pylint/lint/base_options.py index 1c37eac2fb..8540340cda 100644 --- a/pylint/lint/base_options.py +++ b/pylint/lint/base_options.py @@ -343,6 +343,17 @@ def _make_linter_options(linter: PyLinter) -> Options: ), }, ), + ( + "source-roots", + { + "type": "paths_csv", + "metavar": "[,...]", + "default": (), + "help": "Add paths to the list of the source roots. The source root is an absolute " + "path or a path relative to the current working directory used to " + "determine a package namespace for modules located under the source root.", + }, + ), ( "recursive", { diff --git a/pylint/lint/expand_modules.py b/pylint/lint/expand_modules.py index 8259e25ada..bb25986e4e 100644 --- a/pylint/lint/expand_modules.py +++ b/pylint/lint/expand_modules.py @@ -6,6 +6,7 @@ import os import sys +import warnings from collections.abc import Sequence from re import Pattern @@ -24,14 +25,31 @@ def _is_package_cb(inner_path: str, parts: list[str]) -> bool: def get_python_path(filepath: str) -> str: - """TODO This get the python path with the (bad) assumption that there is always - an __init__.py. + # TODO: Remove deprecated function + warnings.warn( + "get_python_path has been deprecated because assumption that there's always an __init__.py " + "is not true since python 3.3 and is causing problems, particularly with PEP 420." + "Use discover_package_path and pass source root(s).", + DeprecationWarning, + stacklevel=2, + ) + return discover_package_path(filepath, []) - This is not true since python 3.3 and is causing problem. - """ - dirname = os.path.realpath(os.path.expanduser(filepath)) + +def discover_package_path(modulepath: str, source_roots: Sequence[str]) -> 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): dirname = os.path.dirname(dirname) + + # 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 + + # 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 while True: if not os.path.exists(os.path.join(dirname, "__init__.py")): return dirname @@ -64,6 +82,7 @@ def _is_ignored_file( # pylint: disable = too-many-locals, too-many-statements def expand_modules( files_or_modules: Sequence[str], + source_roots: Sequence[str], ignore_list: list[str], ignore_list_re: list[Pattern[str]], ignore_list_paths_re: list[Pattern[str]], @@ -81,8 +100,8 @@ def expand_modules( something, ignore_list, ignore_list_re, ignore_list_paths_re ): continue - module_path = get_python_path(something) - additional_search_path = [".", module_path] + path + module_package_path = discover_package_path(something, source_roots) + additional_search_path = [".", module_package_path] + path if os.path.exists(something): # this is a file or a directory try: diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index 9730914b75..544a256d33 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -13,7 +13,7 @@ import dill from pylint import reporters -from pylint.lint.utils import _patch_sys_path +from pylint.lint.utils import _augment_sys_path from pylint.message import Message from pylint.typing import FileItem from pylint.utils import LinterStats, merge_stats @@ -37,12 +37,12 @@ def _worker_initialize( - linter: bytes, arguments: None | str | Sequence[str] = None + linter: bytes, extra_packages_paths: Sequence[str] | None = None ) -> None: """Function called to initialize a worker for a Process within a concurrent Pool. :param linter: A linter-class (PyLinter) instance pickled with dill - :param arguments: File or module name(s) to lint and to be added to sys.path + :param extra_packages_paths: Extra entries to be added to sys.path """ global _worker_linter # pylint: disable=global-statement _worker_linter = dill.loads(linter) @@ -53,8 +53,8 @@ def _worker_initialize( _worker_linter.set_reporter(reporters.CollectingReporter()) _worker_linter.open() - # Patch sys.path so that each argument is importable just like in single job mode - _patch_sys_path(arguments or ()) + if extra_packages_paths: + _augment_sys_path(extra_packages_paths) def _worker_check_single_file( @@ -130,7 +130,7 @@ def check_parallel( linter: PyLinter, jobs: int, files: Iterable[FileItem], - arguments: None | str | Sequence[str] = None, + extra_packages_paths: Sequence[str] | None = None, ) -> None: """Use the given linter to lint the files with given amount of workers (jobs). @@ -140,7 +140,9 @@ def check_parallel( # The linter is inherited by all the pool's workers, i.e. the linter # is identical to the linter object here. This is required so that # a custom PyLinter object can be used. - initializer = functools.partial(_worker_initialize, arguments=arguments) + initializer = functools.partial( + _worker_initialize, extra_packages_paths=extra_packages_paths + ) with ProcessPoolExecutor( max_workers=jobs, initializer=initializer, initargs=(dill.dumps(linter),) ) as executor: diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 4d1bede9a6..5b749d5b2f 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -36,7 +36,11 @@ from pylint.interfaces import HIGH from pylint.lint.base_options import _make_linter_options from pylint.lint.caching import load_results, save_results -from pylint.lint.expand_modules import _is_ignored_file, expand_modules +from pylint.lint.expand_modules import ( + _is_ignored_file, + discover_package_path, + expand_modules, +) from pylint.lint.message_state_handler import _MessageStateHandler from pylint.lint.parallel import check_parallel from pylint.lint.report_functions import ( @@ -46,7 +50,7 @@ ) from pylint.lint.utils import ( _is_relative_to, - fix_import_path, + augmented_sys_path, get_fatal_error_message, prepare_crash_report, ) @@ -675,6 +679,13 @@ def check(self, files_or_modules: Sequence[str] | str) -> None: "Missing filename required for --from-stdin" ) + extra_packages_paths = list( + { + discover_package_path(file_or_module, self.config.source_roots) + for file_or_module in files_or_modules + } + ) + # TODO: Move the parallel invocation into step 5 of the checking process if not self.config.from_stdin and self.config.jobs > 1: original_sys_path = sys.path[:] @@ -682,13 +693,13 @@ def check(self, files_or_modules: Sequence[str] | str) -> None: self, self.config.jobs, self._iterate_file_descrs(files_or_modules), - files_or_modules, # this argument patches sys.path + extra_packages_paths, ) sys.path = original_sys_path return # 3) Get all FileItems - with fix_import_path(files_or_modules): + with augmented_sys_path(extra_packages_paths): if self.config.from_stdin: fileitems = self._get_file_descr_from_stdin(files_or_modules[0]) data: str | None = _read_stdin() @@ -697,7 +708,7 @@ def check(self, files_or_modules: Sequence[str] | str) -> None: data = None # The contextmanager also opens all checkers and sets up the PyLinter class - with fix_import_path(files_or_modules): + with augmented_sys_path(extra_packages_paths): with self._astroid_module_checker() as check_astroid_module: # 4) Get the AST for each FileItem ast_per_fileitem = self._get_asts(fileitems, data) @@ -884,10 +895,13 @@ def _iterate_file_descrs( if self.should_analyze_file(name, filepath, is_argument=is_arg): yield FileItem(name, filepath, descr["basename"]) - def _expand_files(self, modules: Sequence[str]) -> dict[str, ModuleDescriptionDict]: + def _expand_files( + self, files_or_modules: Sequence[str] + ) -> dict[str, ModuleDescriptionDict]: """Get modules and errors from a list of modules and handle errors.""" result, errors = expand_modules( - modules, + files_or_modules, + self.config.source_roots, self.config.ignore, self.config.ignore_patterns, self._ignore_paths, diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index d4ad131f32..98fb8087a7 100644 --- a/pylint/lint/utils.py +++ b/pylint/lint/utils.py @@ -7,12 +7,13 @@ import contextlib import sys import traceback +import warnings from collections.abc import Iterator, Sequence from datetime import datetime from pathlib import Path from pylint.config import PYLINT_HOME -from pylint.lint.expand_modules import get_python_path +from pylint.lint.expand_modules import discover_package_path def prepare_crash_report(ex: Exception, filepath: str, crash_file_path: str) -> Path: @@ -73,14 +74,26 @@ def get_fatal_error_message(filepath: str, issue_template_path: Path) -> str: def _patch_sys_path(args: Sequence[str]) -> list[str]: + # TODO: Remove deprecated function + warnings.warn( + "_patch_sys_path has been deprecated because it relies on auto-magic package path " + "discovery which is implemented by get_python_path that is deprecated. " + "Use _augment_sys_path and pass additional sys.path entries as an argument obtained from " + "discover_package_path.", + DeprecationWarning, + stacklevel=2, + ) + return _augment_sys_path([discover_package_path(arg, []) for arg in args]) + + +def _augment_sys_path(additional_paths: Sequence[str]) -> list[str]: original = list(sys.path) changes = [] seen = set() - for arg in args: - path = get_python_path(arg) - if path not in seen: - changes.append(path) - seen.add(path) + for additional_path in additional_paths: + if additional_path not in seen: + changes.append(additional_path) + seen.add(additional_path) sys.path[:] = changes + sys.path return original @@ -95,7 +108,23 @@ def fix_import_path(args: Sequence[str]) -> Iterator[None]: We avoid adding duplicate directories to sys.path. `sys.path` is reset to its original value upon exiting this context. """ - original = _patch_sys_path(args) + # TODO: Remove deprecated function + warnings.warn( + "fix_import_path has been deprecated because it relies on auto-magic package path " + "discovery which is implemented by get_python_path that is deprecated. " + "Use augmented_sys_path and pass additional sys.path entries as an argument obtained from " + "discover_package_path.", + DeprecationWarning, + stacklevel=2, + ) + with augmented_sys_path([discover_package_path(arg, []) for arg in args]): + yield + + +@contextlib.contextmanager +def augmented_sys_path(additional_paths: Sequence[str]) -> Iterator[None]: + """Augment 'sys.path' by adding non-existent entries from additional_paths.""" + original = _augment_sys_path(additional_paths) try: yield finally: diff --git a/pylint/pyreverse/main.py b/pylint/pyreverse/main.py index 8a2055c411..13669d5b4a 100644 --- a/pylint/pyreverse/main.py +++ b/pylint/pyreverse/main.py @@ -13,7 +13,8 @@ from pylint import constants from pylint.config.arguments_manager import _ArgumentsManager from pylint.config.arguments_provider import _ArgumentsProvider -from pylint.lint.utils import fix_import_path +from pylint.lint import discover_package_path +from pylint.lint.utils import augmented_sys_path from pylint.pyreverse import writer from pylint.pyreverse.diadefslib import DiadefsHandler from pylint.pyreverse.inspector import Linker, project_from_files @@ -203,6 +204,17 @@ "help": "set the output directory path.", }, ), + ( + "source-roots", + { + "type": "paths_csv", + "metavar": "[,...]", + "default": (), + "help": "Add paths to the list of the source roots. The source root is an absolute " + "path or a path relative to the current working directory used to " + "determine a package namespace for modules located under the source root.", + }, + ), ) @@ -235,7 +247,10 @@ def run(self, args: list[str]) -> int: if not args: print(self.help()) return 1 - with fix_import_path(args): + extra_packages_paths = list( + {discover_package_path(arg, self.config.source_roots) for arg in args} + ) + with augmented_sys_path(extra_packages_paths): project = project_from_files( args, project_name=self.config.project, diff --git a/pylint/testutils/pyreverse.py b/pylint/testutils/pyreverse.py index fc20b5453f..7a61ff5fe6 100644 --- a/pylint/testutils/pyreverse.py +++ b/pylint/testutils/pyreverse.py @@ -67,6 +67,7 @@ def __init__( class TestFileOptions(TypedDict): + source_roots: list[str] output_formats: list[str] command_line_args: list[str] @@ -97,7 +98,11 @@ def get_functional_test_files( test_files.append( FunctionalPyreverseTestfile( source=path, - options={"output_formats": ["mmd"], "command_line_args": []}, + options={ + "source_roots": [], + "output_formats": ["mmd"], + "command_line_args": [], + }, ) ) return test_files @@ -106,7 +111,9 @@ def get_functional_test_files( def _read_config(config_file: Path) -> TestFileOptions: config = configparser.ConfigParser() config.read(str(config_file)) + source_roots = config.get("testoptions", "source_roots", fallback=None) return { + "source_roots": source_roots.split(",") if source_roots else [], "output_formats": config.get( "testoptions", "output_formats", fallback="mmd" ).split(","), diff --git a/tests/lint/unittest_expand_modules.py b/tests/lint/unittest_expand_modules.py index 1c3f23b00c..a8c46498cb 100644 --- a/tests/lint/unittest_expand_modules.py +++ b/tests/lint/unittest_expand_modules.py @@ -151,6 +151,7 @@ def test_expand_modules( ignore_list_re: list[re.Pattern[str]] = [] modules, errors = expand_modules( files_or_modules, + [], ignore_list, ignore_list_re, self.linter.config.ignore_paths, @@ -180,6 +181,7 @@ def test_expand_modules_deduplication( ignore_list_re: list[re.Pattern[str]] = [] modules, errors = expand_modules( files_or_modules, + [], ignore_list, ignore_list_re, self.linter.config.ignore_paths, @@ -209,6 +211,7 @@ def test_expand_modules_with_ignore( ignore_list_re: list[re.Pattern[str]] = [] modules, errors = expand_modules( files_or_modules, + [], ignore_list, ignore_list_re, self.linter.config.ignore_paths, diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 3c91eba7f3..23cde49674 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -39,8 +39,7 @@ _warn_about_old_home, ) from pylint.exceptions import InvalidMessageError -from pylint.lint import PyLinter -from pylint.lint.utils import fix_import_path +from pylint.lint import PyLinter, expand_modules from pylint.message import Message from pylint.reporters import text from pylint.testutils import create_files @@ -117,8 +116,17 @@ def fake_path() -> Iterator[list[str]]: sys.path[:] = orig +def test_deprecated() -> None: + """Test that fix_import_path() and get_python_path() are deprecated""" + with tempdir(): + create_files(["__init__.py"]) + with pytest.deprecated_call(): + with lint.fix_import_path([""]): + expand_modules.get_python_path("__init__.py") + + def test_no_args(fake_path: list[str]) -> None: - with lint.fix_import_path([]): + with lint.augmented_sys_path([]): assert sys.path == fake_path assert sys.path == fake_path @@ -131,8 +139,12 @@ def test_one_arg(fake_path: list[str], case: list[str]) -> None: create_files(["a/b/__init__.py"]) expected = [join(chroot, "a")] + fake_path + extra_sys_paths = [ + expand_modules.discover_package_path(arg, []) for arg in case + ] + assert sys.path == fake_path - with lint.fix_import_path(case): + with lint.augmented_sys_path(extra_sys_paths): assert sys.path == expected assert sys.path == fake_path @@ -151,8 +163,12 @@ def test_two_similar_args(fake_path: list[str], case: list[str]) -> None: create_files(["a/b/__init__.py", "a/c/__init__.py"]) expected = [join(chroot, "a")] + fake_path + extra_sys_paths = [ + expand_modules.discover_package_path(arg, []) for arg in case + ] + assert sys.path == fake_path - with lint.fix_import_path(case): + with lint.augmented_sys_path(extra_sys_paths): assert sys.path == expected assert sys.path == fake_path @@ -173,8 +189,12 @@ def test_more_args(fake_path: list[str], case: list[str]) -> None: for suffix in (sep.join(("a", "b")), "a", sep.join(("a", "e"))) ] + fake_path + extra_sys_paths = [ + expand_modules.discover_package_path(arg, []) for arg in case + ] + assert sys.path == fake_path - with lint.fix_import_path(case): + with lint.augmented_sys_path(extra_sys_paths): assert sys.path == expected assert sys.path == fake_path @@ -1188,6 +1208,39 @@ def test_recursive_ignore(ignore_parameter: str, ignore_parameter_value: str) -> assert module in linted_file_paths +def test_recursive_implicit_namespace() -> None: + run = Run( + [ + "--verbose", + "--recursive", + "y", + "--source-roots", + join(REGRTEST_DATA_DIR, "pep420", "basic", "project"), + join(REGRTEST_DATA_DIR, "pep420", "basic"), + ], + exit=False, + ) + run.linter.set_reporter(testutils.GenericTestReporter()) + run.linter.check([join(REGRTEST_DATA_DIR, "pep420", "basic")]) + assert run.linter.file_state.base_name == "namespace.package" + + +def test_recursive_implicit_namespace_wrapper() -> None: + run = Run( + [ + "--recursive", + "y", + "--source-roots", + join(REGRTEST_DATA_DIR, "pep420", "wrapper", "project"), + join(REGRTEST_DATA_DIR, "pep420", "wrapper"), + ], + exit=False, + ) + run.linter.set_reporter(testutils.GenericTestReporter()) + run.linter.check([join(REGRTEST_DATA_DIR, "pep420", "wrapper")]) + assert run.linter.reporter.messages == [] + + def test_relative_imports(initialized_linter: PyLinter) -> None: """Regression test for https://github.com/PyCQA/pylint/issues/3651""" linter = initialized_linter @@ -1235,8 +1288,10 @@ def test_import_sibling_module_from_namespace(initialized_linter: PyLinter) -> N """ ) os.chdir("namespace") + extra_sys_paths = [expand_modules.discover_package_path(tmpdir, [])] + # Add the parent directory to sys.path - with fix_import_path([tmpdir]): + with lint.augmented_sys_path(extra_sys_paths): linter.check(["submodule2.py"]) assert not linter.stats.by_msg @@ -1257,6 +1312,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) - with fix_import_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/primer/packages_to_prime.json b/tests/primer/packages_to_prime.json index 547c3ea584..fa14577730 100644 --- a/tests/primer/packages_to_prime.json +++ b/tests/primer/packages_to_prime.json @@ -61,7 +61,8 @@ }, "poetry-core": { "branch": "main", - "directories": ["src/poetry/core"], - "url": "https://github.com/python-poetry/poetry-core" + "directories": ["src"], + "url": "https://github.com/python-poetry/poetry-core", + "pylint_additional_args": ["--recursive=y", "--source-roots=src"] } } diff --git a/tests/pyreverse/conftest.py b/tests/pyreverse/conftest.py index a37e4bde1a..02a19e6139 100644 --- a/tests/pyreverse/conftest.py +++ b/tests/pyreverse/conftest.py @@ -9,7 +9,7 @@ import pytest from astroid.nodes.scoped_nodes import Module -from pylint.lint import fix_import_path +from pylint.lint import augmented_sys_path, discover_package_path from pylint.pyreverse.inspector import Project, project_from_files from pylint.testutils.pyreverse import PyreverseConfig from pylint.typing import GetProjectCallable @@ -74,7 +74,7 @@ def _get_project(module: str, name: str | None = "No Name") -> Project: def _astroid_wrapper(func: Callable[[str], Module], modname: str) -> Module: return func(modname) - with fix_import_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/functional/class_diagrams/namespaces/pep420/pep420.dot b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.dot new file mode 100644 index 0000000000..b01a46f387 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.dot @@ -0,0 +1,5 @@ +digraph "classes" { +rankdir=BT +charset="utf-8" +"namespaces.pep420.pep420.PEP420" [color="black", fontcolor="black", label=<{PEP420|
|}>, shape="record", style="solid"]; +} diff --git a/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.mmd b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.mmd new file mode 100644 index 0000000000..4deda423bd --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.mmd @@ -0,0 +1,3 @@ +classDiagram + class PEP420 { + } diff --git a/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.puml b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.puml new file mode 100644 index 0000000000..5819158ae4 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.puml @@ -0,0 +1,5 @@ +@startuml classes +set namespaceSeparator none +class "PEP420" as namespaces.pep420.pep420.PEP420 { +} +@enduml diff --git a/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.py b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.py new file mode 100644 index 0000000000..f2d568c47b --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.py @@ -0,0 +1,2 @@ +class PEP420: + pass diff --git a/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.rc b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.rc new file mode 100644 index 0000000000..0ae3403a91 --- /dev/null +++ b/tests/pyreverse/functional/class_diagrams/namespaces/pep420/pep420.rc @@ -0,0 +1,3 @@ +[testoptions] +source_roots=../.. +output_formats=mmd,dot,puml diff --git a/tests/pyreverse/test_main.py b/tests/pyreverse/test_main.py index 4cc0573d64..1e6196270a 100644 --- a/tests/pyreverse/test_main.py +++ b/tests/pyreverse/test_main.py @@ -16,7 +16,7 @@ from _pytest.capture import CaptureFixture from _pytest.fixtures import SubRequest -from pylint.lint import fix_import_path +from pylint.lint import augmented_sys_path, discover_package_path from pylint.pyreverse import main TEST_DATA_DIR = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "data")) @@ -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 fix_import_path([TEST_DATA_DIR]): + with augmented_sys_path([discover_package_path(TEST_DATA_DIR, [])]): assert sys.path == [PROJECT_ROOT_DIR] diff --git a/tests/pyreverse/test_pyreverse_functional.py b/tests/pyreverse/test_pyreverse_functional.py index 15fd1978b6..aab500b818 100644 --- a/tests/pyreverse/test_pyreverse_functional.py +++ b/tests/pyreverse/test_pyreverse_functional.py @@ -1,7 +1,7 @@ # Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html # For details: https://github.com/PyCQA/pylint/blob/main/LICENSE # Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt - +import os from pathlib import Path import pytest @@ -13,7 +13,8 @@ ) FUNCTIONAL_DIR = Path(__file__).parent / "functional" -CLASS_DIAGRAM_TESTS = get_functional_test_files(FUNCTIONAL_DIR / "class_diagrams") +CLASS_DIAGRAMS_DIR = FUNCTIONAL_DIR / "class_diagrams" +CLASS_DIAGRAM_TESTS = get_functional_test_files(CLASS_DIAGRAMS_DIR) CLASS_DIAGRAM_TEST_IDS = [testfile.source.stem for testfile in CLASS_DIAGRAM_TESTS] @@ -24,9 +25,23 @@ ) def test_class_diagrams(testfile: FunctionalPyreverseTestfile, tmp_path: Path) -> None: input_file = testfile.source + input_path = os.path.dirname(input_file) + if testfile.options["source_roots"]: + source_roots = ",".join( + [ + os.path.realpath( + os.path.expanduser(os.path.join(input_path, source_root)) + ) + for source_root in testfile.options["source_roots"] + ] + ) + else: + source_roots = "" for output_format in testfile.options["output_formats"]: with pytest.raises(SystemExit) as sys_exit: args = ["-o", f"{output_format}", "-d", str(tmp_path)] + if source_roots: + args += ["--source-roots", source_roots] args.extend(testfile.options["command_line_args"]) args += [str(input_file)] Run(args) diff --git a/tests/regrtest_data/pep420/basic/project/namespace/package/__init__.py b/tests/regrtest_data/pep420/basic/project/namespace/package/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/__init__.py b/tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/__init__.py new file mode 100644 index 0000000000..8bd4f6df2e --- /dev/null +++ b/tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/__init__.py @@ -0,0 +1,5 @@ +""" +A drop-in replacement for `logging`. +""" + +from .wrapper import * # noqa: F403 diff --git a/tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/wrapper/__init__.py b/tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/wrapper/__init__.py new file mode 100644 index 0000000000..50a2ac8141 --- /dev/null +++ b/tests/regrtest_data/pep420/wrapper/project/namespace/package/logging/wrapper/__init__.py @@ -0,0 +1,5 @@ +""" +A wrapper for original logging module +""" + +from logging import * # noqa: F403 diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index 96e67517ec..9134d485bc 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -10,6 +10,7 @@ import argparse import os +import sys from concurrent.futures import ProcessPoolExecutor from concurrent.futures.process import BrokenProcessPool from pickle import PickleError @@ -21,7 +22,7 @@ import pylint.interfaces import pylint.lint.parallel from pylint.checkers import BaseRawFileChecker -from pylint.lint import PyLinter +from pylint.lint import PyLinter, augmented_sys_path from pylint.lint.parallel import _worker_check_single_file as worker_check_single_file from pylint.lint.parallel import _worker_initialize as worker_initialize from pylint.lint.parallel import check_parallel @@ -173,6 +174,14 @@ def test_worker_initialize(self) -> None: worker_initialize(linter=dill.dumps(linter)) assert isinstance(pylint.lint.parallel._worker_linter, type(linter)) + def test_worker_initialize_with_package_paths(self) -> None: + linter = PyLinter(reporter=Reporter()) + with augmented_sys_path([]): + worker_initialize( + linter=dill.dumps(linter), extra_packages_paths=["fake-path"] + ) + assert "fake-path" in sys.path + @pytest.mark.needs_two_cores def test_worker_initialize_pickling(self) -> None: """Test that we can pickle objects that standard pickling in multiprocessing can't. @@ -324,7 +333,6 @@ def test_sequential_checkers_work(self) -> None: linter, jobs=1, files=iter(single_file_container), - arguments=["--enable", "R9999"], ) assert len(linter.get_checkers()) == 2, ( "We should only have the 'main' and 'sequential-checker' " @@ -390,7 +398,9 @@ def test_invoke_single_job(self) -> None: # Invoke the lint process in a multi-process way, although we only specify one # job. check_parallel( - linter, jobs=1, files=iter(single_file_container), arguments=None + linter, + jobs=1, + files=iter(single_file_container), ) assert { @@ -504,7 +514,6 @@ def test_compare_workers_to_single_proc( linter, jobs=num_jobs, files=file_infos, - arguments=None, ) stats_check_parallel = linter.stats assert linter.msg_status == 0, "We should not fail the lint" @@ -572,7 +581,6 @@ def test_map_reduce(self, num_files: int, num_jobs: int, num_checkers: int) -> N linter, jobs=num_jobs, files=file_infos, - arguments=None, ) stats_check_parallel = linter.stats assert str(stats_single_proc.by_msg) == str( @@ -600,5 +608,5 @@ def test_no_deadlock_due_to_initializer_error(self) -> None: files=iter(single_file_container), # This will trigger an exception in the initializer for the parallel jobs # because arguments has to be an Iterable. - arguments=1, # type: ignore[arg-type] + extra_packages_paths=1, # type: ignore[arg-type] ) diff --git a/tests/test_deprecation.py b/tests/test_deprecation.py index d30e69b85d..ed57c73069 100644 --- a/tests/test_deprecation.py +++ b/tests/test_deprecation.py @@ -11,6 +11,7 @@ import pytest from astroid import nodes +from pylint import lint from pylint.checkers import BaseChecker from pylint.checkers.mapreduce_checker import MapReduceMixin from pylint.config import load_results, save_results @@ -106,3 +107,16 @@ def test_collectblocklines() -> None: state = FileState("foo", MessageDefinitionStore()) with pytest.warns(DeprecationWarning): state.collect_block_lines(MessageDefinitionStore(), nodes.Module("foo")) + + +def test_patch_sys_path() -> None: + """Test that _patch_sys_path() is deprecated""" + with pytest.deprecated_call(): + lint._patch_sys_path([]) + + +def test_fix_import_path() -> None: + """Test that fix_import_path() is deprecated""" + with pytest.deprecated_call(): + with lint.fix_import_path([]): + pass From 0e3986b2893c9f28e360bf735e9fcd0f99860c92 Mon Sep 17 00:00:00 2001 From: Alexey Pelykh Date: Tue, 7 Feb 2023 18:23:49 +0100 Subject: [PATCH 2/3] Update doc/user_guide/usage/run.rst Co-authored-by: Andreas Finkler <3929834+DudeNr33@users.noreply.github.com> --- doc/user_guide/usage/run.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/user_guide/usage/run.rst b/doc/user_guide/usage/run.rst index fb298dae9a..cee6f362ff 100644 --- a/doc/user_guide/usage/run.rst +++ b/doc/user_guide/usage/run.rst @@ -48,9 +48,9 @@ or if ``directory`` is in the python path. With implicit namespace packages -------------------------------- -If the analyzed sources use the Implicit Namespace Packages (PEP 420), the source root(s) should -be specified to Pylint using the ``--source-roots`` option. Otherwise, the package names are -detected incorrectly, since the Implicit Namespace Packages don't contain the ``__init__.py``. +If the analyzed sources use implicit namespace packages (PEP 420), the source root(s) should +be specified using the ``--source-roots`` option. Otherwise, the package names are +detected incorrectly, since implicit namespace packages don't contain an ``__init__.py``. Command line options -------------------- From 9428ba426ee718632b5dd2626b2de54dabb39a57 Mon Sep 17 00:00:00 2001 From: Alexey Pelykh Date: Tue, 7 Feb 2023 18:23:57 +0100 Subject: [PATCH 3/3] Update doc/whatsnew/fragments/8154.feature Co-authored-by: Andreas Finkler <3929834+DudeNr33@users.noreply.github.com> --- doc/whatsnew/fragments/8154.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/whatsnew/fragments/8154.feature b/doc/whatsnew/fragments/8154.feature index 840540de29..9c46a1c8da 100644 --- a/doc/whatsnew/fragments/8154.feature +++ b/doc/whatsnew/fragments/8154.feature @@ -1,3 +1,3 @@ -Support Implicit Namespace Packages (PEP 420). +Support implicit namespace packages (PEP 420). Closes #8154