From 426ed1fe9e056e03abf728d98d7fb4c578afe5eb Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Fri, 22 Nov 2024 14:17:34 +0000 Subject: [PATCH] Prepare for pyright hook enablement (#4410) --- .pre-commit-config.yaml | 27 ++++++++++++++++++++++++- .taplo.toml | 4 ++++ .vscode/settings.json | 3 ++- conftest.py | 6 ++---- pyproject.toml | 15 +++++++------- src/ansiblelint/_internal/rules.py | 6 ++++-- src/ansiblelint/rules/args.py | 2 +- src/ansiblelint/rules/avoid_implicit.py | 2 +- src/ansiblelint/rules/fqcn.py | 4 ++++ src/ansiblelint/rules/key_order.py | 1 - src/ansiblelint/rules/role_name.py | 4 ++++ src/ansiblelint/rules/run_once.py | 5 +++-- src/ansiblelint/skip_utils.py | 25 ++++++++++++++--------- src/ansiblelint/transformer.py | 10 ++++++--- src/ansiblelint/utils.py | 2 +- src/ansiblelint/yaml_utils.py | 6 +++--- tox.ini | 1 - 17 files changed, 85 insertions(+), 38 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index f0d37077d2..bc0c6859a5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -177,6 +177,31 @@ repos: test/local-content/.*| plugins/.* )$ + # - repo: https://github.com/RobertCraigie/pyright-python + # rev: v1.1.389 + # hooks: + # - id: pyright + # additional_dependencies: + # - nodejs-wheel-binaries + # - ansible-compat>=24.8.0 + # - black>=22.10.0 + # - cryptography>=39.0.1 + # - filelock>=3.12.2 + # - importlib_metadata + # - jinja2 + # - license-expression >= 30.3.0 + # - pip>=22.3.1 + # - pytest-mock + # - pytest>=7.2.2 + # - rich>=13.2.0 + # - ruamel-yaml-clib>=0.2.8 + # - ruamel-yaml>=0.18.6 + # - subprocess-tee + # - types-PyYAML + # - types-jsonschema>=4.20.0.0 + # - types-setuptools + # - wcmatch + # - yamllint - repo: https://github.com/pycqa/pylint rev: v3.3.1 hooks: @@ -186,7 +211,7 @@ repos: additional_dependencies: - ansible-compat>=24.8.0 - ansible-core>=2.14.0 - - black>=22.10.0 + - black>=24.10.0 - docutils - filelock>=3.12.2 - importlib_metadata diff --git a/.taplo.toml b/.taplo.toml index 4cfd7f7d68..0d2e98636a 100644 --- a/.taplo.toml +++ b/.taplo.toml @@ -1,2 +1,6 @@ [formatting] +# compatibility with toml-sort-fix pre-commit hook array_trailing_comma = false +compact_arrays = true +compact_inline_tables = true +inline_table_expand = false diff --git a/.vscode/settings.json b/.vscode/settings.json index bbfb53d30f..869aaa787e 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -42,5 +42,6 @@ }, "editor.defaultFormatter": "ms-python.black-formatter", "editor.formatOnSave": true - } + }, + "evenBetterToml.formatter.arrayTrailingComma": true } diff --git a/conftest.py b/conftest.py index 8762edf2c7..3770a7e0cd 100644 --- a/conftest.py +++ b/conftest.py @@ -1,11 +1,11 @@ """PyTest Fixtures.""" -import importlib import os import platform import subprocess import sys import warnings +from importlib.util import find_spec from pathlib import Path import pytest @@ -16,9 +16,7 @@ # checking if user is running pytest without installing test dependencies: missing = [ - module - for module in ["ansible", "black", "mypy", "pylint"] - if not importlib.util.find_spec(module) + module for module in ["ansible", "black", "mypy", "pylint"] if not find_spec(module) ] if missing: pytest.exit( diff --git a/pyproject.toml b/pyproject.toml index b93e2c8f75..03a31ee8a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -113,9 +113,6 @@ module = [ [tool.pylint.IMPORTS] preferred-modules = ["py:pathlib", "unittest:pytest"] -[tool.pylint.MAIN] -extension-pkg-allow-list = ["black.parsing"] - [tool.pylint.MASTER] bad-names = [ # spell-checker:ignore linenumber @@ -133,6 +130,7 @@ disable = [ "line-too-long", # covered by black "protected-access", # covered by ruff SLF001 "too-many-branches", # covered by ruff C901 + "wrong-import-order", # covered by ruff # TODO(ssbarnea): remove temporary skips adding during initial adoption: "duplicate-code", # unable to disable it inside tests @@ -155,17 +153,20 @@ output-format = "colorized" score = "n" [tool.pyright] +exclude = ["venv", ".cache"] include = ["src"] +mode = "standard" # https://github.com/microsoft/pyright/blob/main/docs/configuration.md#sample-pyprojecttoml-file -pythonVersion = "3.10" -# https://github.com/microsoft/pyright/issues/777 -"stubPath" = "" +# pythonVersion = "3.10" +# reportMissingImports = false +# https://github.com/microsoft/pyright/issues/9494 +reportPossiblyUnboundVariable = false # spell-checker:ignore filterwarnings norecursedirs optionflags [tool.pytest.ini_options] # do not add options here as this will likely break either console runs or IDE # integration like vscode or pycharm -addopts = "-p no:pytest_cov" +addopts = "-p no:pytest_cov --failed-first" # https://code.visualstudio.com/docs/python/testing # coverage is re-enabled in `tox.ini`. That approach is safer than # `--no-cov` which prevents activation from tox.ini and which also fails diff --git a/src/ansiblelint/_internal/rules.py b/src/ansiblelint/_internal/rules.py index 38cb835199..8506a8173e 100644 --- a/src/ansiblelint/_internal/rules.py +++ b/src/ansiblelint/_internal/rules.py @@ -56,6 +56,8 @@ class BaseRule: _help: str | None = None # Added when a rule is registered into a collection, gives access to options _collection: RulesCollection | None = None + # Allow rules to provide a custom short description instead of using __doc__ + _shortdesc: str = "" @property def help(self) -> str: @@ -83,7 +85,7 @@ def url(self) -> str: @property def shortdesc(self) -> str: """Return the short description of the rule, basically the docstring.""" - return self.__doc__ or "" + return self._shortdesc or self.__doc__ or "" def getmatches(self, file: Lintable) -> list[MatchError]: """Return all matches while ignoring exceptions.""" @@ -190,7 +192,7 @@ class RuntimeErrorRule(BaseRule): """Unexpected internal error.""" id = "internal-error" - shortdesc = "Unexpected internal error" + _shortdesc = "Unexpected internal error" severity = "VERY_HIGH" tags = ["core"] version_added = "v5.0.0" diff --git a/src/ansiblelint/rules/args.py b/src/ansiblelint/rules/args.py index 0cfc5345d0..29d4dd0ebf 100644 --- a/src/ansiblelint/rules/args.py +++ b/src/ansiblelint/rules/args.py @@ -73,7 +73,7 @@ class ValidationPassedError(Exception): class CustomAnsibleModule(basic.AnsibleModule): # type: ignore[misc] """Mock AnsibleModule class.""" - def __init__(self, *args: str, **kwargs: Any) -> None: + def __init__(self, *args: Any, **kwargs: Any) -> None: """Initialize AnsibleModule mock.""" kwargs["no_log"] = True super().__init__(*args, **kwargs) diff --git a/src/ansiblelint/rules/avoid_implicit.py b/src/ansiblelint/rules/avoid_implicit.py index 7f3f72ecca..6aed61a158 100644 --- a/src/ansiblelint/rules/avoid_implicit.py +++ b/src/ansiblelint/rules/avoid_implicit.py @@ -17,7 +17,7 @@ class AvoidImplicitRule(AnsibleLintRule): """Rule that identifies use of undocumented or discouraged implicit behaviors.""" id = "avoid-implicit" - shortdesc = "Avoid implicit behaviors" + _shortdesc = "Avoid implicit behaviors" description = ( "Items which are templated should use ``template`` instead of " "``copy`` with ``content`` to ensure correctness." diff --git a/src/ansiblelint/rules/fqcn.py b/src/ansiblelint/rules/fqcn.py index 33b21f66e8..0bfb2d7cb0 100644 --- a/src/ansiblelint/rules/fqcn.py +++ b/src/ansiblelint/rules/fqcn.py @@ -126,6 +126,10 @@ def matchtask( if module not in self.module_aliases: loaded_module = load_plugin(module) + if not isinstance(loaded_module.resolved_fqcn, str): + msg = f"Invalid value ({loaded_module.resolved_fqcn})for resolved_fqcn attribute of {module} module." + _logger.warning(msg) + return [] target = loaded_module.resolved_fqcn self.module_aliases[module] = target if target is None: diff --git a/src/ansiblelint/rules/key_order.py b/src/ansiblelint/rules/key_order.py index 0c0a2f1a30..a16fa1154a 100644 --- a/src/ansiblelint/rules/key_order.py +++ b/src/ansiblelint/rules/key_order.py @@ -69,7 +69,6 @@ class KeyOrderRule(AnsibleLintRule, TransformMixin): """Ensure specific order of keys in mappings.""" id = "key-order" - shortdesc = __doc__ severity = "LOW" tags = ["formatting"] version_added = "v6.6.2" diff --git a/src/ansiblelint/rules/role_name.py b/src/ansiblelint/rules/role_name.py index ebe0b1a179..7794f48ea0 100644 --- a/src/ansiblelint/rules/role_name.py +++ b/src/ansiblelint/rules/role_name.py @@ -120,11 +120,15 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: if "roles" in play: line = play["__line__"] for role in play["roles"]: + role_name = None if isinstance(role, dict): line = role["__line__"] role_name = role["role"] elif isinstance(role, str): role_name = role + if not isinstance(role_name, str): + msg = "Role dependency has unexpected type." + raise TypeError(msg) if "/" in role_name: result.append( self.create_matcherror( diff --git a/src/ansiblelint/rules/run_once.py b/src/ansiblelint/rules/run_once.py index d656711858..6eeed0bb64 100644 --- a/src/ansiblelint/rules/run_once.py +++ b/src/ansiblelint/rules/run_once.py @@ -35,16 +35,17 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: if not file or file.kind != "playbook" or not data: return [] - strategy = data.get("strategy") + strategy: Any = data.get("strategy") run_once = data.get("run_once", False) if (not strategy and not run_once) or strategy != "free": return [] + lineno = getattr(strategy, "_line_number", 1) return [ self.create_matcherror( message="Play uses strategy: free", filename=file, tag=f"{self.id}[play]", - lineno=strategy._line_number, # noqa: SLF001 + lineno=lineno, ), ] diff --git a/src/ansiblelint/skip_utils.py b/src/ansiblelint/skip_utils.py index a36b401dc2..ecff3d68f5 100644 --- a/src/ansiblelint/skip_utils.py +++ b/src/ansiblelint/skip_utils.py @@ -26,6 +26,7 @@ import logging import re import warnings +from collections.abc import MutableMapping, Sequence from functools import cache from itertools import product from typing import TYPE_CHECKING, Any @@ -46,7 +47,7 @@ from ansiblelint.errors import LintWarning, WarnSource if TYPE_CHECKING: - from collections.abc import Generator, Sequence + from collections.abc import Generator from ansible.parsing.yaml.objects import AnsibleBaseYAMLObject @@ -169,7 +170,7 @@ def _append_skipped_rules( "galaxy", ]: # AnsibleMapping, dict - if hasattr(pyyaml_data, "get"): + if isinstance(pyyaml_data, MutableMapping): pyyaml_data[SKIPPED_RULES_KEY] = skipped_rules # AnsibleSequence, list elif ( @@ -182,15 +183,19 @@ def _append_skipped_rules( return pyyaml_data # create list of blocks of tasks or nested tasks - if lintable.kind in ("tasks", "handlers"): - ruamel_task_blocks = ruamel_data - pyyaml_task_blocks = pyyaml_data - elif lintable.kind == "playbook": - try: - pyyaml_task_blocks = _get_task_blocks_from_playbook(pyyaml_data) - ruamel_task_blocks = _get_task_blocks_from_playbook(ruamel_data) - except (AttributeError, TypeError): + pyyaml_task_blocks: Sequence[Any] + if lintable.kind in ("tasks", "handlers", "playbook"): + if not isinstance(pyyaml_data, Sequence): return pyyaml_data + if lintable.kind in ("tasks", "handlers"): + ruamel_task_blocks = ruamel_data + pyyaml_task_blocks = pyyaml_data + else: + try: + pyyaml_task_blocks = _get_task_blocks_from_playbook(pyyaml_data) + ruamel_task_blocks = _get_task_blocks_from_playbook(ruamel_data) + except (AttributeError, TypeError): + return pyyaml_data else: # For unsupported file types, we return empty skip lists return None diff --git a/src/ansiblelint/transformer.py b/src/ansiblelint/transformer.py index c610704651..cd6a00e785 100644 --- a/src/ansiblelint/transformer.py +++ b/src/ansiblelint/transformer.py @@ -121,14 +121,18 @@ def run(self) -> None: ) continue - if self.write_set != {"none"}: - self._do_transforms(file, ruamel_data or data, file_is_yaml, matches) + if self.write_set != {"none"}: + self._do_transforms( + file, ruamel_data or data, file_is_yaml, matches + ) - if file_is_yaml: _logger.debug("%s %s, version=%s", self.DUMP_MSG, file, yaml.version) # noinspection PyUnboundLocalVariable file.content = yaml.dumps(ruamel_data) + elif self.write_set != {"none"}: + self._do_transforms(file, ruamel_data or data, file_is_yaml, matches) + if file.updated: file.write() diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index 534db9cda5..83a70d4c0b 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -119,7 +119,7 @@ def ansible_templar(basedir: Path, templatevars: Any) -> Templar: basedir = basedir.parent dataloader = DataLoader() - dataloader.set_basedir(basedir) + dataloader.set_basedir(str(basedir)) templar = Templar(dataloader, variables=templatevars) return templar diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index c326afa8f5..f273357228 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -1012,15 +1012,15 @@ def version(self) -> tuple[int, int] | None: return None @version.setter - def version(self, value: tuple[int, int] | None) -> None: + def version(self, val: tuple[int, int] | None) -> None: """Ensure that yaml version uses our default value. The yaml Reader updates this value based on the ``%YAML`` directive in files. So, if a file does not include the directive, it sets this to None. But, None effectively resets the parsing version to YAML 1.2 (ruamel's default). """ - if value is not None: - self._yaml_version = value + if val is not None: + self._yaml_version = val elif hasattr(self, "_yaml_version_default"): self._yaml_version = self._yaml_version_default # We do nothing if the object did not have a previous default version defined diff --git a/tox.ini b/tox.ini index bae58b10df..fc2245183d 100644 --- a/tox.ini +++ b/tox.ini @@ -33,7 +33,6 @@ commands_pre = sh -c "rm -f {envdir}/.coverage.* 2>/dev/null || true" # safety measure to assure we do not accidentally run tests with broken dependencies {envpython} -m pip check - {envpython} -m pip freeze bash ./tools/install-reqs.sh ansible --version commands =