From 3a73b965a63b6867f54455283b63557da77544b2 Mon Sep 17 00:00:00 2001 From: Pedro Algarvio Date: Fri, 2 Feb 2024 11:53:19 +0000 Subject: [PATCH] Start using pre-commit to maintain this project --- .pre-commit-config.yaml | 55 ++++ LICENSE | 1 - README.md | 2 +- pyproject.toml | 128 ++++++++ saltpylint/__init__.py | 1 - saltpylint/blacklist.py | 613 +++++++++++++++++++++++---------------- saltpylint/checkers.py | 23 -- saltpylint/dunder_del.py | 49 +--- saltpylint/fileperms.py | 140 +++++---- saltpylint/smartup.py | 42 +-- saltpylint/thirdparty.py | 154 +++++----- saltpylint/virt.py | 83 +++--- setup.cfg | 2 +- 13 files changed, 762 insertions(+), 531 deletions(-) create mode 100644 .pre-commit-config.yaml delete mode 100644 saltpylint/checkers.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..e1ededb --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,55 @@ +--- +default_language_version: + python: python3 + +exclude: ^(doc/_static/.*|doc/_themes/.*)$ +repos: + + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: check-merge-conflict # Check for files that contain merge conflict strings. + - id: trailing-whitespace # Trims trailing whitespace. + args: + - --markdown-linebreak-ext=md + - id: mixed-line-ending # Replaces or checks mixed line ending. + args: + - --fix=lf + - id: end-of-file-fixer # Makes sure files end in a newline and only a newline. + - id: check-ast # Simply check whether files parse as valid python. + - id: check-case-conflict # Check for files with names that would conflict on a + # case-insensitive filesystem like MacOS HFS+ or Windows FAT. + - id: check-json # Attempts to load all json files to verify syntax. + - id: check-symlinks # Checks for symlinks which do not point to anything. + - id: debug-statements # Check for debugger imports and py37+ breakpoint() calls in python source. + - id: fix-byte-order-marker # removes UTF-8 byte order marker + - id: forbid-submodules # forbids any submodules in the repository. + - id: fix-encoding-pragma # Remove `# -*- coding: utf-8 -*-` from the top of python files. + args: + - --remove + + # ----- Code Formatting and Analysis ----------------------------------------------------------> + - repo: https://github.com/charliermarsh/ruff-pre-commit + rev: "v0.2.0" + hooks: + - id: ruff + args: + - --fix + exclude: (.pre-commit-hooks/.*|docs/.*)\.py + + - repo: https://github.com/psf/black + rev: 24.1.1 + hooks: + - id: black + args: [-l 100] + exclude: src/saf/version.py + + - repo: https://github.com/asottile/blacken-docs + rev: 1.16.0 + hooks: + - id: blacken-docs + args: [--skip-errors] + files: ^(docs/.*\.rst|.*\.py)$ + additional_dependencies: + - black==24.1.1 + # <---- Code Formatting and Analysis ----------------------------------------------------------- diff --git a/LICENSE b/LICENSE index e06d208..5c304d1 100644 --- a/LICENSE +++ b/LICENSE @@ -199,4 +199,3 @@ Apache License WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - diff --git a/README.md b/README.md index 0314621..0264bd3 100644 --- a/README.md +++ b/README.md @@ -1 +1 @@ -# salt-pylint \ No newline at end of file +# salt-pylint diff --git a/pyproject.toml b/pyproject.toml index d213840..b18499a 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -5,3 +5,131 @@ build-backend = "setuptools.build_meta" [tool.setuptools_scm] write_to = "saltpylint/version.py" write_to_template = "# pylint: skip-file\n\n__version__ = \"{version}\"\n" + +[tool.ruff] +line-length = 120 +show-fixes = true +output-format = "grouped" +target-version = "py38" +respect-gitignore = true +src = [ + "saltpylint", + "tests", + "tools", + "examples", +] +extend-exclude = [ + ".nox/**", +] +extend-include = [ + "setup.py", + "noxfile.py", +] +builtins = [ + "__opts__", + "__salt__", +# "__context__", +# "__grains__", +# "__pillar__", + "__salt_system_encoding__", +] + +[tool.ruff.lint] +select = ["ALL"] +ignore = [ + # D* pydocstyle + "D200", # Reformat to one line + "D212", # Remove whitespace after opening quotes + "COM", # flake8-commas - Black takes care of this + "ERA", # eradicate + "SIM108", # Use ternary operator `A = X if Y else Z` instead of `if`-`else`-block + "PERF203", # `try`-`except` within a loop incurs performance overhead" + "PERF401", # Use a list comprehension to create a transformed list + "PERF402", # Use `list` or `list.copy` to create a copy of a list + "ANN", # Type annotations +] +ignore-init-module-imports = true + +[tool.ruff.lint.per-file-ignores] +"**/*.py" = [ + "D100", # Missing docstring in public module + "D101", # Missing docstring in public class + "D102", # Missing docstring in public method + "D103", # Missing docstring in public function + "D104", # Missing docstring in public package + "D107", # Missing docstring in `__init__` + "D205", # 1 blank line required between summary line and description + "D415", # First line should end with a period, question mark, or exclamation point + "PTH", # use pathlib.Path + "ARG001", # Unused function argument: * + "ARG002", # Unused method argument: * +] +"setup.py" = [ + "D", +] +"noxfile.py" = [ + "D", + "ANN", + "SLF001", + "C901", + "PLR0912", + "DTZ005", + "FBT002", + "PLR0913", # Too many arguments to function call" + "PLR0915", # Too many statements +] +"saltpylint/blacklist.py" = [ + "BLE001", # Do not catch blind exception: `Exception` +] +"saltpylint/thirdparty.py" = [ + "BLE001", # Do not catch blind exception: `Exception` +] +"tools/**/*.py" = [ + "ANN201", # Missing return type annotation for public function" + "D104", # Missing docstring in public package + "FBT001", # Boolean positional arg in function definition + "FBT002", # Boolean default value in function definition +] +"tests/**/*.py" = [ + "ANN", # Ignore missing type annotations in tests + "ARG001", # Unused function argument + "DTZ003", # The use of `datetime.datetime.utcnow()` is not allowed, use `datetime.datetime.now(tz=)` instead + "PLR2004", # Magic value used in comparison, consider replacing 3 with a constant variable + "PT001", # use @pytest.fixture() over @pytest.fixture + "PT023", # use @pytest.mark.() over @pytest.mark. + "RET504", # Unnecessary variable assignment before `return` statement" + "S101", # Ignore the use of 'assert ...' in tests + "S603", # `subprocess` call: check for execution of untrusted input + "SIM117", # Use a single `with` statement with multiple contexts instead of nested `with` statements + "TCH002", # Move third-party import into a type-checking block + "TCH003", # Move standard library import `pathlib` into a type-checking block +] + +[tool.ruff.lint.flake8-quotes] +docstring-quotes = "double" + +[tool.ruff.lint.pydocstyle] +# Use Google-style docstrings. +convention = "google" + +[tool.ruff.lint.isort] +combine-as-imports = false +force-single-line = true +known-first-party = ["src"] +forced-separate = ["tests"] + +[tool.ruff.lint.pep8-naming] +ignore-names = [ + "__virtual__", +] +[tool.ruff.lint.pyupgrade] +# Preserve types, even if a file imports `from __future__ import annotations`. +keep-runtime-typing = true + +[tool.ruff.lint.mccabe] +max-complexity = 45 + +[tool.ruff.lint.pylint] +max-branches = 45 +max-returns = 10 +max-statements = 100 diff --git a/saltpylint/__init__.py b/saltpylint/__init__.py index 40a96af..e69de29 100644 --- a/saltpylint/__init__.py +++ b/saltpylint/__init__.py @@ -1 +0,0 @@ -# -*- coding: utf-8 -*- diff --git a/saltpylint/blacklist.py b/saltpylint/blacklist.py index ff1722c..6681b5d 100644 --- a/saltpylint/blacklist.py +++ b/saltpylint/blacklist.py @@ -1,250 +1,341 @@ -# -*- coding: utf-8 -*- -''' - :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)` - :copyright: © 2017 by the SaltStack Team, see AUTHORS for more details. - :license: Apache 2.0, see LICENSE for more details. +""" +saltpylint.blacklist +~~~~~~~~~~~~~~~~~~~~ +Checks blacklisted imports and code usage on salt +""" - saltpylint.blacklist - ~~~~~~~~~~~~~~~~~~~~ - - Checks blacklisted imports and code usage on salt -''' - -# Import python libs -from __future__ import absolute_import -import os +import contextlib import fnmatch +import os -# Import pylint libs import astroid -from saltpylint.checkers import BaseChecker, utils +from pylint.checkers import BaseChecker +from pylint.checkers import utils BLACKLISTED_IMPORTS_MSGS = { - 'E9402': ('Uses of a blacklisted module %r: %s', - 'blacklisted-module', - 'Used a module marked as blacklisted is imported.'), - 'E9403': ('Uses of a blacklisted external module %r: %s', - 'blacklisted-external-module', - 'Used a module marked as blacklisted is imported.'), - 'E9404': ('Uses of a blacklisted import %r: %s', - 'blacklisted-import', - 'Used an import marked as blacklisted.'), - 'E9405': ('Uses of an external blacklisted import %r: %s', - 'blacklisted-external-import', - 'Used an external import marked as blacklisted.'), - 'E9406': ('Uses of blacklisted test module execution code: %s', - 'blacklisted-test-module-execution', - 'Uses of blacklisted test module execution code.'), - 'E9407': ('Uses of blacklisted sys.path updating through \'ensure_in_syspath\'. ' - 'Please remove the import and any calls to \'ensure_in_syspath()\'.', - 'blacklisted-syspath-update', - 'Uses of blacklisted sys.path updating through ensure_in_syspath.'), - } + "E9402": ( + "Uses of a blacklisted module %r: %s", + "blacklisted-module", + "Used a module marked as blacklisted is imported.", + ), + "E9403": ( + "Uses of a blacklisted external module %r: %s", + "blacklisted-external-module", + "Used a module marked as blacklisted is imported.", + ), + "E9404": ( + "Uses of a blacklisted import %r: %s", + "blacklisted-import", + "Used an import marked as blacklisted.", + ), + "E9405": ( + "Uses of an external blacklisted import %r: %s", + "blacklisted-external-import", + "Used an external import marked as blacklisted.", + ), + "E9406": ( + "Uses of blacklisted test module execution code: %s", + "blacklisted-test-module-execution", + "Uses of blacklisted test module execution code.", + ), + "E9407": ( + "Uses of blacklisted sys.path updating through 'ensure_in_syspath'. " + "Please remove the import and any calls to 'ensure_in_syspath()'.", + "blacklisted-syspath-update", + "Uses of blacklisted sys.path updating through ensure_in_syspath.", + ), +} class BlacklistedImportsChecker(BaseChecker): - - name = 'blacklisted-imports' + name = "blacklisted-imports" msgs = BLACKLISTED_IMPORTS_MSGS priority = -2 def open(self): - self.blacklisted_modules = ('salttesting', - 'integration', - 'unit', - 'mock', - 'six', - 'distutils.version', - 'unittest', - 'unittest2') + self.blacklisted_modules = ( + "salttesting", + "integration", + "unit", + "mock", + "six", + "distutils.version", + "unittest", + "unittest2", + ) def visit_import(self, node): - '''triggered when an import statement is seen''' + """Triggered when an import statement is seen.""" module_filename = node.root().file - if fnmatch.fnmatch(module_filename, '__init__.py*') and \ - not fnmatch.fnmatch(module_filename, 'test_*.py*'): + if fnmatch.fnmatch(module_filename, "__init__.py*") and not fnmatch.fnmatch( + module_filename, + "test_*.py*", + ): return - modnode = node.root() + node.root() names = [name for name, _ in node.names] for name in names: self._check_blacklisted_module(node, name) def visit_importfrom(self, node): - '''triggered when a from statement is seen''' + """Triggered when a from statement is seen.""" module_filename = node.root().file - if fnmatch.fnmatch(module_filename, '__init__.py*') and \ - not fnmatch.fnmatch(module_filename, 'test_*.py*'): + if fnmatch.fnmatch(module_filename, "__init__.py*") and not fnmatch.fnmatch( + module_filename, + "test_*.py*", + ): return basename = node.modname self._check_blacklisted_module(node, basename) def _check_blacklisted_module(self, node, mod_path): - '''check if the module is blacklisted''' + """Check if the module is blacklisted.""" for mod_name in self.blacklisted_modules: - if mod_path == mod_name or mod_path.startswith(mod_name + '.'): + if mod_path == mod_name or mod_path.startswith(mod_name + "."): names = [] for name, name_as in node.names: if name_as: - names.append('{0} as {1}'.format(name, name_as)) + names.append(f"{name} as {name_as}") else: names.append(name) try: import_from_module = node.modname - if import_from_module == 'salttesting.helpers': + if import_from_module == "salttesting.helpers": for name in names: - if name == 'ensure_in_syspath': - self.add_message('blacklisted-syspath-update', node=node) + if name == "ensure_in_syspath": + self.add_message("blacklisted-syspath-update", node=node) continue - msg = 'Please use \'from tests.support.helpers import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from tests.support.helpers import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module in ('salttesting.mock', 'mock', 'unittest.mock', 'unittest2.mock'): + if import_from_module in ( + "salttesting.mock", + "mock", + "unittest.mock", + "unittest2.mock", + ): for name in names: - msg = 'Please use \'from tests.support.mock import {0}\''.format(name) - if import_from_module in ('salttesting.mock', 'unittest.mock', 'unittest2.mock'): - message_id = 'blacklisted-module' + msg = f"Please use 'from tests.support.mock import {name}'" + if import_from_module in ( + "salttesting.mock", + "unittest.mock", + "unittest2.mock", + ): + message_id = "blacklisted-module" else: - message_id = 'blacklisted-external-module' + message_id = "blacklisted-external-module" self.add_message(message_id, node=node, args=(mod_path, msg)) continue - if import_from_module == 'salttesting.parser': + if import_from_module == "salttesting.parser": for name in names: - msg = 'Please use \'from tests.support.parser import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from tests.support.parser import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module == 'salttesting.case': + if import_from_module == "salttesting.case": for name in names: - msg = 'Please use \'from tests.support.case import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from tests.support.case import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module == 'salttesting.unit': + if import_from_module == "salttesting.unit": for name in names: - msg = 'Please use \'from tests.support.unit import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from tests.support.unit import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module.startswith(('unittest', 'unittest2')): + if import_from_module.startswith(("unittest", "unittest2")): for name in names: - msg = 'Please use \'from tests.support.unit import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from tests.support.unit import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module == 'salttesting.mixins': + if import_from_module == "salttesting.mixins": for name in names: - msg = 'Please use \'from tests.support.mixins import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from tests.support.mixins import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module == 'six': + if import_from_module == "six": for name in names: - msg = 'Please use \'from salt.ext.six import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from salt.ext.six import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue - if import_from_module == 'distutils.version': + if import_from_module == "distutils.version": for name in names: - msg = 'Please use \'from salt.utils.versions import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + msg = f"Please use 'from salt.utils.versions import {name}'" + self.add_message("blacklisted-module", node=node, args=(mod_path, msg)) continue if names: for name in names: - if name in ('TestLoader', 'TextTestRunner', 'TestCase', 'expectedFailure', - 'TestSuite', 'skipIf', 'TestResult'): - msg = 'Please use \'from tests.support.unit import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + if name in ( + "TestLoader", + "TextTestRunner", + "TestCase", + "expectedFailure", + "TestSuite", + "skipIf", + "TestResult", + ): + msg = f"Please use 'from tests.support.unit import {name}'" + self.add_message( + "blacklisted-module", + node=node, + args=(mod_path, msg), + ) continue - if name in ('SaltReturnAssertsMixin', 'SaltMinionEventAssertsMixin'): - msg = 'Please use \'from tests.support.mixins import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + if name in ("SaltReturnAssertsMixin", "SaltMinionEventAssertsMixin"): + msg = f"Please use 'from tests.support.mixins import {name}'" + self.add_message( + "blacklisted-module", + node=node, + args=(mod_path, msg), + ) continue - if name in ('ModuleCase', 'SyndicCase', 'ShellCase', 'SSHCase'): - msg = 'Please use \'from tests.support.case import {0}\''.format(name) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + if name in ("ModuleCase", "SyndicCase", "ShellCase", "SSHCase"): + msg = f"Please use 'from tests.support.case import {name}'" + self.add_message( + "blacklisted-module", + node=node, + args=(mod_path, msg), + ) continue - if name == 'run_tests': - msg = 'Please remove the \'if __name__ == "__main__":\' section from the end of the module' - self.add_message('blacklisted-test-module-execution', node=node, args=msg) + if name == "run_tests": + self.add_message( + "blacklisted-test-module-execution", + node=node, + args=( + "Please remove the 'if __name__ == \"__main__\":' section from the " + "end of the module" + ), + ) continue - if mod_name in ('integration', 'unit'): - if name in ('SYS_TMP_DIR', - 'TMP', - 'FILES', - 'PYEXEC', - 'MOCKBIN', - 'SCRIPT_DIR', - 'TMP_STATE_TREE', - 'TMP_PRODENV_STATE_TREE', - 'TMP_CONF_DIR', - 'TMP_SUB_MINION_CONF_DIR', - 'TMP_SYNDIC_MINION_CONF_DIR', - 'TMP_SYNDIC_MASTER_CONF_DIR', - 'CODE_DIR', - 'TESTS_DIR', - 'CONF_DIR', - 'PILLAR_DIR', - 'TMP_SCRIPT_DIR', - 'ENGINES_DIR', - 'LOG_HANDLERS_DIR', - 'INTEGRATION_TEST_DIR'): - msg = 'Please use \'from tests.support.paths import {0}\''.format(name) - self.add_message('blacklisted-import', node=node, args=(mod_path, msg)) + if mod_name in ("integration", "unit"): + if name in ( + "SYS_TMP_DIR", + "TMP", + "FILES", + "PYEXEC", + "MOCKBIN", + "SCRIPT_DIR", + "TMP_STATE_TREE", + "TMP_PRODENV_STATE_TREE", + "TMP_CONF_DIR", + "TMP_SUB_MINION_CONF_DIR", + "TMP_SYNDIC_MINION_CONF_DIR", + "TMP_SYNDIC_MASTER_CONF_DIR", + "CODE_DIR", + "TESTS_DIR", + "CONF_DIR", + "PILLAR_DIR", + "TMP_SCRIPT_DIR", + "ENGINES_DIR", + "LOG_HANDLERS_DIR", + "INTEGRATION_TEST_DIR", + ): + msg = f"Please use 'from tests.support.paths import {name}'" + self.add_message( + "blacklisted-import", + node=node, + args=(mod_path, msg), + ) continue - msg = 'Please use \'from tests.{0} import {1}\''.format(mod_path, name) - self.add_message('blacklisted-import', node=node, args=(mod_path, msg)) + self.add_message( + "blacklisted-import", + node=node, + args=( + mod_path, + f"Please use 'from tests.{mod_path} import {name}'", + ), + ) continue - msg = 'Please report this error to SaltStack so we can fix it: Trying to import {0} from {1}'.format(name, mod_path) - self.add_message('blacklisted-module', node=node, args=(mod_path, msg)) + self.add_message( + "blacklisted-module", + node=node, + args=( + mod_path, + "Please report this error to SaltStack so we can fix it: " + f"Trying to import {name} from {mod_path}", + ), + ) except AttributeError: - if mod_name in ('integration', 'unit', 'mock', 'six', 'distutils.version', - 'unittest', 'unittest2'): - if mod_name in ('integration', 'unit'): - msg = 'Please use \'import tests.{0} as {0}\''.format(mod_name) - message_id = 'blacklisted-import' - elif mod_name == 'mock': - msg = 'Please use \'import tests.support.{0} as {0}\''.format(mod_name) - message_id = 'blacklisted-external-import' - elif mod_name == 'six': - msg = 'Please use \'import salt.ext.{0} as {0}\''.format(name) - message_id = 'blacklisted-external-import' - elif mod_name == 'distutils.version': - msg = 'Please use \'import salt.utils.versions\' instead' - message_id = 'blacklisted-import' - elif mod_name.startswith(('unittest', 'unittest2')): - msg = 'Please use \'import tests.support.unit as {}\' instead'.format(mod_name) - message_id = 'blacklisted-import' + if mod_name in ( + "integration", + "unit", + "mock", + "six", + "distutils.version", + "unittest", + "unittest2", + ): + if mod_name in ("integration", "unit"): + msg = f"Please use 'import tests.{mod_name} as {mod_name}'" + message_id = "blacklisted-import" + elif mod_name == "mock": + msg = f"Please use 'import tests.support.{mod_name} as {mod_name}'" + message_id = "blacklisted-external-import" + elif mod_name == "six": + msg = f"Please use 'import salt.ext.{name} as {name}'" + message_id = "blacklisted-external-import" + elif mod_name == "distutils.version": + msg = "Please use 'import salt.utils.versions' instead" + message_id = "blacklisted-import" + elif mod_name.startswith(("unittest", "unittest2")): + msg = f"Please use 'import tests.support.unit as {mod_name}' instead" + message_id = "blacklisted-import" self.add_message(message_id, node=node, args=(mod_path, msg)) continue - msg = 'Please report this error to SaltStack so we can fix it: Trying to import {0}'.format(mod_path) - self.add_message('blacklisted-import', node=node, args=(mod_path, msg)) + msg = f"Please report this error to SaltStack so we can fix it: Trying to import {mod_path}" + self.add_message("blacklisted-import", node=node, args=(mod_path, msg)) + BLACKLISTED_LOADER_USAGE_MSGS = { - 'E9501': ('Blacklisted salt loader dunder usage. Setting dunder attribute %r to module %r. ' - 'Use \'salt.support.mock\' and \'patch.dict()\' instead.', - 'unmocked-patch-dunder', - 'Uses a blacklisted salt loader dunder usage in tests.'), - 'E9502': ('Blacklisted salt loader dunder usage. Setting attribute %r to module %r. ' - 'Use \'salt.support.mock\' and \'patch()\' instead.', - 'unmocked-patch', - 'Uses a blacklisted salt loader dunder usage in tests.'), - 'E9503': ('Blacklisted salt loader dunder usage. Updating dunder attribute %r on module %r. ' - 'Use \'salt.support.mock\' and \'patch.dict()\' instead.', - 'unmocked-patch-dunder-update', - 'Uses a blacklisted salt loader dunder usage in tests.'), + "E9501": ( + "Blacklisted salt loader dunder usage. Setting dunder attribute %r to module %r. " + "Use 'salt.support.mock' and 'patch.dict()' instead.", + "unmocked-patch-dunder", + "Uses a blacklisted salt loader dunder usage in tests.", + ), + "E9502": ( + "Blacklisted salt loader dunder usage. Setting attribute %r to module %r. " + "Use 'salt.support.mock' and 'patch()' instead.", + "unmocked-patch", + "Uses a blacklisted salt loader dunder usage in tests.", + ), + "E9503": ( + "Blacklisted salt loader dunder usage. Updating dunder attribute %r on module %r. " + "Use 'salt.support.mock' and 'patch.dict()' instead.", + "unmocked-patch-dunder-update", + "Uses a blacklisted salt loader dunder usage in tests.", + ), } class BlacklistedLoaderModulesUsageChecker(BaseChecker): - - name = 'blacklisted-unmocked-patching' + name = "blacklisted-unmocked-patching" msgs = BLACKLISTED_LOADER_USAGE_MSGS priority = -2 def open(self): self.process_module = False self.salt_dunders = ( - '__opts__', '__salt__', '__runner__', '__context__', '__utils__', - '__ext_pillar__', '__thorium__', '__states__', '__serializers__', - '__ret__', '__grains__', '__pillar__', '__sdb__', '__proxy__', - '__low__', '__orchestration_jid__', '__running__', '__intance_id__', - '__lowstate__', '__env__' + "__opts__", + "__salt__", + "__runner__", + "__context__", + "__utils__", + "__ext_pillar__", + "__thorium__", + "__states__", + "__serializers__", + "__ret__", + "__grains__", + "__pillar__", + "__sdb__", + "__proxy__", + "__low__", + "__orchestration_jid__", + "__running__", + "__intance_id__", + "__lowstate__", + "__env__", ) self.imported_salt_modules = {} @@ -254,7 +345,7 @@ def close(self): def visit_module(self, node): module_filename = node.root().file - if not fnmatch.fnmatch(os.path.basename(module_filename), 'test_*.py*'): + if not fnmatch.fnmatch(os.path.basename(module_filename), "test_*.py*"): return self.process_module = True @@ -265,11 +356,11 @@ def leave_module(self, node): self.imported_salt_modules = {} def visit_import(self, node): - '''triggered when an import statement is seen''' + """Triggered when an import statement is seen.""" if self.process_module: # Store salt imported modules for module, import_as in node.names: - if not module.startswith('salt'): + if not module.startswith("salt"): continue if import_as and import_as not in self.imported_salt_modules: self.imported_salt_modules[import_as] = module @@ -278,9 +369,9 @@ def visit_import(self, node): self.imported_salt_modules[module] = module def visit_importfrom(self, node): - '''triggered when a from statement is seen''' + """Triggered when a from statement is seen.""" if self.process_module: - if not node.modname.startswith('salt'): + if not node.modname.startswith("salt"): return # Store salt imported modules for module, import_as in node.names: @@ -303,10 +394,12 @@ def visit_assign(self, node, *args): if node_left.value.attrname in self.salt_dunders: if node_left.value.expr.name in self.imported_salt_modules: self.add_message( - 'unmocked-patch-dunder-update', + "unmocked-patch-dunder-update", node=node, - args=(node_left.value.attrname, - self.imported_salt_modules[node_left.value.expr.name]) + args=( + node_left.value.attrname, + self.imported_salt_modules[node_left.value.expr.name], + ), ) return @@ -326,32 +419,27 @@ def visit_assign(self, node, *args): if node_left.attrname in self.salt_dunders: # We're changing salt dunders self.add_message( - 'unmocked-patch-dunder', + "unmocked-patch-dunder", node=node, - args=(node_left.attrname, - self.imported_salt_modules[node_left.expr.name]) + args=(node_left.attrname, self.imported_salt_modules[node_left.expr.name]), ) return # Changing random attributes self.add_message( - 'unmocked-patch', + "unmocked-patch", node=node, - args=(node_left.attrname, - self.imported_salt_modules[node_left.expr.name]) + args=(node_left.attrname, self.imported_salt_modules[node_left.expr.name]), ) RESOURCE_LEAKAGE_MSGS = { - 'W8470': ('Resource leakage detected. %s ', - 'resource-leakage', - 'Resource leakage detected.'), + "W8470": ("Resource leakage detected. %s ", "resource-leakage", "Resource leakage detected."), } class ResourceLeakageChecker(BaseChecker): - - name = 'resource-leakage' + name = "resource-leakage" msgs = RESOURCE_LEAKAGE_MSGS priority = -2 @@ -369,37 +457,49 @@ def leave_with(self, node): def visit_call(self, node): if isinstance(node.func, astroid.Attribute): - if node.func.attrname == 'fopen' and self.inside_with_ctx is False: - msg = ('Please call \'salt.utils.files.fopen\' using the \'with\' context ' - 'manager, otherwise the file handle won\'t be closed and ' - 'resource leakage will occur.') - self.add_message('resource-leakage', node=node, args=(msg,)) - elif isinstance(node.func, astroid.Name): - if utils.is_builtin(node.func.name) and node.func.name == 'open': - if self.inside_with_ctx: - msg = ('Please use \'with salt.utils.files.fopen()\' instead of ' - '\'with open()\'. It assures salt does not leak ' - 'file handles.') - else: - msg = ('Please use \'salt.utils.files.fopen()\' instead of \'open()\' ' - 'using the \'with\' context manager, otherwise the file ' - 'handle won\'t be closed and resource leakage will occur.') - self.add_message('resource-leakage', node=node, args=(msg,)) + if node.func.attrname == "fopen" and self.inside_with_ctx is False: + msg = ( + "Please call 'salt.utils.files.fopen' using the 'with' context " + "manager, otherwise the file handle won't be closed and " + "resource leakage will occur." + ) + self.add_message("resource-leakage", node=node, args=(msg,)) + elif ( + isinstance(node.func, astroid.Name) + and utils.is_builtin(node.func.name) + and node.func.name == "open" + ): + if self.inside_with_ctx: + msg = ( + "Please use 'with salt.utils.files.fopen()' instead of " + "'with open()'. It assures salt does not leak " + "file handles." + ) + else: + msg = ( + "Please use 'salt.utils.files.fopen()' instead of 'open()' " + "using the 'with' context manager, otherwise the file " + "handle won't be closed and resource leakage will occur." + ) + self.add_message("resource-leakage", node=node, args=(msg,)) MOVED_TEST_CASE_CLASSES_MSGS = { - 'E9490': ('Moved test case base class detected. %s', - 'moved-test-case-class', - 'Moved test case base class detected.'), - 'E9491': ('Moved test case mixin class detected. %s', - 'moved-test-case-mixin', - 'Moved test case mixin class detected.'), + "E9490": ( + "Moved test case base class detected. %s", + "moved-test-case-class", + "Moved test case base class detected.", + ), + "E9491": ( + "Moved test case mixin class detected. %s", + "moved-test-case-mixin", + "Moved test case mixin class detected.", + ), } class MovedTestCaseClassChecker(BaseChecker): - - name = 'moved-test-case-class' + name = "moved-test-case-class" msgs = MOVED_TEST_CASE_CLASSES_MSGS priority = -2 @@ -411,7 +511,7 @@ def close(self): def visit_module(self, node): module_filename = node.root().file - if not fnmatch.fnmatch(os.path.basename(module_filename), 'test_*.py*'): + if not fnmatch.fnmatch(os.path.basename(module_filename), "test_*.py*"): return self.process_module = True @@ -421,9 +521,9 @@ def leave_module(self, node): self.process_module = False def visit_importfrom(self, node): - '''triggered when a from statement is seen''' + """Triggered when a from statement is seen.""" if self.process_module: - if not node.modname.startswith('tests.integration'): + if not node.modname.startswith("tests.integration"): return # Store salt imported modules for module, import_as in node.names: @@ -434,60 +534,66 @@ def visit_importfrom(self, node): def visit_classdef(self, node): for base in node.bases: - if not hasattr(base, 'attrname'): + if not hasattr(base, "attrname"): continue - if base.attrname in ('TestCase',): - msg = 'Please use \'from tests.support.unit import {0}\''.format(base.attrname) - self.add_message('moved-test-case-class', node=node, args=(msg,)) - if base.attrname in ('ModuleCase', 'SyndicCase', 'ShellCase', 'SSHCase'): - msg = 'Please use \'from tests.support.case import {0}\''.format(base.attrname) - self.add_message('moved-test-case-class', node=node, args=(msg,)) - if base.attrname in ('AdaptedConfigurationTestCaseMixin', 'ShellCaseCommonTestsMixin', - 'SaltMinionEventAssertsMixin'): - msg = 'Please use \'from tests.support.mixins import {0}\''.format(base.attrname) - self.add_message('moved-test-case-mixin', node=node, args=(msg,)) + if base.attrname in ("TestCase",): + msg = f"Please use 'from tests.support.unit import {base.attrname}'" + self.add_message("moved-test-case-class", node=node, args=(msg,)) + if base.attrname in ("ModuleCase", "SyndicCase", "ShellCase", "SSHCase"): + msg = f"Please use 'from tests.support.case import {base.attrname}'" + self.add_message("moved-test-case-class", node=node, args=(msg,)) + if base.attrname in ( + "AdaptedConfigurationTestCaseMixin", + "ShellCaseCommonTestsMixin", + "SaltMinionEventAssertsMixin", + ): + msg = f"Please use 'from tests.support.mixins import {base.attrname}'" + self.add_message("moved-test-case-mixin", node=node, args=(msg,)) def _check_moved_imports(self, node, module, import_as=None): - names = [] for name, name_as in node.names: - if name not in ('ModuleCase', 'SyndicCase', 'ShellCase', 'SSHCase'): + if name not in ("ModuleCase", "SyndicCase", "ShellCase", "SSHCase"): continue if name_as: - msg = 'Please use \'from tests.support.case import {0} as {1}\''.format(name, name_as) + msg = f"Please use 'from tests.support.case import {name} as {name_as}'" else: - msg = 'Please use \'from tests.support.case import {0}\''.format(name) - self.add_message('moved-test-case-class', node=node, args=(msg,)) + msg = f"Please use 'from tests.support.case import {name}'" + self.add_message("moved-test-case-class", node=node, args=(msg,)) BLACKLISTED_FUNCTIONS_MSGS = { - 'E9601': ('Use of blacklisted function %s (use %s instead)', - 'blacklisted-function', - 'Used a function marked as blacklisted'), - } + "E9601": ( + "Use of blacklisted function %s (use %s instead)", + "blacklisted-function", + "Used a function marked as blacklisted", + ), +} class BlacklistedFunctionsChecker(BaseChecker): - - name = 'blacklisted-functions' + name = "blacklisted-functions" msgs = BLACKLISTED_FUNCTIONS_MSGS priority = -2 max_depth = 20 options = ( - ('blacklisted-functions', - {'default': '', 'type': 'string', - 'metavar': 'bad1=good1,bad2=good2', - 'help': 'List of blacklisted functions and their recommended ' - 'replacements'}), + ( + "blacklisted-functions", + { + "default": "", + "type": "string", + "metavar": "bad1=good1,bad2=good2", + "help": "List of blacklisted functions and their recommended replacements", + }, + ), ) def open(self): self.blacklisted_functions = {} - blacklist = [ - x.strip() for x in self.linter.config.blacklisted_functions.split(',')] + blacklist = [x.strip() for x in self.linter.config.blacklisted_functions.split(",")] for item in blacklist: try: - key, val = [x.strip() for x in item.split('=')] + key, val = (x.strip() for x in item.split("=")) except ValueError: pass else: @@ -496,48 +602,43 @@ def open(self): def _get_full_name(self, node): try: func = utils.safe_infer(node.func) - if func.name.__str__() == 'Uninferable': - return + if func.name.__str__() == "Uninferable": + return None except Exception: func = None if func is None: - return + return None ret = [] depth = 0 while func is not None: depth += 1 if depth > self.max_depth: # Prevent endless loop - return + return None try: ret.append(func.name) except AttributeError: - return + return None func = func.parent # ret will contain the levels of the function from last to first (e.g. # ['walk', 'os']. Reverse it and join with dots to get the correct # full name for the function. - return '.'.join(ret[::-1]) + return ".".join(ret[::-1]) def visit_call(self, node): if self.blacklisted_functions: full_name = self._get_full_name(node) if full_name is not None: - try: + with contextlib.suppress(KeyError): self.add_message( - 'blacklisted-function', + "blacklisted-function", node=node, - args=(full_name, self.blacklisted_functions[full_name]) + args=(full_name, self.blacklisted_functions[full_name]), ) - except KeyError: - # Not blacklisted - pass def register(linter): - ''' - Required method to auto register this checker - ''' + """Required method to auto register this checker.""" linter.register_checker(ResourceLeakageChecker(linter)) linter.register_checker(BlacklistedImportsChecker(linter)) linter.register_checker(MovedTestCaseClassChecker(linter)) diff --git a/saltpylint/checkers.py b/saltpylint/checkers.py deleted file mode 100644 index 45858a9..0000000 --- a/saltpylint/checkers.py +++ /dev/null @@ -1,23 +0,0 @@ -# -*- coding: utf-8 -*- -''' - saltpylint.checkers - ~~~~~~~~~~~~~~~~~~~~ - - Works around older astroid versions -''' -# Import python libs -from __future__ import absolute_import - -# Import pylint libs -import astroid -from pylint.checkers import BaseChecker as _BaseChecker -# Imported to avoid needing a separate import from pylint.checkers -from pylint.checkers import utils - - -class BaseChecker(_BaseChecker): - def __init__(self, *args, **kwargs): - super(BaseChecker, self).__init__(*args, **kwargs) - if hasattr(self, 'visit_call') and not hasattr(astroid, 'Call'): - setattr(self, 'visit_callfunc', self.visit_call) - diff --git a/saltpylint/dunder_del.py b/saltpylint/dunder_del.py index a3d48ce..a3466fa 100644 --- a/saltpylint/dunder_del.py +++ b/saltpylint/dunder_del.py @@ -1,57 +1,34 @@ -# -*- coding: utf-8 -*- -# Import Python libs -from __future__ import absolute_import +from typing import ClassVar -# Import PyLint libs -try: - # >= pylint 1.0 - import astroid -except ImportError: # pylint < 1.0 - from logilab import astng as astroid # pylint: disable=no-name-in-module -from saltpylint.checkers import BaseChecker, utils +from pylint.checkers import BaseChecker -try: - # >= pylint 1.0 - from pylint.interfaces import IAstroidChecker -except ImportError: # < pylint 1.0 - from pylint.interfaces import IASTNGChecker as IAstroidChecker # pylint: disable=no-name-in-module - -try: - from astroid.exceptions import NameInferenceError -except ImportError: - class NameInferenceError(Exception): - pass - -WARNING_CODE = 'W1701' +WARNING_CODE = "W1701" class DunderDelChecker(BaseChecker): - ''' - info: This class is used by pylint that checks + """info: This class is used by pylint that checks if "__del__" is not used - if "__del__" is used then a warning will be raised - ''' - __implements__ = IAstroidChecker + if "__del__" is used then a warning will be raised. + """ - name = 'dunder-del' + name = "dunder-del" priority = -1 - msgs = { + msgs: ClassVar = { WARNING_CODE: ( '"__del__" is not allowed!', - 'no-dunder-del', - '"__del__" is not allowed! A "with" block could be a good solution' + "no-dunder-del", + '"__del__" is not allowed! A "with" block could be a good solution', ), } def visit_functiondef(self, node): - ''' - :param node: info about a function or method + """:param node: info about a function or method :return: None - ''' + """ if node.name == "__del__": self.add_message(WARNING_CODE, node=node) def register(linter): - '''required method to auto register this checker ''' + """Required method to auto register this checker.""" linter.register_checker(DunderDelChecker(linter)) diff --git a/saltpylint/fileperms.py b/saltpylint/fileperms.py index 7ad4c57..a6f6b3c 100644 --- a/saltpylint/fileperms.py +++ b/saltpylint/fileperms.py @@ -1,80 +1,79 @@ -# -*- coding: utf-8 -*- -''' - :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)` - :copyright: © 2015 by the SaltStack Team, see AUTHORS for more details. - :license: Apache 2.0, see LICENSE for more details. +""" +PyLint File Permissions Check Plugin +==================================== +PyLint plugin which checks for specific file permissions +""" - ==================================== - PyLint File Permissions Check Plugin - ==================================== - - PyLint plugin which checks for specific file permissions -''' - -# Import Python libs -from __future__ import absolute_import -import os -import sys import glob +import os import stat +import sys +from typing import ClassVar -# Import PyLint libs -from saltpylint.checkers import BaseChecker +from pylint.checkers import BaseChecker class FilePermsChecker(BaseChecker): - ''' - Check for files with undesirable permissions - ''' + """Check for files with undesirable permissions.""" - name = 'fileperms' - msgs = {'E0599': ('Module file has the wrong file permissions(expected %s): %s', - 'file-perms', - ('Wrong file permissions')), - } + name = "fileperms" + msgs: ClassVar = { + "E0599": ( + "Module file has the wrong file permissions(expected %s): %s", + "file-perms", + ("Wrong file permissions"), + ), + } priority = -1 - options = (('fileperms-default', - {'default': '0644', 'type': 'string', 'metavar': 'ZERO_PADDED_PERM', - 'help': 'Desired file permissons. Default: 0644'} - ), - ('fileperms-ignore-paths', - {'default': (), 'type': 'csv', 'metavar': '', - 'help': 'File paths to ignore file permission. Glob patterns allowed.'} - ) - ) + options = ( + ( + "fileperms-default", + { + "default": "0644", + "type": "string", + "metavar": "ZERO_PADDED_PERM", + "help": "Desired file permissons. Default: 0644", + }, + ), + ( + "fileperms-ignore-paths", + { + "default": (), + "type": "csv", + "metavar": "", + "help": "File paths to ignore file permission. Glob patterns allowed.", + }, + ), + ) def process_module(self, node): - ''' - process a module - ''' - + """Process a module.""" for listing in self.config.fileperms_ignore_paths: - if node.file.split('{0}/'.format(os.getcwd()))[-1] in glob.glob(listing): + if node.file.split(f"{os.getcwd()}/")[-1] in glob.glob(listing): # File is ignored, no checking should be done return desired_perm = self.config.fileperms_default - if '-' in desired_perm: - desired_perm = desired_perm.split('-') + if "-" in desired_perm: + desired_perm = desired_perm.split("-") else: desired_perm = [desired_perm] - if len(desired_perm) > 2: - raise RuntimeError('Permission ranges should be like XXXX-YYYY') + if len(desired_perm) > 2: # noqa: PLR2004 + msg = "Permission ranges should be like XXXX-YYYY" + raise RuntimeError(msg) - for idx, perm in enumerate(desired_perm): - desired_perm[idx] = desired_perm[idx].strip('"').strip('\'').lstrip('0').zfill(4) - if desired_perm[idx][0] != '0': + for idx, _perm in enumerate(desired_perm): + desired_perm[idx] = desired_perm[idx].strip('"').strip("'").lstrip("0").zfill(4) + if desired_perm[idx][0] != "0": # Always include a leading zero - desired_perm[idx] = '0{0}'.format(desired_perm[idx]) - if sys.version_info > (3,): - # The octal representation in python 3 has changed to 0o644 instead of 0644 - if desired_perm[idx][1] != 'o': - desired_perm[idx] = '0o' + desired_perm[idx][1:] - if sys.platform.startswith('win'): + desired_perm[idx] = f"0{desired_perm[idx]}" + if desired_perm[idx][1] != "o": + desired_perm[idx] = "0o" + desired_perm[idx][1:] + if sys.platform.startswith("win"): # Windows does not distinguish between user/group/other. # They must all be the same. Also, Windows will automatically # set the execution bit on files with a known extension @@ -86,36 +85,31 @@ def process_module(self, node): desired_perm[idx] = desired_perm[idx][:-3] + (str(user_perm_noexec) * 3) module_perms = oct(stat.S_IMODE(os.stat(node.file).st_mode)) - if sys.version_info < (3,): - module_perms = str(module_perms) if len(desired_perm) == 1: if module_perms != desired_perm[0]: - if sys.platform.startswith('win'): + if sys.platform.startswith("win"): # Check the variant with execution bit set due to the # unreliability of checking the execution bit on Windows. user_perm_noexec = int(desired_perm[0][-3]) desired_perm_exec = desired_perm[0][:-3] + (str(user_perm_noexec + 1) * 3) if module_perms == desired_perm_exec: return - self.add_message('E0599', line=1, args=(desired_perm[0], module_perms)) - else: - if module_perms < desired_perm[0] or module_perms > desired_perm[1]: - if sys.platform.startswith('win'): - # Check the variant with execution bit set due to the - # unreliability of checking the execution bit on Windows. - user_perm_noexec0 = int(desired_perm[0][-3]) - desired_perm_exec0 = desired_perm[0][:-3] + (str(user_perm_noexec0 + 1) * 3) - user_perm_noexec1 = int(desired_perm[1][-3]) - desired_perm_exec1 = desired_perm[1][:-3] + (str(user_perm_noexec1 + 1) * 3) - if desired_perm_exec0 <= module_perms <= desired_perm_exec1: - return - desired_perm = '>= {0} OR <= {1}'.format(*desired_perm) - self.add_message('E0599', line=1, args=(desired_perm, module_perms)) + self.add_message("E0599", line=1, args=(desired_perm[0], module_perms)) + elif module_perms < desired_perm[0] or module_perms > desired_perm[1]: + if sys.platform.startswith("win"): + # Check the variant with execution bit set due to the + # unreliability of checking the execution bit on Windows. + user_perm_noexec0 = int(desired_perm[0][-3]) + desired_perm_exec0 = desired_perm[0][:-3] + (str(user_perm_noexec0 + 1) * 3) + user_perm_noexec1 = int(desired_perm[1][-3]) + desired_perm_exec1 = desired_perm[1][:-3] + (str(user_perm_noexec1 + 1) * 3) + if desired_perm_exec0 <= module_perms <= desired_perm_exec1: + return + desired_perm = ">= {} OR <= {}".format(*desired_perm) + self.add_message("E0599", line=1, args=(desired_perm, module_perms)) def register(linter): - ''' - required method to auto register this checker - ''' + """Required method to auto register this checker.""" linter.register_checker(FilePermsChecker(linter)) diff --git a/saltpylint/smartup.py b/saltpylint/smartup.py index fc152f5..d832a29 100644 --- a/saltpylint/smartup.py +++ b/saltpylint/smartup.py @@ -1,45 +1,33 @@ -# -*- coding: utf-8 -*- -''' - :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)` - :copyright: © 2013-2018 by the SaltStack Team, see AUTHORS for more details. - :license: Apache 2.0, see LICENSE for more details. +""" +Pylint Smartup Transformers +=========================== +This plugin will register some transform functions which will allow PyLint to better +understand some classed used in Salt which trigger, `no-member` and `maybe-no-member` +A bridge between the `pep8`_ library and PyLint - =========================== - Pylint Smartup Transformers - =========================== +""" - This plugin will register some transform functions which will allow PyLint to better - understand some classed used in Salt which trigger, `no-member` and `maybe-no-member` - A bridge between the `pep8`_ library and PyLint - - ''' - -# Import Python libs -from __future__ import absolute_import - -# Import PyLint libs -from astroid import nodes, MANAGER +from astroid import MANAGER +from astroid import nodes def rootlogger_transform(obj): - if obj.name != 'RootLogger': + if obj.name != "RootLogger": return def _inject_method(cls, msg, *args, **kwargs): pass - if not hasattr(obj, 'trace'): - setattr(obj, 'trace', _inject_method) + if not hasattr(obj, "trace"): + obj.trace = _inject_method - if not hasattr(obj, 'garbage'): - setattr(obj, 'garbage', _inject_method) + if not hasattr(obj, "garbage"): + obj.garbage = _inject_method def register(linter): - ''' - Register the transformation functions. - ''' + """Register the transformation functions.""" try: MANAGER.register_transform(nodes.Class, rootlogger_transform) except AttributeError: diff --git a/saltpylint/thirdparty.py b/saltpylint/thirdparty.py index 0cb6a26..7baf11a 100644 --- a/saltpylint/thirdparty.py +++ b/saltpylint/thirdparty.py @@ -1,81 +1,95 @@ -# -*- coding: utf-8 -*- -''' - :codeauthor: :email:`Pedro Algarvio (pedro@algarvio.me)` - :copyright: © 2017 by the SaltStack Team, see AUTHORS for more details. - :license: Apache 2.0, see LICENSE for more details. +""" +saltpylint.thirdparty +~~~~~~~~~~~~~~~~~~~~~ +Checks all imports against a list of known and allowed 3rd-party modules +and raises a lint error if an import not in that known 3rd-party modules list +is not gated. +""" - saltpylint.thirdparty - ~~~~~~~~~~~~~~~~~~~~~ - - Checks all imports against a list of known and allowed 3rd-party modules - and raises a lint error if an import not in that known 3rd-party modules list - is not gated. -''' - -# Import python libs -from __future__ import absolute_import import os +import pkgutil +from typing import ClassVar -# Import pylint libs import astroid import astroid.exceptions -import pkgutil -from astroid.modutils import is_relative, is_standard_module -from saltpylint.checkers import BaseChecker, utils +from astroid.modutils import is_relative +from astroid.modutils import is_standard_module +from pylint.checkers import BaseChecker MSGS = { - 'W8410': ('3rd-party module import is not gated in a try/except: %r', - '3rd-party-module-not-gated', - '3rd-party module imported without being gated in a try/except.'), - 'C8410': ('3rd-party local module import is not gated in a try/except: %r. ' - 'Consider importing at the module global scope level and gate it ' - 'in a try/except.', - '3rd-party-local-module-not-gated', - '3rd-party module locally imported without being gated. Consider importing ' - 'at the module global scope level and gate it in a try/except'), - } + "W8410": ( + "3rd-party module import is not gated in a try/except: %r", + "3rd-party-module-not-gated", + "3rd-party module imported without being gated in a try/except.", + ), + "C8410": ( + "3rd-party local module import is not gated in a try/except: %r. " + "Consider importing at the module global scope level and gate it " + "in a try/except.", + "3rd-party-local-module-not-gated", + "3rd-party module locally imported without being gated. Consider importing " + "at the module global scope level and gate it in a try/except", + ), +} def get_import_package(modname): - ''' - Return the import package. + """Return the import package. Given modname is 'salt.utils', returns 'salt' - ''' - return modname.split('.')[0] + """ + return modname.split(".")[0] class ThirdPartyImportsChecker(BaseChecker): - - name = '3rd-party-imports' + name = "3rd-party-imports" msgs = MSGS priority = -2 options = ( - ('allowed-3rd-party-modules', { - 'default': (), - 'type': 'csv', - 'metavar': '<3rd-party-modules>', - 'help': 'Known 3rd-party modules which don\' require being gated, separated by a comma'}), + ( + "allowed-3rd-party-modules", + { + "default": (), + "type": "csv", + "metavar": "<3rd-party-modules>", + "help": "Known 3rd-party modules which don' require being gated, separated by a comma", + }, + ), ) - known_py2_modules = ['__builtin__', 'exceptions'] - known_py3_modules = ['builtins'] - - unix_modules = ('posix', 'pwd', 'spwd', 'grp', 'crypt', 'termios', 'tty', 'pty', 'fcntl', 'pipes', 'resource', 'nis', 'syslog', 'posixpath') - win_modules = ('msilib', 'msvcrt', 'winreg', 'winsound', 'ntpath') + known_py2_modules: ClassVar = ["__builtin__", "exceptions"] + known_py3_modules: ClassVar = ["builtins"] + + unix_modules = ( + "posix", + "pwd", + "spwd", + "grp", + "crypt", + "termios", + "tty", + "pty", + "fcntl", + "pipes", + "resource", + "nis", + "syslog", + "posixpath", + ) + win_modules = ("msilib", "msvcrt", "winreg", "winsound", "ntpath") - all_modules = {m[1]: m[0] for m in pkgutil.iter_modules()} - std_modules_path = all_modules["os"] - std_modules = [] + all_modules: ClassVar = {m[1]: m[0] for m in pkgutil.iter_modules()} + std_modules_path: ClassVar = all_modules["os"] + std_modules: ClassVar = [] for mod, path in all_modules.items(): if path == std_modules_path and mod not in unix_modules + win_modules: std_modules.append(mod) known_std_modules = known_py2_modules + known_py3_modules + std_modules - def __init__(self, linter=None): + def __init__(self, linter=None) -> None: BaseChecker.__init__(self, linter) self._inside_try_except = False self._inside_funcdef = False @@ -84,9 +98,11 @@ def __init__(self, linter=None): self.allowed_3rd_party_modules = [] def open(self): - super(ThirdPartyImportsChecker, self).open() + super().open() self.cwd = os.getcwd() - self.allowed_3rd_party_modules = set(self.linter.config.allowed_3rd_party_modules) # pylint: disable=no-member + self.allowed_3rd_party_modules = set( + self.linter.config.allowed_3rd_party_modules, + ) # pylint: disable=no-member # pylint: disable=unused-argument def visit_if(self, node): @@ -106,6 +122,7 @@ def visit_functiondef(self, node): def leave_functiondef(self, node): self._inside_funcdef = False + # pylint: enable=unused-argument def visit_import(self, node): @@ -126,7 +143,7 @@ def _check_third_party_import(self, node, modname): # Is the import relative to the curent module being checked return - base_modname = modname.split('.', 1)[0] + base_modname = modname.split(".", 1)[0] import_modname = modname while True: try: @@ -142,30 +159,29 @@ def _check_third_party_import(self, node, modname): except Exception: # pylint: disable=broad-except # This is, for example, from salt.ext.six.moves import Y # Because `moves` is a dynamic/runtime module - import_modname = import_modname.rsplit('.', 1)[0] + import_modname = import_modname.rsplit(".", 1)[0] if import_modname == base_modname or not import_modname: break try: - if not is_standard_module(modname): - if self._inside_try_except is False: - if get_import_package(modname) in self.allowed_3rd_party_modules: - return - if self._inside_if or self._inside_funcdef: - message_id = '3rd-party-local-module-not-gated' - else: - message_id = '3rd-party-module-not-gated' - self.add_message(message_id, node=node, args=modname) + if not is_standard_module(modname) and self._inside_try_except is False: + if get_import_package(modname) in self.allowed_3rd_party_modules: + return + if self._inside_if or self._inside_funcdef: + message_id = "3rd-party-local-module-not-gated" + else: + message_id = "3rd-party-module-not-gated" + self.add_message(message_id, node=node, args=modname) except astroid.exceptions.AstroidBuildingException: # Failed to import if self._inside_try_except is False: if get_import_package(modname) in self.allowed_3rd_party_modules: return if self._inside_if or self._inside_funcdef: - message_id = '3rd-party-local-module-not-gated' + message_id = "3rd-party-local-module-not-gated" else: - message_id = '3rd-party-module-not-gated' + message_id = "3rd-party-module-not-gated" self.add_message(message_id, node=node, args=modname) except astroid.exceptions.InferenceError: # Failed to import @@ -173,9 +189,9 @@ def _check_third_party_import(self, node, modname): if get_import_package(modname) in self.allowed_3rd_party_modules: return if self._inside_if or self._inside_funcdef: - message_id = '3rd-party-local-module-not-gated' + message_id = "3rd-party-local-module-not-gated" else: - message_id = '3rd-party-module-not-gated' + message_id = "3rd-party-module-not-gated" self.add_message(message_id, node=node, args=modname) except ImportError: # Definitly not a standard library import @@ -183,12 +199,12 @@ def _check_third_party_import(self, node, modname): if get_import_package(modname) in self.allowed_3rd_party_modules: return if self._inside_if or self._inside_funcdef: - message_id = '3rd-party-local-module-not-gated' + message_id = "3rd-party-local-module-not-gated" else: - message_id = '3rd-party-module-not-gated' + message_id = "3rd-party-module-not-gated" self.add_message(message_id, node=node, args=modname) def register(linter): - '''required method to auto register this checker ''' + """Required method to auto register this checker.""" linter.register_checker(ThirdPartyImportsChecker(linter)) diff --git a/saltpylint/virt.py b/saltpylint/virt.py index ce578e5..7c22390 100644 --- a/saltpylint/virt.py +++ b/saltpylint/virt.py @@ -1,42 +1,40 @@ -from __future__ import absolute_import -import astroid +from typing import ClassVar -from pylint.interfaces import IAstroidChecker +import astroid from pylint.checkers import BaseChecker -class VirtChecker(BaseChecker): - ''' - checks for compliance inside __virtual__ - ''' - __implements__ = IAstroidChecker +VIRT_LOG = "log-in-virtual" - name = 'virt-checker' - VIRT_LOG = 'log-in-virtual' +class VirtChecker(BaseChecker): + """checks for compliance inside __virtual__.""" + + name = "virt-checker" - msgs = { - 'E1401': ('Log statement detected inside __virtual__ function. Remove it.', - VIRT_LOG, - 'Loader processes __virtual__ so logging not in scope'), + msgs: ClassVar = { + "E1401": ( + "Log statement detected inside __virtual__ function. Remove it.", + VIRT_LOG, + "Loader processes __virtual__ so logging not in scope", + ), } options = () priority = -1 def visit_functiondef(self, node): - ''' - Verifies no logger statements inside __virtual__ - ''' - if (not isinstance(node, astroid.FunctionDef) or - node.is_method() - or node.type != 'function' + """Verifies no logger statements inside __virtual__.""" + if ( + not isinstance(node, astroid.FunctionDef) + or node.is_method() + or node.type != "function" or not node.body - ): + ): # only process functions return try: - if not node.name == '__virtual__': + if node.name != "__virtual__": # only need to process the __virtual__ function return except AttributeError: @@ -45,27 +43,26 @@ def visit_functiondef(self, node): # walk contents of __virtual__ function for child in node.get_children(): for functions in child.get_children(): - if isinstance(functions, astroid.Call): - if isinstance(functions.func, astroid.Attribute): - try: - # Inspect each statement for an instance of 'logging' - for inferred in functions.func.expr.infer(): - try: - instance_type = inferred.pytype().split('.')[0] - except TypeError: - continue - if instance_type == 'logging': - self.add_message( - self.VIRT_LOG, node=functions - ) - # Found logger, don't need to keep processing this line - break - except AttributeError: - # Not a log function - return + if isinstance(functions, astroid.Call) and isinstance( + functions.func, + astroid.Attribute, + ): + try: + # Inspect each statement for an instance of 'logging' + for inferred in functions.func.expr.infer(): + try: + instance_type = inferred.pytype().split(".")[0] + except TypeError: + continue + if instance_type == "logging": + self.add_message(VIRT_LOG, node=functions) + # Found logger, don't need to keep processing this line + break + except AttributeError: + # Not a log function + return + def register(linter): - ''' - required method to auto register this checker - ''' + """Required method to auto register this checker.""" linter.register_checker(VirtChecker(linter)) diff --git a/setup.cfg b/setup.cfg index b097cf5..5825b27 100644 --- a/setup.cfg +++ b/setup.cfg @@ -33,7 +33,7 @@ platforms = unix, linux, osx, cygwin, win32 zip_safe = False include_package_data = True packages = find: -python_requires = >= 3.7 +python_requires = >= 3.8 setup_requires = setuptools>=50.3.2 setuptools_scm[toml]>=3.4