Reduce use of options global (2) (#3828)

pyproject.toml

@@ -163,10 +163,6 @@ enable = [
 [tool.pylint.REPORTING]
 output-format = "colorized"
 
-[tool.pylint.TYPECHECK]
-# pylint is unable to detect Namespace attributes and will throw a E1101
-generated-members = "options.*"
-
 [tool.pylint.SUMMARY]
 # We don't need the score spamming console, as we either pass or fail
 score = "n"

src/ansiblelint/_internal/rules.py

@@ -9,6 +9,7 @@
 from ansiblelint.constants import RULE_DOC_URL
 
 if TYPE_CHECKING:
+    from ansiblelint.config import Options
     from ansiblelint.errors import MatchError
     from ansiblelint.file_utils import Lintable
     from ansiblelint.rules import RulesCollection
@@ -160,16 +161,21 @@ def ids(cls) -> dict[str, str]:
     @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, {})
+        rule_config = self.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
 
+    @property
+    def options(self) -> Options:
+        """Used to access linter configuration."""
+        if self._collection is None:
+            msg = f"A rule ({self.id}) that is not part of a collection cannot access its configuration."
+            _logger.warning(msg)
+            raise RuntimeError(msg)
+        return self._collection.options
+
 
 # pylint: enable=unused-argument
 

src/ansiblelint/errors.py

@@ -6,7 +6,6 @@
 from typing import TYPE_CHECKING, Any
 
 from ansiblelint._internal.rules import BaseRule, RuntimeErrorRule
-from ansiblelint.config import options
 from ansiblelint.file_utils import Lintable
 
 if TYPE_CHECKING:
@@ -27,17 +26,6 @@ class WarnSource:
     message: str | None = None
 
 
-class StrictModeError(RuntimeError):
-    """Raise when we encounter a warning in strict mode."""
-
-    def __init__(
-        self,
-        message: str = "Warning treated as error due to --strict option.",
-    ):
-        """Initialize a StrictModeError instance."""
-        super().__init__(message)
-
-
 @dataclass(frozen=True)
 class RuleMatchTransformMeta:
     """Additional metadata about a match error to be used during transformation."""
@@ -117,7 +105,7 @@ def __post_init__(self) -> None:
     def level(self) -> str:
         """Return the level of the rule: error, warning or notice."""
         if not self.ignored and {self.tag, self.rule.id, *self.rule.tags}.isdisjoint(
-            options.warn_list,
+            self.rule.options.warn_list,
         ):
             return "error"
         return "warning"

src/ansiblelint/logger.py

@@ -17,15 +17,3 @@ def timed_info(msg: Any, *args: Any) -> Iterator[None]:
     finally:
         elapsed = time.time() - start
         _logger.info(msg + " (%.2fs)", *(*args, elapsed))  # noqa: G003
-
-
-def warn_or_fail(message: str) -> None:
-    """Warn or fail depending on the strictness level."""
-    # pylint: disable=import-outside-toplevel
-    from ansiblelint.config import options
-    from ansiblelint.errors import StrictModeError
-
-    if options.strict:
-        raise StrictModeError(message)
-
-    _logger.warning(message)

src/ansiblelint/rules/__init__.py

@@ -223,7 +223,16 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
         if isinstance(yaml, str):
             if yaml.startswith("$ANSIBLE_VAULT"):
                 return []
-            return [MatchError(lintable=file, rule=LoadingFailureRule())]
+            if self._collection is None:
+                msg = f"Rule {self.id} was not added to a collection."
+                raise RuntimeError(msg)
+            return [
+                # pylint: disable=E1136
+                MatchError(
+                    lintable=file,
+                    rule=self._collection["load-failure"],
+                ),
+            ]
         if not yaml:
             return matches
 
@@ -443,6 +452,14 @@ def __len__(self) -> int:
         """Return the length of the RulesCollection data."""
         return len(self.rules)
 
+    def __getitem__(self, item: Any) -> BaseRule:
+        """Return a rule from inside the collection based on its id."""
+        for rule in self.rules:
+            if rule.id == item:
+                return rule
+        msg = f"Rule {item} is not present inside this collection."
+        raise ValueError(msg)
+
     def extend(self, more: list[AnsibleLintRule]) -> None:
         """Combine rules."""
         self.rules.extend(more)
@@ -469,7 +486,7 @@ def run(
                     MatchError(
                         message=str(exc),
                         lintable=file,
-                        rule=LoadingFailureRule(),
+                        rule=self["load-failure"],
                         tag=f"{LoadingFailureRule.id}[{exc.__class__.__name__.lower()}]",
                     ),
                 ]

src/ansiblelint/rules/only_builtins.py

@@ -5,7 +5,6 @@
 import sys
 from typing import TYPE_CHECKING
 
-from ansiblelint.config import options
 from ansiblelint.rules import AnsibleLintRule
 from ansiblelint.rules.fqcn import builtins
 from ansiblelint.skip_utils import is_nested_task
@@ -33,9 +32,9 @@ def matchtask(
         allowed_collections = [
             "ansible.builtin",
             "ansible.legacy",
-            *options.only_builtins_allow_collections,
+            *self.options.only_builtins_allow_collections,
         ]
-        allowed_modules = builtins + options.only_builtins_allow_modules
+        allowed_modules = builtins + self.options.only_builtins_allow_modules
 
         is_allowed = (
             any(module.startswith(f"{prefix}.") for prefix in allowed_collections)

src/ansiblelint/rules/schema.py

@@ -119,7 +119,7 @@ def _get_field_matches(
                             message=msg,
                             lineno=data.get("__line__", 1),
                             lintable=file,
-                            rule=ValidateSchemaRule(),
+                            rule=self,
                             details=ValidateSchemaRule.description,
                             tag=f"schema[{file.kind}]",
                         ),
@@ -143,7 +143,7 @@ def matchtask(
                     MatchError(
                         message=msg,
                         lintable=file,
-                        rule=ValidateSchemaRule(),
+                        rule=self,
                         details=ValidateSchemaRule.description,
                         tag=f"schema[{tag}]",
                     ),
@@ -168,7 +168,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
                 MatchError(
                     message=error,
                     lintable=file,
-                    rule=ValidateSchemaRule(),
+                    rule=self,
                     details=ValidateSchemaRule.description,
                     tag=f"schema[{file.kind}]",
                 ),

src/ansiblelint/runner.py

@@ -28,15 +28,13 @@
 from ansiblelint._internal.rules import (
     BaseRule,
     LoadingFailureRule,
-    RuntimeErrorRule,
-    WarningRule,
 )
 from ansiblelint.app import App, get_app
 from ansiblelint.constants import States
 from ansiblelint.errors import LintWarning, MatchError, WarnSource
 from ansiblelint.file_utils import Lintable, expand_dirs_in_lintables
 from ansiblelint.logger import timed_info
-from ansiblelint.rules.syntax_check import OUTPUT_PATTERNS, AnsibleSyntaxCheckRule
+from ansiblelint.rules.syntax_check import OUTPUT_PATTERNS
 from ansiblelint.text import strip_ansi_escape
 from ansiblelint.utils import (
     PLAYBOOK_DIR,
@@ -176,7 +174,7 @@ def run(self) -> list[MatchError]:
                     if isinstance(warn.source, WarnSource):
                         match = MatchError(
                             message=warn.source.message or warn.category.__name__,
-                            rule=WarningRule(),
+                            rule=self.rules["warning"],
                             filename=warn.source.filename.filename,
                             tag=warn.source.tag,
                             lineno=warn.source.lineno,
@@ -187,7 +185,7 @@ def run(self) -> list[MatchError]:
                             message=warn.message
                             if isinstance(warn.message, str)
                             else "?",
-                            rule=WarningRule(),
+                            rule=self.rules["warning"],
                             filename=str(filename),
                         )
                     matches.append(match)
@@ -219,7 +217,7 @@ def _run(self) -> list[MatchError]:
                         lintable=lintable,
                         message=str(lintable.exc),
                         details=str(lintable.exc.__cause__),
-                        rule=LoadingFailureRule(),
+                        rule=self.rules["load-failure"],
                         tag=f"load-failure[{lintable.exc.__class__.__name__.lower()}]",
                     ),
                 )
@@ -230,7 +228,7 @@ def _run(self) -> list[MatchError]:
                     MatchError(
                         lintable=lintable,
                         message="File or directory not found.",
-                        rule=LoadingFailureRule(),
+                        rule=self.rules["load-failure"],
                         tag="load-failure[not-found]",
                     ),
                 )
@@ -303,7 +301,12 @@ def _get_ansible_syntax_check_matches(
         app: App,
     ) -> list[MatchError]:
         """Run ansible syntax check and return a list of MatchError(s)."""
-        default_rule: BaseRule = AnsibleSyntaxCheckRule()
+        try:
+            default_rule: BaseRule = self.rules["syntax-check"]
+        except ValueError:
+            # if syntax-check is not loaded, we do not perform any syntax check,
+            # that might happen during testing
+            return []
         fh = None
         results = []
         if lintable.kind not in ("playbook", "role"):
@@ -404,7 +407,7 @@ def _get_ansible_syntax_check_matches(
                     )
 
             if not results:
-                rule = RuntimeErrorRule()
+                rule = self.rules["internal-error"]
                 message = (
                     f"Unexpected error code {run.returncode} from "
                     f"execution of: {' '.join(cmd)}"
@@ -450,7 +453,7 @@ def _emit_matches(self, files: list[Lintable]) -> Generator[MatchError, None, No
                     exc.rule = LoadingFailureRule()
                     yield exc
                 except AttributeError:
-                    yield MatchError(lintable=lintable, rule=LoadingFailureRule())
+                    yield MatchError(lintable=lintable, rule=self.rules["load-failure"])
                 visited.add(lintable)
 
     def find_children(self, lintable: Lintable) -> list[Lintable]:
@@ -472,7 +475,7 @@ def find_children(self, lintable: Lintable) -> list[Lintable]:
         results = []
         # playbook_ds can be an AnsibleUnicode string, which we consider invalid
         if isinstance(playbook_ds, str):
-            raise MatchError(lintable=lintable, rule=LoadingFailureRule())
+            raise MatchError(lintable=lintable, rule=self.rules["load-failure"])
         for item in ansiblelint.utils.playbook_items(playbook_ds):
             # if lintable.kind not in ["playbook"]:
             for child in self.play_children(

src/ansiblelint/testing/fixtures.py

@@ -7,19 +7,16 @@
 """
 from __future__ import annotations
 
-import copy
 from typing import TYPE_CHECKING
 
 import pytest
 
-from ansiblelint.config import Options, options
+from ansiblelint.config import Options
 from ansiblelint.constants import DEFAULT_RULESDIR
 from ansiblelint.rules import RulesCollection
 from ansiblelint.testing import RunFromText
 
 if TYPE_CHECKING:
-    from collections.abc import Iterator
-
     from _pytest.fixtures import SubRequest
 
 
@@ -29,13 +26,12 @@
 def fixture_default_rules_collection() -> RulesCollection:
     """Return default rule collection."""
     assert DEFAULT_RULESDIR.is_dir()
-    # For testing we want to manually enable opt-in rules
-    test_options = copy.deepcopy(options)
-    test_options.enable_list = ["no-same-owner"]
+    config_options = Options()
+    config_options.enable_list = ["no-same-owner"]
     # That is instantiated very often and do want to avoid ansible-galaxy
     # install errors due to concurrency.
-    test_options.offline = True
-    return RulesCollection(rulesdirs=[DEFAULT_RULESDIR], options=test_options)
+    config_options.offline = True
+    return RulesCollection(rulesdirs=[DEFAULT_RULESDIR], options=config_options)
 
 
 @pytest.fixture()
@@ -45,19 +41,17 @@ def default_text_runner(default_rules_collection: RulesCollection) -> RunFromTex
 
 
 @pytest.fixture()
-def rule_runner(request: SubRequest, config_options: Options) -> RunFromText:
+def rule_runner(request: SubRequest) -> RunFromText:
     """Return runner for a specific rule class."""
     rule_class = request.param
+    config_options = Options()
     config_options.enable_list.append(rule_class().id)
     collection = RulesCollection(options=config_options)
     collection.register(rule_class())
     return RunFromText(collection)
 
 
 @pytest.fixture(name="config_options")
-def fixture_config_options() -> Iterator[Options]:
+def fixture_config_options() -> Options:
     """Return configuration options that will be restored after testrun."""
-    global options  # pylint: disable=global-statement # noqa: PLW0603
-    original_options = copy.deepcopy(options)
-    yield options
-    options = original_options
+    return Options()

test/rules/test_syntax_check.py

@@ -58,11 +58,10 @@ def test_extra_vars_passed_to_command(
     assert not result
 
 
-def test_syntax_check_role() -> None:
+def test_syntax_check_role(default_rules_collection: RulesCollection) -> None:
     """Validate syntax check of a broken role."""
     lintable = Lintable("examples/playbooks/roles/invalid_due_syntax", kind="role")
-    rules = RulesCollection()
-    result = Runner(lintable, rules=rules).run()
+    result = Runner(lintable, rules=default_rules_collection).run()
     assert len(result) == 1, result
     assert result[0].lineno == 2
     assert result[0].filename == "examples/roles/invalid_due_syntax/tasks/main.yml"

test/test_ansiblelintrule.py

@@ -23,7 +23,7 @@ def test_rule_config(
     rule_config: dict[str, Any],
     config_options: Options,
 ) -> None:
-    """Check that a rule config is inherited from options."""
+    """Check that a rule config can be accessed."""
     config_options.rules["load-failure"] = rule_config
     rules = RulesCollection(options=config_options)
     for rule in rules:

test/test_cli_role_paths.py

@@ -174,7 +174,10 @@ def test_run_single_role_path_with_roles_path_env(local_test_dir: Path) -> None:
 
 @pytest.mark.parametrize(
     ("result", "env"),
-    ((True, {"GITHUB_ACTIONS": "true", "GITHUB_WORKFLOW": "foo"}), (False, None)),
+    (
+        (True, {"GITHUB_ACTIONS": "true", "GITHUB_WORKFLOW": "foo", "NO_COLOR": "1"}),
+        (False, None),
+    ),
     ids=("on", "off"),
 )
 def test_run_playbook_github(result: bool, env: dict[str, str]) -> None:

test/test_formatter.py

@@ -23,10 +23,12 @@
 from ansiblelint.errors import MatchError
 from ansiblelint.file_utils import Lintable
 from ansiblelint.formatters import Formatter
-from ansiblelint.rules import AnsibleLintRule
+from ansiblelint.rules import AnsibleLintRule, RulesCollection
 
+collection = RulesCollection()
 rule = AnsibleLintRule()
 rule.id = "TCF0001"
+collection.register(rule)
 formatter = Formatter(pathlib.Path.cwd(), display_relative_path=True)
 # These details would generate a rich rendering error if not escaped:
 DETAILS = "Some [/tmp/foo] details."