From 85d39356d91b4f68eb7ed9935933fce850c8fec5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 20 Apr 2022 10:19:09 +0200 Subject: [PATCH 1/9] Don't add namespace packages to ``sys.path`` --- ChangeLog | 7 ++++++ doc/whatsnew/2.14.rst | 7 ++++++ pylint/lint/utils.py | 22 +++++++++++++++++++ test_namespace_pkg/existing/__init__.py | 7 ++++++ test_namespace_pkg/something_else/__init__.py | 0 tests/lint/unittest_lint.py | 11 ++++++++++ 6 files changed, 54 insertions(+) create mode 100644 test_namespace_pkg/existing/__init__.py create mode 100644 test_namespace_pkg/something_else/__init__.py diff --git a/ChangeLog b/ChangeLog index 7969361007..4141cac52d 100644 --- a/ChangeLog +++ b/ChangeLog @@ -17,6 +17,13 @@ Release date: TBA Closes #4301 +* Improved recognition of namespace packages. They should no longer be added to ``sys.path`` + which should prevent various issues with false positives. + Because of the difficulities with mirroring the python import mechanics we would gladly + hear feedback on this change. + + Closes #5226, Closes #2648 + * ``BaseChecker`` classes now require the ``linter`` argument to be passed. * Fix a failure to respect inline disables for ``fixme`` occurring on the last line diff --git a/doc/whatsnew/2.14.rst b/doc/whatsnew/2.14.rst index b6760cf9a3..23e815cf21 100644 --- a/doc/whatsnew/2.14.rst +++ b/doc/whatsnew/2.14.rst @@ -81,6 +81,13 @@ Other Changes Closes #4301 +* Improved recognition of namespace packages. They should no longer be added to ``sys.path`` + which should prevent various issues with false positives. + Because of the difficulities with mirroring the python import mechanics we would gladly + hear feedback on this change. + + Closes #5226, Closes #2648 + * Update ``invalid-slots-object`` message to show bad object rather than its inferred value. Closes #6101 diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index 30694c25cf..792c47f2d7 100644 --- a/pylint/lint/utils.py +++ b/pylint/lint/utils.py @@ -11,6 +11,8 @@ from datetime import datetime from pathlib import Path +from astroid import modutils + from pylint.config import PYLINT_HOME from pylint.lint.expand_modules import get_python_path @@ -71,12 +73,32 @@ def get_fatal_error_message(filepath: str, issue_template_path: Path) -> str: ) +def _is_part_of_namespace_package(filename: str) -> bool: + """Check if a file is part of a namespace package.""" + filepath = Path(filename) + try: + modname = modutils.modpath_from_file(filename) + except ImportError: + modname = [filepath.stem] + if filepath.is_dir(): + filepath = filepath / "__init__.py" + + try: + spec = modutils.file_info_from_modpath(modname) + except ImportError: + return False + + return modutils.is_namespace(spec) + + def _patch_sys_path(args: Sequence[str]) -> list[str]: original = list(sys.path) changes = [] seen = set() for arg in args: path = get_python_path(arg) + if _is_part_of_namespace_package(path): + continue if path not in seen: changes.append(path) seen.add(path) diff --git a/test_namespace_pkg/existing/__init__.py b/test_namespace_pkg/existing/__init__.py new file mode 100644 index 0000000000..6eda69e2bc --- /dev/null +++ b/test_namespace_pkg/existing/__init__.py @@ -0,0 +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 + +from existing import my_func # type: ignore[attr-defined] + +my_func() diff --git a/test_namespace_pkg/something_else/__init__.py b/test_namespace_pkg/something_else/__init__.py new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 4b738b76fa..7773dd109b 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -9,6 +9,7 @@ import argparse import os import re +import subprocess import sys import tempfile from collections.abc import Iterable, Iterator @@ -170,6 +171,16 @@ def test_more_args(fake_path, case): assert sys.path == fake_path +def test_namespace_package_sys_path() -> None: + """Test that we do not append namespace packages to sys.path.""" + result = subprocess.run( + [sys.executable, "-m", "pylint", "test_namespace_pkg/"], + check=False, + stdout=subprocess.PIPE, + ) + assert "Module import itself" not in result.stdout.decode("utf-8") + + @pytest.fixture(scope="module") def disable(): return ["I"] From e298b1bc4b80b002e99c7c7e7746851259a9f8f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Wed, 20 Apr 2022 10:27:52 +0200 Subject: [PATCH 2/9] Update pylint/lint/utils.py --- pylint/lint/utils.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/pylint/lint/utils.py b/pylint/lint/utils.py index 792c47f2d7..a08cd6d2a9 100644 --- a/pylint/lint/utils.py +++ b/pylint/lint/utils.py @@ -75,13 +75,10 @@ def get_fatal_error_message(filepath: str, issue_template_path: Path) -> str: def _is_part_of_namespace_package(filename: str) -> bool: """Check if a file is part of a namespace package.""" - filepath = Path(filename) try: modname = modutils.modpath_from_file(filename) except ImportError: - modname = [filepath.stem] - if filepath.is_dir(): - filepath = filepath / "__init__.py" + modname = [Path(filename).stem] try: spec = modutils.file_info_from_modpath(modname) From 9acac8c082c889d20b60c96825718806e8454b9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Thu, 21 Apr 2022 00:14:40 +0200 Subject: [PATCH 3/9] Apply suggestions from code review Co-authored-by: Jacob Walls --- ChangeLog | 2 +- doc/whatsnew/2.14.rst | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ChangeLog b/ChangeLog index 4141cac52d..164acaf2f1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,7 +19,7 @@ Release date: TBA * Improved recognition of namespace packages. They should no longer be added to ``sys.path`` which should prevent various issues with false positives. - Because of the difficulities with mirroring the python import mechanics we would gladly + Because of the difficulties with mirroring the python import mechanics we would gladly hear feedback on this change. Closes #5226, Closes #2648 diff --git a/doc/whatsnew/2.14.rst b/doc/whatsnew/2.14.rst index 23e815cf21..49264bf4c3 100644 --- a/doc/whatsnew/2.14.rst +++ b/doc/whatsnew/2.14.rst @@ -83,7 +83,7 @@ Other Changes * Improved recognition of namespace packages. They should no longer be added to ``sys.path`` which should prevent various issues with false positives. - Because of the difficulities with mirroring the python import mechanics we would gladly + Because of the difficulties with mirroring the python import mechanics we would gladly hear feedback on this change. Closes #5226, Closes #2648 From af6387f1f561778837bdc4408cbf0475b0399da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Fri, 29 Apr 2022 18:33:49 +0200 Subject: [PATCH 4/9] Update tests/lint/unittest_lint.py Co-authored-by: Jacob Walls --- tests/lint/unittest_lint.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 7773dd109b..a354f58518 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -173,12 +173,10 @@ def test_more_args(fake_path, case): def test_namespace_package_sys_path() -> None: """Test that we do not append namespace packages to sys.path.""" - result = subprocess.run( - [sys.executable, "-m", "pylint", "test_namespace_pkg/"], - check=False, - stdout=subprocess.PIPE, - ) - assert "Module import itself" not in result.stdout.decode("utf-8") + pylint_output = StringIO() + reporter = text.TextReporter(pylint_output) + Run(["test_namespace_pkg/"], reporter=reporter, exit=False) + assert "Module import itself" not in pylint_output.getvalue() @pytest.fixture(scope="module") From 421775937253b29c817045d18914f1131078fbbc Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 29 Apr 2022 16:34:45 +0000 Subject: [PATCH 5/9] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/lint/unittest_lint.py | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index a354f58518..c7c9fdbcb3 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -9,7 +9,6 @@ import argparse import os import re -import subprocess import sys import tempfile from collections.abc import Iterable, Iterator From 3d3e92683b274c4a734d57914eb7658ee1f43524 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 30 Apr 2022 15:35:13 +0200 Subject: [PATCH 6/9] Change test structure --- test_namespace_pkg/existing/__init__.py | 7 ------- tests/lint/test_namespace_packages.py | 21 +++++++++++++++++++ tests/lint/unittest_lint.py | 8 ------- .../namespace_import_self/else}/__init__.py | 0 .../existing/__init__.py | 3 +++ 5 files changed, 24 insertions(+), 15 deletions(-) delete mode 100644 test_namespace_pkg/existing/__init__.py create mode 100644 tests/lint/test_namespace_packages.py rename {test_namespace_pkg/something_else => tests/regrtest_data/namespace_import_self/else}/__init__.py (100%) create mode 100644 tests/regrtest_data/namespace_import_self/existing/__init__.py diff --git a/test_namespace_pkg/existing/__init__.py b/test_namespace_pkg/existing/__init__.py deleted file mode 100644 index 6eda69e2bc..0000000000 --- a/test_namespace_pkg/existing/__init__.py +++ /dev/null @@ -1,7 +0,0 @@ -# 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 - -from existing import my_func # type: ignore[attr-defined] - -my_func() diff --git a/tests/lint/test_namespace_packages.py b/tests/lint/test_namespace_packages.py new file mode 100644 index 0000000000..8b03575816 --- /dev/null +++ b/tests/lint/test_namespace_packages.py @@ -0,0 +1,21 @@ +# 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 + +"""Tests related to linting of namespace packages.""" + +from io import StringIO + +from pylint.reporters.text import TextReporter +from pylint.testutils._run import _Run as Run + + +def test_namespace_package_sys_path() -> None: + """Test that we do not append namespace packages to sys.path. + + The test package is based on https://github.com/PyCQA/pylint/issues/2648. + """ + pylint_output = StringIO() + reporter = TextReporter(pylint_output) + Run(["tests/regrtest_data/namespace_import_self/"], reporter=reporter, exit=False) + assert "Module import itself" not in pylint_output.getvalue() diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index e2d41040fb..78b89974c8 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -171,14 +171,6 @@ def test_more_args(fake_path, case): assert sys.path == fake_path -def test_namespace_package_sys_path() -> None: - """Test that we do not append namespace packages to sys.path.""" - pylint_output = StringIO() - reporter = text.TextReporter(pylint_output) - Run(["test_namespace_pkg/"], reporter=reporter, exit=False) - assert "Module import itself" not in pylint_output.getvalue() - - @pytest.fixture(scope="module") def disable(): return ["I"] diff --git a/test_namespace_pkg/something_else/__init__.py b/tests/regrtest_data/namespace_import_self/else/__init__.py similarity index 100% rename from test_namespace_pkg/something_else/__init__.py rename to tests/regrtest_data/namespace_import_self/else/__init__.py diff --git a/tests/regrtest_data/namespace_import_self/existing/__init__.py b/tests/regrtest_data/namespace_import_self/existing/__init__.py new file mode 100644 index 0000000000..1b93e7d82b --- /dev/null +++ b/tests/regrtest_data/namespace_import_self/existing/__init__.py @@ -0,0 +1,3 @@ +from existing import my_func # type: ignore[attr-defined] + +my_func() From ff9c1556b09cdcac8276b1e84433c7beb9b241b2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 30 Apr 2022 15:43:59 +0200 Subject: [PATCH 7/9] Update expand modules test --- tests/lint/unittest_expand_modules.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/tests/lint/unittest_expand_modules.py b/tests/lint/unittest_expand_modules.py index 2e1875d968..36fd7ed4d7 100644 --- a/tests/lint/unittest_expand_modules.py +++ b/tests/lint/unittest_expand_modules.py @@ -69,6 +69,14 @@ def test__is_in_ignore_list_re_match() -> None: "path": str(TEST_DIRECTORY / "lint/test_pylinter.py"), } +test_namespace_packages = { + "basename": "lint", + "basepath": INIT_PATH, + "isarg": False, + "name": "lint.test_namespace_packages", + "path": str(TEST_DIRECTORY / "lint/test_namespace_packages.py"), +} + init_of_package = { "basename": "lint", "basepath": INIT_PATH, @@ -98,6 +106,7 @@ class Checker(BaseChecker): [str(Path(__file__).parent)], [ init_of_package, + test_namespace_packages, test_pylinter, test_utils, this_file_from_init, From 1dceed3b1991afff1d61d5910047f7716f073a1b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 30 Apr 2022 16:07:15 +0200 Subject: [PATCH 8/9] Assert on messages --- tests/lint/test_namespace_packages.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/lint/test_namespace_packages.py b/tests/lint/test_namespace_packages.py index 8b03575816..e3fc901f6e 100644 --- a/tests/lint/test_namespace_packages.py +++ b/tests/lint/test_namespace_packages.py @@ -17,5 +17,9 @@ def test_namespace_package_sys_path() -> None: """ pylint_output = StringIO() reporter = TextReporter(pylint_output) - Run(["tests/regrtest_data/namespace_import_self/"], reporter=reporter, exit=False) - assert "Module import itself" not in pylint_output.getvalue() + runner = Run( + ["tests/regrtest_data/namespace_import_self/"], + reporter=reporter, + exit=False, + ) + assert not runner.linter.reporter.messages From 3767d1f70553865eb0db7581471256f1fd3416d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Dani=C3=ABl=20van=20Noord?= <13665637+DanielNoord@users.noreply.github.com> Date: Sat, 30 Apr 2022 16:25:41 +0200 Subject: [PATCH 9/9] Potential fix for test --- tests/lint/test_namespace_packages.py | 2 +- .../regrtest_data/namespace_import_self/existing/__init__.py | 3 --- tests/regrtest_data/namespace_import_self/pylint/__init__.py | 5 +++++ 3 files changed, 6 insertions(+), 4 deletions(-) delete mode 100644 tests/regrtest_data/namespace_import_self/existing/__init__.py create mode 100644 tests/regrtest_data/namespace_import_self/pylint/__init__.py diff --git a/tests/lint/test_namespace_packages.py b/tests/lint/test_namespace_packages.py index e3fc901f6e..6ac96f09ec 100644 --- a/tests/lint/test_namespace_packages.py +++ b/tests/lint/test_namespace_packages.py @@ -22,4 +22,4 @@ def test_namespace_package_sys_path() -> None: reporter=reporter, exit=False, ) - assert not runner.linter.reporter.messages + assert not runner.linter.stats.by_msg diff --git a/tests/regrtest_data/namespace_import_self/existing/__init__.py b/tests/regrtest_data/namespace_import_self/existing/__init__.py deleted file mode 100644 index 1b93e7d82b..0000000000 --- a/tests/regrtest_data/namespace_import_self/existing/__init__.py +++ /dev/null @@ -1,3 +0,0 @@ -from existing import my_func # type: ignore[attr-defined] - -my_func() diff --git a/tests/regrtest_data/namespace_import_self/pylint/__init__.py b/tests/regrtest_data/namespace_import_self/pylint/__init__.py new file mode 100644 index 0000000000..cce5e53024 --- /dev/null +++ b/tests/regrtest_data/namespace_import_self/pylint/__init__.py @@ -0,0 +1,5 @@ +"""Module that imports from its own namespace.""" + +from pylint import run_pylint + +run_pylint()