Skip to content

Commit

Permalink
Reduce use of options global variable (#3827)
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea authored Oct 10, 2023
1 parent 5ca5679 commit ddc1ef3
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 63 deletions.
13 changes: 13 additions & 0 deletions src/ansiblelint/_internal/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
9 changes: 0 additions & 9 deletions src/ansiblelint/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -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."""
Expand Down
9 changes: 3 additions & 6 deletions src/ansiblelint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 5 additions & 4 deletions src/ansiblelint/rules/complexity.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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()

Expand Down
10 changes: 4 additions & 6 deletions src/ansiblelint/rules/name.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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",
Expand Down
8 changes: 4 additions & 4 deletions src/ansiblelint/rules/no_prompting.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
14 changes: 10 additions & 4 deletions src/ansiblelint/rules/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
from ansiblelint.text import has_jinja

if TYPE_CHECKING:
from ansiblelint.config import Options
from ansiblelint.utils import Task


Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()

Expand All @@ -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()

Expand Down
10 changes: 7 additions & 3 deletions src/ansiblelint/rules/var_naming.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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
Expand Down
11 changes: 8 additions & 3 deletions src/ansiblelint/rules/yaml_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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()

Expand Down
23 changes: 12 additions & 11 deletions test/test_ansiblelintrule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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
1 change: 0 additions & 1 deletion test/test_cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down
12 changes: 6 additions & 6 deletions test/test_mockings.py
Original file line number Diff line number Diff line change
@@ -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
9 changes: 6 additions & 3 deletions test/test_rules_collection.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions test/test_schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"]
Expand Down
2 changes: 1 addition & 1 deletion test/test_transformer.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit ddc1ef3

Please sign in to comment.