From ddc1ef309940853a3a03c6aa047fd74fe352e75f Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Tue, 10 Oct 2023 11:29:02 +0100 Subject: [PATCH] Reduce use of options global variable (#3827) --- src/ansiblelint/_internal/rules.py | 13 +++++++++++++ src/ansiblelint/config.py | 9 --------- src/ansiblelint/rules/__init__.py | 9 +++------ src/ansiblelint/rules/complexity.py | 9 +++++---- src/ansiblelint/rules/name.py | 10 ++++------ src/ansiblelint/rules/no_prompting.py | 8 ++++---- src/ansiblelint/rules/schema.py | 14 ++++++++++---- src/ansiblelint/rules/var_naming.py | 10 +++++++--- src/ansiblelint/rules/yaml_rule.py | 11 ++++++++--- test/test_ansiblelintrule.py | 23 ++++++++++++----------- test/test_cli.py | 1 - test/test_mockings.py | 12 ++++++------ test/test_rules_collection.py | 9 ++++++--- test/test_schemas.py | 2 -- test/test_transformer.py | 2 +- 15 files changed, 79 insertions(+), 63 deletions(-) diff --git a/src/ansiblelint/_internal/rules.py b/src/ansiblelint/_internal/rules.py index acaf0f38c1..b85cfbeac5 100644 --- a/src/ansiblelint/_internal/rules.py +++ b/src/ansiblelint/_internal/rules.py @@ -157,6 +157,19 @@ def ids(cls) -> dict[str, str]: """ return getattr(cls, "_ids", {cls.id: cls.shortdesc}) + @property + def rule_config(self) -> dict[str, Any]: + """Retrieve rule specific configuration.""" + if not self._collection: + msg = "A rule that is not part of a collection cannot access its configuration." + _logger.warning(msg) + raise RuntimeError(msg) + rule_config = self._collection.options.rules.get(self.id, {}) + if not isinstance(rule_config, dict): # pragma: no branch + msg = f"Invalid rule config for {self.id}: {rule_config}" + raise RuntimeError(msg) + return rule_config + # pylint: enable=unused-argument diff --git a/src/ansiblelint/config.py b/src/ansiblelint/config.py index b65dc26782..3098fdb7ea 100644 --- a/src/ansiblelint/config.py +++ b/src/ansiblelint/config.py @@ -172,15 +172,6 @@ class Options: # pylint: disable=too-many-instance-attributes log_entries: list[tuple[int, str]] = [] -def get_rule_config(rule_id: str) -> dict[str, Any]: - """Get configurations for the rule ``rule_id``.""" - rule_config = options.rules.get(rule_id, {}) - if not isinstance(rule_config, dict): # pragma: no branch - msg = f"Invalid rule config for {rule_id}: {rule_config}" - raise RuntimeError(msg) - return rule_config - - @lru_cache def ansible_collections_path() -> str: """Return collection path variable for current version of Ansible.""" diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index 05949b8c3b..ddc4acd5b6 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/src/ansiblelint/rules/__init__.py @@ -23,7 +23,7 @@ WarningRule, ) from ansiblelint.app import App, get_app -from ansiblelint.config import PROFILES, Options, get_rule_config +from ansiblelint.config import PROFILES, Options from ansiblelint.config import options as default_options from ansiblelint.constants import LINE_NUMBER_KEY, RULE_DOC_URL, SKIPPED_RULES_KEY from ansiblelint.errors import MatchError @@ -55,11 +55,6 @@ def url(self) -> str: """Return rule documentation url.""" return RULE_DOC_URL + self.id + "/" - @property - def rule_config(self) -> dict[str, Any]: - """Retrieve rule specific configuration.""" - return get_rule_config(self.id) - def get_config(self, key: str) -> Any: """Return a configured value for given key string.""" return self.rule_config.get(key, None) @@ -408,6 +403,8 @@ def __init__( # pylint: disable=too-many-arguments WarningRule(), ], ) + for rule in self.rules: + rule._collection = self # noqa: SLF001 for rule in load_plugins(rulesdirs_str): self.register(rule, conditional=conditional) self.rules = sorted(self.rules) diff --git a/src/ansiblelint/rules/complexity.py b/src/ansiblelint/rules/complexity.py index b48937faab..29a8382ae6 100644 --- a/src/ansiblelint/rules/complexity.py +++ b/src/ansiblelint/rules/complexity.py @@ -9,6 +9,7 @@ from ansiblelint.rules import AnsibleLintRule, RulesCollection if TYPE_CHECKING: + from ansiblelint.config import Options from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -77,7 +78,6 @@ def calculate_block_depth(self, task: Task) -> int: import pytest # pylint: disable=ungrouped-imports - from ansiblelint.config import options from ansiblelint.runner import Runner @pytest.mark.parametrize( @@ -99,11 +99,12 @@ def test_complexity( file: str, expected_results: list[str], monkeypatch: pytest.MonkeyPatch, + config_options: Options, ) -> None: """Test rule.""" - monkeypatch.setattr(options, "max_tasks", 5) - monkeypatch.setattr(options, "max_block_depth", 3) - collection = RulesCollection(options=options) + monkeypatch.setattr(config_options, "max_tasks", 5) + monkeypatch.setattr(config_options, "max_block_depth", 3) + collection = RulesCollection(options=config_options) collection.register(ComplexityRule()) results = Runner(file, rules=collection).run() diff --git a/src/ansiblelint/rules/name.py b/src/ansiblelint/rules/name.py index ced0e74ca0..57d6689b3c 100644 --- a/src/ansiblelint/rules/name.py +++ b/src/ansiblelint/rules/name.py @@ -3,7 +3,6 @@ import re import sys -from copy import deepcopy from typing import TYPE_CHECKING, Any from ansiblelint.constants import LINE_NUMBER_KEY @@ -12,6 +11,7 @@ if TYPE_CHECKING: from ruamel.yaml.comments import CommentedMap, CommentedSeq + from ansiblelint.config import Options from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -181,7 +181,6 @@ def transform( if "pytest" in sys.modules: - from ansiblelint.config import options from ansiblelint.file_utils import Lintable from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner @@ -203,11 +202,10 @@ def test_file_negative() -> None: errs = bad_runner.run() assert len(errs) == 5 - def test_name_prefix_negative() -> None: + def test_name_prefix_negative(config_options: Options) -> None: """Negative test for name[missing].""" - custom_options = deepcopy(options) - custom_options.enable_list = ["name[prefix]"] - collection = RulesCollection(options=custom_options) + config_options.enable_list = ["name[prefix]"] + collection = RulesCollection(options=config_options) collection.register(NameRule()) failure = Lintable( "examples/playbooks/tasks/rule-name-prefix-fail.yml", diff --git a/src/ansiblelint/rules/no_prompting.py b/src/ansiblelint/rules/no_prompting.py index 1a2db27cd7..7b6b987c9e 100644 --- a/src/ansiblelint/rules/no_prompting.py +++ b/src/ansiblelint/rules/no_prompting.py @@ -8,6 +8,7 @@ from ansiblelint.rules import AnsibleLintRule if TYPE_CHECKING: + from ansiblelint.config import Options from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -60,15 +61,14 @@ def matchtask( if "pytest" in sys.modules: - from ansiblelint.config import options from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner - def test_no_prompting_fail() -> None: + def test_no_prompting_fail(config_options: Options) -> None: """Negative test for no-prompting.""" # For testing we want to manually enable opt-in rules - options.enable_list = ["no-prompting"] - rules = RulesCollection(options=options) + config_options.enable_list = ["no-prompting"] + rules = RulesCollection(options=config_options) rules.register(NoPromptingRule()) results = Runner("examples/playbooks/rule-no-prompting.yml", rules=rules).run() assert len(results) == 2 diff --git a/src/ansiblelint/rules/schema.py b/src/ansiblelint/rules/schema.py index 08e9545f33..5f81158fc0 100644 --- a/src/ansiblelint/rules/schema.py +++ b/src/ansiblelint/rules/schema.py @@ -14,6 +14,7 @@ from ansiblelint.text import has_jinja if TYPE_CHECKING: + from ansiblelint.config import Options from ansiblelint.utils import Task @@ -184,7 +185,6 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: import pytest # pylint: disable=ungrouped-imports - from ansiblelint.config import options from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner @@ -343,12 +343,17 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: ), ), ) - def test_schema(file: str, expected_kind: str, expected: list[str]) -> None: + def test_schema( + file: str, + expected_kind: str, + expected: list[str], + config_options: Options, + ) -> None: """Validate parsing of ansible output.""" lintable = Lintable(file) assert lintable.kind == expected_kind - rules = RulesCollection(options=options) + rules = RulesCollection(options=config_options) rules.register(ValidateSchemaRule()) results = Runner(lintable, rules=rules).run() @@ -375,12 +380,13 @@ def test_schema_moves( expected_kind: str, expected_tag: str, count: int, + config_options: Options, ) -> None: """Validate ability to detect schema[moves].""" lintable = Lintable(file) assert lintable.kind == expected_kind - rules = RulesCollection(options=options) + rules = RulesCollection(options=config_options) rules.register(ValidateSchemaRule()) results = Runner(lintable, rules=rules).run() diff --git a/src/ansiblelint/rules/var_naming.py b/src/ansiblelint/rules/var_naming.py index a73394b5ef..b64cb4029f 100644 --- a/src/ansiblelint/rules/var_naming.py +++ b/src/ansiblelint/rules/var_naming.py @@ -9,7 +9,7 @@ from ansible.parsing.yaml.objects import AnsibleUnicode from ansible.vars.reserved import get_reserved_names -from ansiblelint.config import options +from ansiblelint.config import Options, options from ansiblelint.constants import ( ANNOTATION_KEYS, LINE_NUMBER_KEY, @@ -349,9 +349,13 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: pytest.param("examples/Taskfile.yml", 0, id="1"), ), ) - def test_invalid_var_name_playbook(file: str, expected: int) -> None: + def test_invalid_var_name_playbook( + file: str, + expected: int, + config_options: Options, + ) -> None: """Test rule matches.""" - rules = RulesCollection(options=options) + rules = RulesCollection(options=config_options) rules.register(VariableNamingRule()) results = Runner(Lintable(file), rules=rules).run() assert len(results) == expected diff --git a/src/ansiblelint/rules/yaml_rule.py b/src/ansiblelint/rules/yaml_rule.py index 0784706da8..42e911982f 100644 --- a/src/ansiblelint/rules/yaml_rule.py +++ b/src/ansiblelint/rules/yaml_rule.py @@ -16,6 +16,7 @@ if TYPE_CHECKING: from typing import Any + from ansiblelint.config import Options from ansiblelint.errors import MatchError _logger = logging.getLogger(__name__) @@ -134,7 +135,6 @@ def _fetch_skips(data: Any, collector: dict[int, set[str]]) -> dict[int, set[str import pytest # pylint: disable=ungrouped-imports - from ansiblelint.config import options from ansiblelint.rules import RulesCollection from ansiblelint.runner import Runner @@ -195,12 +195,17 @@ def _fetch_skips(data: Any, collector: dict[int, set[str]]) -> dict[int, set[str ), ) @pytest.mark.filterwarnings("ignore::ansible_compat.runtime.AnsibleWarning") - def test_yamllint(file: str, expected_kind: str, expected: list[str]) -> None: + def test_yamllint( + file: str, + expected_kind: str, + expected: list[str], + config_options: Options, + ) -> None: """Validate parsing of ansible output.""" lintable = Lintable(file) assert lintable.kind == expected_kind - rules = RulesCollection(options=options) + rules = RulesCollection(options=config_options) rules.register(YamllintRule()) results = Runner(lintable, rules=rules).run() diff --git a/test/test_ansiblelintrule.py b/test/test_ansiblelintrule.py index c576e0f9fb..472cece267 100644 --- a/test/test_ansiblelintrule.py +++ b/test/test_ansiblelintrule.py @@ -5,11 +5,10 @@ import pytest -from ansiblelint.config import options -from ansiblelint.rules import AnsibleLintRule +from ansiblelint.rules import AnsibleLintRule, RulesCollection if TYPE_CHECKING: - from _pytest.monkeypatch import MonkeyPatch + from ansiblelint.config import Options def test_unjinja() -> None: @@ -20,12 +19,14 @@ def test_unjinja() -> None: @pytest.mark.parametrize("rule_config", ({}, {"foo": True, "bar": 1})) -def test_rule_config(rule_config: dict[str, Any], monkeypatch: MonkeyPatch) -> None: +def test_rule_config( + rule_config: dict[str, Any], + config_options: Options, +) -> None: """Check that a rule config is inherited from options.""" - rule_id = "rule-0" - monkeypatch.setattr(AnsibleLintRule, "id", rule_id) - monkeypatch.setitem(options.rules, rule_id, rule_config) - - rule = AnsibleLintRule() - assert set(rule.rule_config.items()) == set(rule_config.items()) - assert all(rule.get_config(k) == v for k, v in rule_config.items()) + config_options.rules["load-failure"] = rule_config + rules = RulesCollection(options=config_options) + for rule in rules: + if rule.id == "load-failure": + assert rule._collection # noqa: SLF001 + assert rule.rule_config == rule_config diff --git a/test/test_cli.py b/test/test_cli.py index 3e3bcb9c20..967b61cf24 100644 --- a/test/test_cli.py +++ b/test/test_cli.py @@ -160,7 +160,6 @@ def test_ensure_write_cli_does_not_consume_lintables( from_file=[], from_cli=orig_cli_value, ) - # breakpoint() assert cli_value == expected diff --git a/test/test_mockings.py b/test/test_mockings.py index 0e8d77a339..417d5d5b14 100644 --- a/test/test_mockings.py +++ b/test/test_mockings.py @@ -1,18 +1,18 @@ """Test mockings module.""" -from typing import Any + +from pathlib import Path import pytest from ansiblelint._mockings import _make_module_stub -from ansiblelint.config import options +from ansiblelint.config import Options from ansiblelint.constants import RC -def test_make_module_stub(mocker: Any) -> None: +def test_make_module_stub(config_options: Options) -> None: """Test make module stub.""" - mocker.patch("ansiblelint.config.options.cache_dir", return_value=".") - assert options.cache_dir is not None + config_options.cache_dir = Path() # current directory with pytest.raises(SystemExit) as exc: - _make_module_stub(module_name="", options=options) + _make_module_stub(module_name="", options=config_options) assert exc.type == SystemExit assert exc.value.code == RC.INVALID_CONFIG diff --git a/test/test_rules_collection.py b/test/test_rules_collection.py index 8dec2063bf..f652ec8474 100644 --- a/test/test_rules_collection.py +++ b/test/test_rules_collection.py @@ -23,14 +23,17 @@ import collections import re from pathlib import Path +from typing import TYPE_CHECKING import pytest -from ansiblelint.config import options from ansiblelint.file_utils import Lintable from ansiblelint.rules import RulesCollection from ansiblelint.testing import run_ansible_lint +if TYPE_CHECKING: + from ansiblelint.config import Options + @pytest.fixture(name="test_rules_collection") def fixture_test_rules_collection() -> RulesCollection: @@ -153,12 +156,12 @@ def test_rich_rule_listing() -> None: assert rule.description[:30] in result.stdout -def test_rules_id_format() -> None: +def test_rules_id_format(config_options: Options) -> None: """Assure all our rules have consistent format.""" rule_id_re = re.compile("^[a-z-]{4,30}$") rules = RulesCollection( [Path("./src/ansiblelint/rules").resolve()], - options=options, + options=config_options, conditional=False, ) keys: set[str] = set() diff --git a/test/test_schemas.py b/test/test_schemas.py index 29a37de650..6276518660 100644 --- a/test/test_schemas.py +++ b/test/test_schemas.py @@ -92,8 +92,6 @@ def test_spdx() -> None: with spdx_config_path.open(encoding="utf-8") as license_fh: licenses = json.load(license_fh) for lic in licenses: - # for lic in lic_dic: - # breakpoint() if lic.get("is_deprecated"): continue lic_id = lic["spdx_license_key"] diff --git a/test/test_transformer.py b/test/test_transformer.py index dad039d6bc..13af46a41c 100644 --- a/test/test_transformer.py +++ b/test/test_transformer.py @@ -130,7 +130,7 @@ def fixture_runner_result( ), ) @mock.patch.dict(os.environ, {"ANSIBLE_LINT_WRITE_TMP": "1"}, clear=True) -def test_transformer( # pylint: disable=too-many-arguments, too-many-locals +def test_transformer( config_options: Options, playbook_str: str, runner_result: LintResult,