diff --git a/pyproject.toml b/pyproject.toml index 681251aa3c..94d773af3f 100644 --- a/pyproject.toml +++ b/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" diff --git a/src/ansiblelint/_internal/rules.py b/src/ansiblelint/_internal/rules.py index b85cfbeac5..3c3f475ac5 100644 --- a/src/ansiblelint/_internal/rules.py +++ b/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 diff --git a/src/ansiblelint/errors.py b/src/ansiblelint/errors.py index 8ebe6e7394..8bebca617a 100644 --- a/src/ansiblelint/errors.py +++ b/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" diff --git a/src/ansiblelint/logger.py b/src/ansiblelint/logger.py index f0477cd59c..9588c7a9e6 100644 --- a/src/ansiblelint/logger.py +++ b/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) diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index ddc4acd5b6..9273130e3a 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/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()}]", ), ] diff --git a/src/ansiblelint/rules/only_builtins.py b/src/ansiblelint/rules/only_builtins.py index 78ad93a9df..344df19aff 100644 --- a/src/ansiblelint/rules/only_builtins.py +++ b/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) diff --git a/src/ansiblelint/rules/schema.py b/src/ansiblelint/rules/schema.py index 5f81158fc0..0614f4e5e1 100644 --- a/src/ansiblelint/rules/schema.py +++ b/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}]", ), diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py index 6c2db41436..9776f16a65 100644 --- a/src/ansiblelint/runner.py +++ b/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( diff --git a/src/ansiblelint/testing/fixtures.py b/src/ansiblelint/testing/fixtures.py index fd4cf36319..010b0d73ff 100644 --- a/src/ansiblelint/testing/fixtures.py +++ b/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,9 +41,10 @@ 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()) @@ -55,9 +52,6 @@ def rule_runner(request: SubRequest, config_options: Options) -> RunFromText: @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() diff --git a/test/rules/test_syntax_check.py b/test/rules/test_syntax_check.py index 2fe36a3520..4efec8a6a0 100644 --- a/test/rules/test_syntax_check.py +++ b/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" diff --git a/test/test_ansiblelintrule.py b/test/test_ansiblelintrule.py index 472cece267..2ad5e46160 100644 --- a/test/test_ansiblelintrule.py +++ b/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: diff --git a/test/test_cli_role_paths.py b/test/test_cli_role_paths.py index 148e1ed3ce..f400fd485f 100644 --- a/test/test_cli_role_paths.py +++ b/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: diff --git a/test/test_formatter.py b/test/test_formatter.py index 68f05084b9..1e351ff6a0 100644 --- a/test/test_formatter.py +++ b/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." diff --git a/test/test_formatter_json.py b/test/test_formatter_json.py index 25aa5f550c..0edd73804f 100644 --- a/test/test_formatter_json.py +++ b/test/test_formatter_json.py @@ -11,7 +11,7 @@ from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.formatters import CodeclimateJSONFormatter -from ansiblelint.rules import AnsibleLintRule +from ansiblelint.rules import AnsibleLintRule, RulesCollection class TestCodeclimateJSONFormatter: @@ -20,12 +20,14 @@ class TestCodeclimateJSONFormatter: rule = AnsibleLintRule() matches: list[MatchError] = [] formatter: CodeclimateJSONFormatter | None = None + collection = RulesCollection() def setup_class(self) -> None: """Set up few MatchError objects.""" self.rule = AnsibleLintRule() self.rule.id = "TCF0001" self.rule.severity = "VERY_HIGH" + self.collection.register(self.rule) self.matches = [] self.matches.append( MatchError( diff --git a/test/test_formatter_sarif.py b/test/test_formatter_sarif.py index 1b33fda637..77aa13d28d 100644 --- a/test/test_formatter_sarif.py +++ b/test/test_formatter_sarif.py @@ -13,7 +13,7 @@ from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.formatters import SarifFormatter -from ansiblelint.rules import AnsibleLintRule +from ansiblelint.rules import AnsibleLintRule, RulesCollection class TestSarifFormatter: @@ -22,6 +22,7 @@ class TestSarifFormatter: rule = AnsibleLintRule() matches: list[MatchError] = [] formatter: SarifFormatter | None = None + collection = RulesCollection() def setup_class(self) -> None: """Set up few MatchError objects.""" @@ -31,6 +32,8 @@ def setup_class(self) -> None: self.rule.description = "This is the rule description." self.rule.link = "https://rules/help#TCF0001" self.rule.tags = ["tag1", "tag2"] + self.collection.register(self.rule) + self.matches = [] self.matches.append( MatchError(