diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 17c48b66f8..112ca89b51 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -180,7 +180,7 @@ repos: - wcmatch - yamllint>=1.34.0 - repo: https://github.com/RobertCraigie/pyright-python - rev: v1.1.397 + rev: v1.1.398 hooks: - id: pyright additional_dependencies: *deps diff --git a/conftest.py b/conftest.py index 3770a7e0cd..1b6d60fd90 100644 --- a/conftest.py +++ b/conftest.py @@ -31,6 +31,11 @@ def pytest_configure(config: pytest.Config) -> None: if is_help_option_present(config): return if is_master(config): + # linter should be able de detect and convert some deprecation warnings + # into validation errors but during testing we disable this to avoid + # unnecessary noise. Still, we might want to enable it for particular + # tests, for testing our ability to detect deprecations. + os.environ["ANSIBLE_DEPRECATION_WARNINGS"] = "False" # we need to be sure that we have the requirements installed as some tests # might depend on these. This approach is compatible with GHA caching. try: diff --git a/examples/.no_collection_version/galaxy.yml b/examples/.no_collection_version/galaxy.yml index 95d9d180ec..c59e1c5734 100644 --- a/examples/.no_collection_version/galaxy.yml +++ b/examples/.no_collection_version/galaxy.yml @@ -6,8 +6,7 @@ authors: - John description: your collection description license: - - GPL - - Apache + - GPL-3.0-or-later dependencies: {} repository: http://example.com/repository diff --git a/pyproject.toml b/pyproject.toml index 17a0e8936e..60fc3c0107 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -388,6 +388,8 @@ mode = "standard" # reportMissingImports = false # https://github.com/microsoft/pyright/issues/9494 reportPossiblyUnboundVariable = false +# Introduced in v1.1.398 but already covered by ruff +reportPrivateImportUsage = false # spell-checker:ignore filterwarnings norecursedirs optionflags [tool.pytest.ini_options] diff --git a/src/ansiblelint/constants.py b/src/ansiblelint/constants.py index 6826565354..811d0317a7 100644 --- a/src/ansiblelint/constants.py +++ b/src/ansiblelint/constants.py @@ -89,6 +89,7 @@ def main(): "ansible-lint-config", "sanity-ignore-file", # tests/sanity/ignore file "plugin", + "galaxy", # galaxy.yml "", # unknown file type ] diff --git a/src/ansiblelint/rules/__init__.py b/src/ansiblelint/rules/__init__.py index c0469cfe0c..7fc6f589a8 100644 --- a/src/ansiblelint/rules/__init__.py +++ b/src/ansiblelint/rules/__init__.py @@ -76,15 +76,22 @@ def create_matcherror( self, message: str = "", lineno: int = 1, + column: int | None = None, details: str = "", filename: Lintable | None = None, tag: str = "", transform_meta: RuleMatchTransformMeta | None = None, + data: Any | None = None, ) -> MatchError: """Instantiate a new MatchError.""" + if data is not None and lineno == 1 and column is None: + lineno, column = ansiblelint.yaml_utils.get_line_column( + data, default_line=lineno + ) match = MatchError( message=message, lineno=lineno, + column=column, details=details, lintable=filename or Lintable(""), rule=copy.copy(self), diff --git a/src/ansiblelint/rules/args.py b/src/ansiblelint/rules/args.py index d2daacebe6..76d786c7f7 100644 --- a/src/ansiblelint/rules/args.py +++ b/src/ansiblelint/rules/args.py @@ -289,13 +289,26 @@ def test_args_module_fail(default_rules_collection: RulesCollection) -> None: results = Runner(success, rules=default_rules_collection).run() assert len(results) == 5 assert results[0].tag == "args[module]" - assert "missing required arguments" in results[0].message + # First part of regex is for ansible-core up to 2.18, second part is for ansible-core 2.19+ + assert re.match( + r"(missing required arguments|Unsupported parameters for \(basic.py\) module: foo)", + results[0].message, + ) assert results[1].tag == "args[module]" - assert "missing parameter(s) required by " in results[1].message + assert re.match( + r"(missing parameter\(s\) required by |Unsupported parameters for \(basic.py\) module: foo. Supported parameters include: fact_path)", + results[1].message, + ) assert results[2].tag == "args[module]" - assert "Unsupported parameters for" in results[2].message + assert re.match( + r"(Unsupported parameters for|missing parameter\(s\) required by 'enabled': name)", + results[2].message, + ) assert results[3].tag == "args[module]" - assert "Unsupported parameters for" in results[3].message + assert re.match( + r"(Unsupported parameters for|missing required arguments: repo)", + results[3].message, + ) assert results[4].tag == "args[module]" assert "value of state must be one of" in results[4].message diff --git a/src/ansiblelint/rules/complexity.py b/src/ansiblelint/rules/complexity.py index 5ef6cd5e83..820beccef8 100644 --- a/src/ansiblelint/rules/complexity.py +++ b/src/ansiblelint/rules/complexity.py @@ -6,7 +6,6 @@ import sys from typing import TYPE_CHECKING, Any -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.rules import AnsibleLintRule, RulesCollection if TYPE_CHECKING: @@ -40,9 +39,9 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: results.append( self.create_matcherror( message=f"Maximum tasks allowed in a play is {self._collection.options.max_tasks}.", - lineno=data[LINE_NUMBER_KEY], tag=f"{self.id}[play]", filename=file, + data=data, ), ) return results diff --git a/src/ansiblelint/rules/fqcn.py b/src/ansiblelint/rules/fqcn.py index 5a2c34bb8b..5107d542ab 100644 --- a/src/ansiblelint/rules/fqcn.py +++ b/src/ansiblelint/rules/fqcn.py @@ -8,7 +8,6 @@ from ruamel.yaml.comments import CommentedSeq -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.rules import AnsibleLintRule, TransformMixin from ansiblelint.utils import load_plugin @@ -159,7 +158,7 @@ def matchtask( message=message, details=details, filename=file, - lineno=task.line, + data=module, tag="fqcn[action-core]", ), ) @@ -169,7 +168,7 @@ def matchtask( message=f"Use FQCN for module actions, such `{self.module_aliases[module]}`.", details=f"Action `{module}` is not FQCN.", filename=file, - lineno=task.line, + data=module, tag="fqcn[action]", ), ) @@ -183,7 +182,7 @@ def matchtask( self.create_matcherror( message=f"You should use canonical module name `{self.module_aliases[module]}` instead of `{module}`.", filename=file, - lineno=task[LINE_NUMBER_KEY], + data=module, tag="fqcn[canonical]", ), ) @@ -219,7 +218,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: return [ self.create_matcherror( message="Avoid `collections` keyword by using FQCN for all plugins, modules, roles and playbooks.", - lineno=data.get(LINE_NUMBER_KEY, 1), + data=data, tag="fqcn[keyword]", filename=file, ), diff --git a/src/ansiblelint/rules/galaxy.py b/src/ansiblelint/rules/galaxy.py index 0a036271f9..0d1c775e85 100644 --- a/src/ansiblelint/rules/galaxy.py +++ b/src/ansiblelint/rules/galaxy.py @@ -40,7 +40,7 @@ class GalaxyRule(AnsibleLintRule): def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: """Return matches found for a specific play (entry in playbook).""" - if file.kind != "galaxy": # type: ignore[comparison-overlap] + if file.kind != "galaxy": return [] # Defined by Automation Hub Team and Partner Engineering @@ -161,7 +161,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: results.append( self.create_matcherror( message="galaxy.yaml should have version tag.", - lineno=data[LINE_NUMBER_KEY], + data=data, tag="galaxy[version-missing]", filename=file, ), diff --git a/src/ansiblelint/rules/galaxy_version_incorrect.py b/src/ansiblelint/rules/galaxy_version_incorrect.py index 6378503e0c..27af6ef524 100644 --- a/src/ansiblelint/rules/galaxy_version_incorrect.py +++ b/src/ansiblelint/rules/galaxy_version_incorrect.py @@ -24,16 +24,16 @@ class GalaxyVersionIncorrectRule(AnsibleLintRule): def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: """Return matches found for a specific play (entry in playbook).""" - if file.kind != "galaxy": # type: ignore[comparison-overlap] + if file.kind != "galaxy": return [] results = [] version = data.get("version") - if Version(version) < Version("1.0.0"): + if not version or Version(version) < Version("1.0.0"): results.append( self.create_matcherror( message="collection version should be greater than or equal to 1.0.0", - lineno=version._line_number, # noqa: SLF001 + data=version, filename=file, ), ) diff --git a/src/ansiblelint/rules/jinja.py b/src/ansiblelint/rules/jinja.py index 1e98b995a7..fd96508c41 100644 --- a/src/ansiblelint/rules/jinja.py +++ b/src/ansiblelint/rules/jinja.py @@ -6,6 +6,7 @@ import os import re import sys +from collections.abc import Mapping from dataclasses import dataclass from pathlib import Path from typing import TYPE_CHECKING, NamedTuple @@ -13,7 +14,6 @@ import black import jinja2 from ansible.errors import AnsibleError, AnsibleFilterError, AnsibleParserError -from ansible.parsing.yaml.objects import AnsibleUnicode from jinja2.exceptions import TemplateSyntaxError from ansiblelint.errors import RuleMatchTransformMeta @@ -22,6 +22,7 @@ from ansiblelint.runner import get_matches from ansiblelint.skip_utils import get_rule_skips_from_line from ansiblelint.text import has_jinja +from ansiblelint.types import AnsibleTemplateSyntaxError from ansiblelint.utils import ( # type: ignore[attr-defined] Templar, parse_yaml_from_file, @@ -187,14 +188,23 @@ def matchtask( elif re.match(r"^lookup plugin (.*) not found$", exc.message): # lookup plugin 'template' not found bypass = True + elif isinstance( + orig_exc, AnsibleTemplateSyntaxError + ) and re.match( + r"^Syntax error in template: No filter named '.*'.", + exc.message, + ): + bypass = True # AnsibleError: template error while templating string: expected token ':', got '}'. String: {{ {{ '1' }} }} # AnsibleError: template error while templating string: unable to locate collection ansible.netcommon. String: Foo {{ buildset_registry.host | ipwrap }} if not bypass: + lineno = task.get_error_line([*path, key]) result.append( self.create_matcherror( message=str(exc), - lineno=task.get_error_line(path), + lineno=lineno, + data=v, filename=file, tag=f"{self.id}[invalid]", ), @@ -206,6 +216,7 @@ def matchtask( lintable=file, ) if reformatted != v: + lineno = task.get_error_line([*path, key]) result.append( self.create_matcherror( message=self._msg( @@ -213,7 +224,8 @@ def matchtask( value=v, reformatted=reformatted, ), - lineno=task.get_error_line(path), + lineno=lineno, + data=v, details=details, filename=file, tag=f"{self.id}[{tag}]", @@ -237,10 +249,10 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: if str(file.kind) == "vars": data = parse_yaml_from_file(str(file.path)) - if not isinstance(data, dict): + if not isinstance(data, Mapping): return results for key, v, _path in nested_items_path(data): - if isinstance(v, AnsibleUnicode): + if isinstance(v, str): reformatted, details, tag = self.check_whitespace( v, key=key, @@ -254,7 +266,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: value=v, reformatted=reformatted, ), - lineno=v.ansible_pos[1], + data=v, details=details, filename=file, tag=f"{self.id}[{tag}]", @@ -506,31 +518,23 @@ def blacken(text: str) -> str: from ansiblelint.runner import Runner from ansiblelint.transformer import Transformer - @pytest.fixture(name="error_expected_lines") - def fixture_error_expected_lines() -> list[int]: - """Return list of expected error lines.""" - return [33, 36, 39, 42, 45, 48, 74] - - # 21 68 - @pytest.fixture(name="lint_error_lines") - def fixture_lint_error_lines() -> list[int]: - """Get VarHasSpacesRules linting results on test_playbook.""" + def test_jinja_spacing_playbook() -> None: + """Ensure that expected error lines are matching found linting error lines.""" + # list unexpected error lines or non-matching error lines + lineno_list = [33, 36, 39, 42, 45, 48, 74] + lintable = Lintable("examples/playbooks/jinja-spacing.yml") collection = RulesCollection() collection.register(JinjaRule()) - lintable = Lintable("examples/playbooks/jinja-spacing.yml") results = Runner(lintable, rules=collection).run() - return [item.lineno for item in results] + assert len(results) == len(lineno_list) + for index, result in enumerate(results): + assert result.tag == "jinja[spacing]" + assert result.lineno == lineno_list[index] - def test_jinja_spacing_playbook( - error_expected_lines: list[int], - lint_error_lines: list[int], - ) -> None: - """Ensure that expected error lines are matching found linting error lines.""" - # list unexpected error lines or non-matching error lines - error_lines_difference = list( - set(error_expected_lines).symmetric_difference(set(lint_error_lines)), - ) - assert len(error_lines_difference) == 0 + # error_lines_difference = list( + # set(error_expected_lines).symmetric_difference(set(lint_error_lines)), + # ) + # assert len(error_lines_difference) == 0 def test_jinja_spacing_vars() -> None: """Ensure that expected error details are matching found linting error details.""" @@ -836,7 +840,7 @@ def test_jinja_invalid() -> None: assert errs[0].lineno == 9 assert errs[1].tag == "jinja[invalid]" assert errs[1].rule.id == "jinja" - assert errs[1].lineno == 9 + assert errs[1].lineno in [9, 10] # 2.19 has better line identification def test_jinja_valid() -> None: """Tests our ability to parse jinja, even when variables may not be defined.""" diff --git a/src/ansiblelint/rules/key_order.py b/src/ansiblelint/rules/key_order.py index 70afe8e7fb..965496b7dc 100644 --- a/src/ansiblelint/rules/key_order.py +++ b/src/ansiblelint/rules/key_order.py @@ -7,7 +7,7 @@ from dataclasses import dataclass from typing import TYPE_CHECKING, Any -from ansiblelint.constants import ANNOTATION_KEYS, LINE_NUMBER_KEY +from ansiblelint.constants import ANNOTATION_KEYS from ansiblelint.errors import MatchError, RuleMatchTransformMeta from ansiblelint.rules import AnsibleLintRule, TransformMixin @@ -90,8 +90,8 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: f"You can improve the play key order to: {', '.join(sorted_keys)}", filename=file, tag=f"{self.id}[play]", - lineno=data[LINE_NUMBER_KEY], transform_meta=KeyOrderTMeta(fixed=tuple(sorted_keys)), + data=data, ), ) return result diff --git a/src/ansiblelint/rules/loop_var_prefix.py b/src/ansiblelint/rules/loop_var_prefix.py index 2fbe3c2359..729ba9b0a2 100644 --- a/src/ansiblelint/rules/loop_var_prefix.py +++ b/src/ansiblelint/rules/loop_var_prefix.py @@ -49,10 +49,16 @@ def matchtask( self.prefix = re.compile( options.loop_var_prefix.format(role=toidentifier(file.role)), ) - has_loop = "loop" in task.raw_task - for key in task.raw_task: - if key.startswith("with_"): - has_loop = True + has_loop = False + if "loop" in task.raw_task: + data = task.raw_task["loop"] + has_loop = True + else: + for key in task.raw_task: + if key.startswith("with_"): + data = key + has_loop = True + break if has_loop: loop_control = task.raw_task.get("loop_control", {}) @@ -64,6 +70,7 @@ def matchtask( self.create_matcherror( message=f"Loop variable name does not match /{options.loop_var_prefix}/ regex, where role={toidentifier(file.role)}.", filename=file, + data=loop_var, tag="loop-var-prefix[wrong]", ), ] @@ -72,6 +79,7 @@ def matchtask( self.create_matcherror( message=f"Replace unsafe implicit `item` loop variable by adding a `loop_var` that is matching /{options.loop_var_prefix}/ regex.", filename=file, + data=data, tag="loop-var-prefix[missing]", ), ] diff --git a/src/ansiblelint/rules/meta_incorrect.py b/src/ansiblelint/rules/meta_incorrect.py index aa0fb37f35..511c27d9d4 100644 --- a/src/ansiblelint/rules/meta_incorrect.py +++ b/src/ansiblelint/rules/meta_incorrect.py @@ -6,7 +6,6 @@ import sys from typing import TYPE_CHECKING -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.rules import AnsibleLintRule if TYPE_CHECKING: @@ -48,8 +47,8 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: results.append( self.create_matcherror( filename=file, - lineno=file.data[LINE_NUMBER_KEY], message=f"Should change default metadata: {field}", + data=file.data, ), ) diff --git a/src/ansiblelint/rules/meta_video_links.py b/src/ansiblelint/rules/meta_video_links.py index a25d568673..37f305576c 100644 --- a/src/ansiblelint/rules/meta_video_links.py +++ b/src/ansiblelint/rules/meta_video_links.py @@ -56,16 +56,24 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: self.create_matcherror( "Expected item in 'video_links' to be a dictionary", filename=file, + data=video, ), ) continue - if set(video) != {"url", "title", FILENAME_KEY, LINE_NUMBER_KEY}: + unexpected_keys = set(video) - { + "url", + "title", + FILENAME_KEY, + LINE_NUMBER_KEY, + } + if unexpected_keys: results.append( self.create_matcherror( "Expected item in 'video_links' to contain " "only keys 'url' and 'title'", filename=file, + data=video, ), ) continue @@ -79,7 +87,9 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: "Expected it be a shared link from Vimeo, YouTube, " "or Google Drive." ) - results.append(self.create_matcherror(msg, filename=file)) + results.append( + self.create_matcherror(msg, filename=file, data=video["url"]) + ) return results @@ -97,10 +107,10 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: pytest.param( "examples/roles/meta_video_links_fail/meta/main.yml", ( + "URL format 'https://www.youtube.com/watch?v=aWmRepTSFKs&feature=youtu.be' is not recognized. Expected it be a shared link from Vimeo, YouTube, or Google Drive.", "Expected item in 'video_links' to be a dictionary", "Expected item in 'video_links' to contain only keys 'url' and 'title'", - "URL format 'https://www.youtube.com/watch?v=aWmRepTSFKs&feature=youtu.be' is not recognized. Expected it be a shared link from Vimeo, YouTube, or Google Drive.", - "URL format 'www.acme.com/vid' is not recognized", + "URL format 'www.acme.com/vid' is not recognized. Expected it be a shared link from Vimeo, YouTube, or Google Drive.", ), id="1", ), @@ -119,6 +129,8 @@ def test_video_links( """Test rule matches.""" results = Runner(test_file, rules=default_rules_collection).run() assert len(results) == len(failures) - for index, result in enumerate(results): + # order of results is different between pre/post 2.19 due to increased + # line precision in 2.19 + for result in results: assert result.tag == "meta-video-links" - assert failures[index] in result.message + assert result.message in failures diff --git a/src/ansiblelint/rules/name.py b/src/ansiblelint/rules/name.py index 1cd0a25835..46d0a602c1 100644 --- a/src/ansiblelint/rules/name.py +++ b/src/ansiblelint/rules/name.py @@ -10,7 +10,6 @@ import wcmatch.pathlib import wcmatch.wcmatch -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.file_utils import Lintable from ansiblelint.rules import AnsibleLintRule, TransformMixin @@ -53,16 +52,16 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: return [ self.create_matcherror( message="All plays should be named.", - lineno=data.get(LINE_NUMBER_KEY, 1), tag="name[play]", filename=file, + data=data, ), ] results.extend( self._check_name( data["name"], lintable=file, - lineno=data.get(LINE_NUMBER_KEY, 1), + data=data, ), ) return results @@ -112,16 +111,13 @@ def _prefix_check( self._check_name( effective_name, lintable=lintable, - lineno=lineno, + data=lintable.data, ), ) return results def _check_name( - self, - name: str, - lintable: Lintable | None, - lineno: int, + self, name: str, lintable: Lintable | None, data: Any ) -> list[MatchError]: # This rules applies only to languages that do have uppercase and # lowercase letter, so we ignore anything else. On Unicode isupper() @@ -147,7 +143,7 @@ def _check_name( results.append( self.create_matcherror( message=f"Task name should start with '{prefix}'.", - lineno=lineno, + data=data, tag="name[prefix]", filename=lintable, ), @@ -164,7 +160,7 @@ def _check_name( results.append( self.create_matcherror( message="All names should start with an uppercase letter.", - lineno=lineno, + data=name, tag="name[casing]", filename=lintable, ), @@ -173,7 +169,7 @@ def _check_name( results.append( self.create_matcherror( message="Jinja templates should only be at the end of 'name'", - lineno=lineno, + data=name, tag="name[template]", filename=lintable, ), diff --git a/src/ansiblelint/rules/no_handler.py b/src/ansiblelint/rules/no_handler.py index 0c01b96234..49be72ff9a 100644 --- a/src/ansiblelint/rules/no_handler.py +++ b/src/ansiblelint/rules/no_handler.py @@ -23,11 +23,13 @@ from __future__ import annotations import sys +from collections.abc import Sequence from typing import TYPE_CHECKING from ansiblelint.rules import AnsibleLintRule if TYPE_CHECKING: + from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -69,18 +71,26 @@ def matchtask( self, task: Task, file: Lintable | None = None, - ) -> bool | str: - if task["__ansible_action_type__"] != "task" or task.is_handler(): - return False + ) -> list[MatchError]: + if task.is_handler(): + return [] when = task.get("when") - result = False - - if isinstance(when, list): - if len(when) <= 1: - result = _changed_in_when(when[0]) - elif isinstance(when, str): - result = _changed_in_when(when) + result = [] + + if (isinstance(when, str) and _changed_in_when(when)) or ( + isinstance(when, Sequence) + and not isinstance(when, str) + and len(when) <= 1 + and _changed_in_when(when[0]) + ): + result.append( + self.create_matcherror( + message=self.shortdesc, + data=when, + filename=file, + ) + ) return result @@ -114,5 +124,5 @@ def test_role_with_handler() -> None: """Test role with handler.""" role_path = "examples/roles/role_with_handler" - results = run_ansible_lint("-v", role_path) + results = run_ansible_lint("-v", role_path, env={"NO_COLOR": "1"}) assert "no-handler" not in results.stdout diff --git a/src/ansiblelint/rules/no_jinja_when.py b/src/ansiblelint/rules/no_jinja_when.py index b036b5185a..02149ec3b2 100644 --- a/src/ansiblelint/rules/no_jinja_when.py +++ b/src/ansiblelint/rules/no_jinja_when.py @@ -7,7 +7,6 @@ from collections.abc import MutableMapping from typing import TYPE_CHECKING, Any -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.rules import AnsibleLintRule, TransformMixin if TYPE_CHECKING: @@ -55,7 +54,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: self.create_matcherror( details=str({"when": role}), filename=file, - lineno=role[LINE_NUMBER_KEY], + data=role, ) for role in data["roles"] if ( diff --git a/src/ansiblelint/rules/no_prompting.py b/src/ansiblelint/rules/no_prompting.py index b72d698e0d..a67daaed99 100644 --- a/src/ansiblelint/rules/no_prompting.py +++ b/src/ansiblelint/rules/no_prompting.py @@ -5,7 +5,6 @@ import sys from typing import TYPE_CHECKING, Any -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.rules import AnsibleLintRule if TYPE_CHECKING: @@ -40,7 +39,7 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: return [ self.create_matcherror( message="Play uses vars_prompt", - lineno=vars_prompt[0][LINE_NUMBER_KEY], + data=vars_prompt[0], filename=file, ), ] diff --git a/src/ansiblelint/rules/no_tabs.py b/src/ansiblelint/rules/no_tabs.py index d6ad3e77db..74c1d59d51 100644 --- a/src/ansiblelint/rules/no_tabs.py +++ b/src/ansiblelint/rules/no_tabs.py @@ -12,6 +12,7 @@ from ansiblelint.yaml_utils import nested_items_path if TYPE_CHECKING: + from ansiblelint.errors import MatchError from ansiblelint.file_utils import Lintable from ansiblelint.utils import Task @@ -51,19 +52,32 @@ def matchtask( self, task: Task, file: Lintable | None = None, - ) -> bool | str: + ) -> list[MatchError]: + result = [] action = task["action"]["__ansible_module__"] for k, v, _ in nested_items_path(task): if isinstance(k, str) and "\t" in k and not has_jinja(k): - return True + result.append( + self.create_matcherror( + message=self.shortdesc, + data=k, + filename=file, + ) + ) if ( isinstance(v, str) and "\t" in v and (action, k) not in self.allow_list and not has_jinja(v) ): - return True - return False + result.append( + self.create_matcherror( + message=self.shortdesc, + data=v, + filename=file, + ) + ) + return result # testing code to be loaded only with pytest or when executed the rule file @@ -77,12 +91,11 @@ def test_no_tabs_rule(default_rules_collection: RulesCollection) -> None: "examples/playbooks/rule-no-tabs.yml", rules=default_rules_collection, ).run() - expected_results = [ - (10, NoTabsRule().shortdesc), - (13, NoTabsRule().shortdesc), - ] - for i, expected in enumerate(expected_results): - assert len(results) >= i + 1 - assert results[i].lineno == expected[0] - assert results[i].message == expected[1] - assert len(results) == len(expected_results), results + lines = [] + for result in results: + assert result.rule.id == "no-tabs" + lines.append(result.lineno) + assert lines + # 2.19 has more precise line:columns numbers so the effective result + # is different. + assert lines == [10, 13] or lines == [12, 15, 15], lines diff --git a/src/ansiblelint/rules/partial_become.py b/src/ansiblelint/rules/partial_become.py index 1f3daf1107..42d5a4a356 100644 --- a/src/ansiblelint/rules/partial_become.py +++ b/src/ansiblelint/rules/partial_become.py @@ -26,7 +26,6 @@ from ruamel.yaml.comments import CommentedMap, CommentedSeq -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.rules import AnsibleLintRule, TransformMixin if TYPE_CHECKING: @@ -66,7 +65,7 @@ def matchplay( message=self.shortdesc, filename=file, tag=f"{self.id}[play]", - lineno=data[LINE_NUMBER_KEY], + data=data, ) errors.append(error) return errors @@ -90,7 +89,7 @@ def matchtask( message=self.shortdesc, filename=file, tag=f"{self.id}[task]", - lineno=task[LINE_NUMBER_KEY], + lineno=task.line, ) errors.append(error) return errors diff --git a/src/ansiblelint/rules/role_name.py b/src/ansiblelint/rules/role_name.py index f6ae09fd40..c349db1e7c 100644 --- a/src/ansiblelint/rules/role_name.py +++ b/src/ansiblelint/rules/role_name.py @@ -27,9 +27,10 @@ from functools import cache from typing import TYPE_CHECKING -from ansiblelint.constants import LINE_NUMBER_KEY, ROLE_IMPORT_ACTION_NAMES +from ansiblelint.constants import ROLE_IMPORT_ACTION_NAMES from ansiblelint.rules import AnsibleLintRule from ansiblelint.utils import parse_yaml_from_file +from ansiblelint.yaml_utils import get_line_column if TYPE_CHECKING: from pathlib import Path @@ -91,6 +92,7 @@ def matchdir(self, lintable: Lintable) -> list[MatchError]: def matchyaml(self, file: Lintable) -> list[MatchError]: result: list[MatchError] = [] + column: int | None = None if file.kind not in ("meta", "role", "playbook"): return result @@ -105,17 +107,11 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: msg = "Role dependency has unexpected type." raise TypeError(msg) if "/" in role_name: - lineno = 1 - if hasattr(role_name, "ansible_pos"): - lineno = role_name.ansible_pos[ # pyright: ignore[reportAttributeAccessIssue] - 1 - ] - result.append( self.create_matcherror( f"Avoid using paths when importing roles. ({role_name})", filename=file, - lineno=lineno, + data=role_name, tag=f"{self.id}[path]", ), ) @@ -124,11 +120,11 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: if file.kind == "playbook": for play in file.data: if "roles" in play: - line = play[LINE_NUMBER_KEY] + line, column = get_line_column(play) for role in play["roles"]: role_name = None if isinstance(role, dict): - line = role[LINE_NUMBER_KEY] + line, column = get_line_column(role) role_name = role["role"] elif isinstance(role, str): role_name = role @@ -141,6 +137,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: f"Avoid using paths when importing roles. ({role_name})", filename=file, lineno=line, + column=column, tag=f"{self.id}[path]", ), ) diff --git a/src/ansiblelint/rules/schema.py b/src/ansiblelint/rules/schema.py index 65d8d8f504..bc5d4d5573 100644 --- a/src/ansiblelint/rules/schema.py +++ b/src/ansiblelint/rules/schema.py @@ -7,7 +7,6 @@ import sys from typing import TYPE_CHECKING, Any -from ansiblelint.constants import LINE_NUMBER_KEY from ansiblelint.file_utils import Lintable from ansiblelint.rules import AnsibleLintRule from ansiblelint.schemas.__main__ import JSON_SCHEMAS @@ -126,7 +125,7 @@ def _get_field_matches( results.append( self.create_matcherror( message=msg, - lineno=data.get(LINE_NUMBER_KEY, 1), + data=plugin_value, filename=file, details=ValidateSchemaRule.description, tag=f"schema[{kind}]", diff --git a/src/ansiblelint/rules/syntax_check.py b/src/ansiblelint/rules/syntax_check.py index 6d07ca61ff..e282c69a78 100644 --- a/src/ansiblelint/rules/syntax_check.py +++ b/src/ansiblelint/rules/syntax_check.py @@ -6,6 +6,7 @@ from dataclasses import dataclass from ansiblelint.rules import AnsibleLintRule +from ansiblelint.types import ansible_error_format @dataclass @@ -16,6 +17,18 @@ class KnownError: regex: re.Pattern[str] +if ansible_error_format == 1: + _ansible_error_prefix = "ERROR! " + _ansible_error_detail = r"\n\nThe error appears to be in '(?P[\w\/\.\-]+)': line (?P\d+), column (?P\d+)" +elif ansible_error_format == 2: # 2.19 with data tagging + _ansible_error_prefix = r"\[ERROR\]: " + _ansible_error_detail = ( + r"\nOrigin: (?P[\w\/\.\-]+):(?P\d+):(?P\d+)" + ) +else: # pragma: no cover + msg = f"Unsupported ansible_error_format: {ansible_error_format}" + raise NotImplementedError(msg) + # Order matters, we only report the first matching pattern, the one at the end # is used to match generic or less specific patterns. OUTPUT_PATTERNS = ( @@ -30,7 +43,7 @@ class KnownError: KnownError( tag="no-file", regex=re.compile( - r"^ERROR! (?PNo file specified for [^\n]*)", + rf"^{_ansible_error_prefix}(?P<title>No file specified for [^\n]*){_ansible_error_detail}", re.MULTILINE | re.DOTALL | re.DOTALL, ), ), @@ -51,14 +64,14 @@ class KnownError: KnownError( tag="unknown-module", regex=re.compile( - r"^ERROR! (?P<title>couldn't resolve module/action [^\n]*)\n\nThe error appears to be in '(?P<filename>[\w\/\.\-]+)': line (?P<line>\d+), column (?P<column>\d+)", + rf"^{_ansible_error_prefix}(?P<title>couldn't resolve module/action [^\n]*){_ansible_error_detail}", re.MULTILINE | re.DOTALL | re.DOTALL, ), ), KnownError( tag="specific", regex=re.compile( - r"^ERROR! (?P<title>[^\n]*)\n\nThe error appears to be in '(?P<filename>[\w\/\.\-]+)': line (?P<line>\d+), column (?P<column>\d+)", + rf"^{_ansible_error_prefix}(?P<title>[^\n]*){_ansible_error_detail}", re.MULTILINE | re.DOTALL | re.DOTALL, ), ), @@ -66,10 +79,20 @@ class KnownError: KnownError( tag="specific", regex=re.compile( - r"^ERROR! (?P<title>the role '.*' was not found in[^\n]*)'(?P<filename>[\w\/\.\-]+)': line (?P<line>\d+), column (?P<column>\d+)", + rf"^{_ansible_error_prefix}(?P<title>the role '.*' was not found in[^\n]*){_ansible_error_detail}", re.MULTILINE | re.DOTALL | re.DOTALL, ), ), + # 2.19: + # [ERROR]: no module/action detected in task. + # Origin: /Users/ssbarnea/code/a/ansible-lint/examples/roles/invalid_due_syntax/tasks/main.yml:2:3 + # KnownError( + # tag="specific", + # regex=re.compile( + # r"^\[ERROR\]: (?P<title>[^\n]*)\nOrigin: (?P<filename>[\w\/\.\-]+):(?P<line>\d+):(?P<column>\d+)", + # re.MULTILINE | re.DOTALL | re.DOTALL, + # ), + # ), ) diff --git a/src/ansiblelint/rules/var_naming.py b/src/ansiblelint/rules/var_naming.py index a16bdd887e..c738861194 100644 --- a/src/ansiblelint/rules/var_naming.py +++ b/src/ansiblelint/rules/var_naming.py @@ -7,13 +7,11 @@ import sys from typing import TYPE_CHECKING, Any, NamedTuple -from ansible.parsing.yaml.objects import AnsibleUnicode from ansible.vars.reserved import get_reserved_names from ansiblelint.config import Options, options from ansiblelint.constants import ( ANNOTATION_KEYS, - LINE_NUMBER_KEY, PLAYBOOK_ROLE_KEYWORDS, RC, ) @@ -137,6 +135,7 @@ def get_var_naming_matcherror( tag="var-naming[non-ascii]", message=f"Variables names must be ASCII. ({ident})", filename=file, + data=ident, ) if keyword.iskeyword(ident): @@ -144,6 +143,7 @@ def get_var_naming_matcherror( tag="var-naming[no-keyword]", message=f"Variables names must not be Python keywords. ({ident})", filename=file, + data=ident, ) if ident in self.reserved_names: @@ -151,6 +151,7 @@ def get_var_naming_matcherror( tag="var-naming[no-reserved]", message=f"Variables names must not be Ansible reserved names. ({ident})", filename=file, + data=ident, ) if ident in self.read_only_names: @@ -158,6 +159,7 @@ def get_var_naming_matcherror( tag="var-naming[read-only]", message=f"This special variable is read-only. ({ident})", filename=file, + data=ident, ) # We want to allow use of jinja2 templating for variable names @@ -171,6 +173,7 @@ def get_var_naming_matcherror( tag="var-naming[pattern]", message=f"Variables names should match {self.re_pattern_str} regex. ({ident})", filename=file, + data=ident, ) if ( @@ -183,6 +186,7 @@ def get_var_naming_matcherror( tag="var-naming[no-role-prefix]", message=f"Variables names from within roles should use {prefix.value}_ as a prefix.", filename=file, + data=ident, ) return None @@ -198,32 +202,20 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: for key in our_vars: match_error = self.get_var_naming_matcherror(key, file=file) if match_error: - match_error.lineno = ( - key.ansible_pos[1] - if isinstance(key, AnsibleUnicode) - else our_vars[LINE_NUMBER_KEY] - ) raw_results.append(match_error) roles = data.get("roles", []) for role in roles: - if isinstance(role, AnsibleUnicode): + if isinstance(role, str): continue role_fqcn = role.get("role", role.get("name")) prefix = self._parse_prefix(role_fqcn) for key in list(role.keys()): if key not in PLAYBOOK_ROLE_KEYWORDS: match_error = self.get_var_naming_matcherror( - key, - prefix=prefix, - file=file, + key, prefix=prefix, file=file ) if match_error: match_error.message += f" (vars: {key})" - match_error.lineno = ( - key.ansible_pos[1] - if isinstance(key, AnsibleUnicode) - else role[LINE_NUMBER_KEY] - ) raw_results.append(match_error) our_vars = role.get("vars", {}) @@ -235,11 +227,6 @@ def matchplay(self, file: Lintable, data: dict[str, Any]) -> list[MatchError]: ) if match_error: match_error.message += f" (vars: {key})" - match_error.lineno = ( - key.ansible_pos[1] - if isinstance(key, AnsibleUnicode) - else our_vars[LINE_NUMBER_KEY] - ) raw_results.append(match_error) if raw_results: lines = file.content.splitlines() @@ -281,7 +268,6 @@ def matchtask( file=file or Lintable(""), ) if match_error: - match_error.lineno = our_vars[LINE_NUMBER_KEY] match_error.message += f" (vars: {key})" results.append(match_error) @@ -336,7 +322,6 @@ def matchyaml(self, file: Lintable) -> list[MatchError]: file=file, ) if match_error: - match_error.lineno = key.ansible_pos[1] match_error.message += f" (vars: {key})" raw_results.append(match_error) if raw_results: diff --git a/src/ansiblelint/types.py b/src/ansiblelint/types.py index ea1da22fec..a1bff42f7f 100644 --- a/src/ansiblelint/types.py +++ b/src/ansiblelint/types.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Mapping, Sequence from typing import TypeAlias from ansible.parsing.yaml.objects import ( # pyright: ignore[reportMissingImports] @@ -18,17 +19,40 @@ from ansible.parsing.yaml.objects import ( # pyright: ignore[reportMissingImports] AnsibleBaseYAMLObject, # pyright: ignore[reportRedeclaration] ) + + TrustedAsTemplate = None + + class AnsibleTemplateSyntaxError: + """Fake class introduced in 2.19.""" + + ansible_error_format = 1 # core 2.19 + data tagging: except ImportError: # pragma: no cover + # cspell: ignore datatag + from ansible._internal._datatag._tags import ( # type: ignore[import-not-found,no-redef] + TrustedAsTemplate, + ) from ansible._internal._yaml._constructor import ( # type: ignore[import-not-found,no-redef] # pyright: ignore[reportMissingImports] # pylint: disable=import-error,no-name-in-module AnsibleConstructor, ) + from ansible.errors import ( # type: ignore[no-redef,attr-defined,unused-ignore] + AnsibleTemplateSyntaxError, # pyright: ignore[reportAttributeAccessIssue] + ) AnsibleBaseYAMLObject: TypeAlias = ( # type: ignore[no-redef] # pyright: ignore[reportRedeclaration] - AnsibleSequence | AnsibleMapping | AnsibleUnicode | str | None + AnsibleSequence + | AnsibleMapping + | AnsibleUnicode + | str + | Mapping + | Sequence + | None ) - -AnsibleJSON: TypeAlias = AnsibleSequence | AnsibleMapping | AnsibleUnicode | str | None + ansible_error_format = 2 +# temporary ignoring the type parameters for Sequence and Mapping because once +# add them we can no longer use isinstance() to check for them and we will +# need to implement a more complex runtime type checking. +AnsibleJSON: TypeAlias = Sequence | Mapping | AnsibleUnicode | str | None # type: ignore[type-arg] __all__ = [ "AnsibleBaseYAMLObject", @@ -36,6 +60,9 @@ "AnsibleJSON", "AnsibleMapping", "AnsibleSequence", + "AnsibleTemplateSyntaxError", "AnsibleUnicode", "AnsibleVaultEncryptedUnicode", + "TrustedAsTemplate", + "ansible_error_format", ] diff --git a/src/ansiblelint/utils.py b/src/ansiblelint/utils.py index e0e6023fcb..af01827e5e 100644 --- a/src/ansiblelint/utils.py +++ b/src/ansiblelint/utils.py @@ -94,6 +94,7 @@ AnsibleJSON, AnsibleMapping, AnsibleSequence, + TrustedAsTemplate, ) if TYPE_CHECKING: @@ -203,6 +204,8 @@ def ansible_template( kwargs["disable_lookups"] = True for _i in range(10): try: + if TrustedAsTemplate and not isinstance(varname, TrustedAsTemplate): + varname = TrustedAsTemplate().tag(varname) templated = templar.template(varname, **kwargs) # type: ignore[no-untyped-call] except AnsibleError as exc: if lookup_error in exc.message: @@ -468,10 +471,14 @@ def roles_children( if isinstance(role, dict): if "role" in role or "name" in role: if "tags" not in role or "skip_ansible_lint" not in role["tags"]: + role_name = role.get("role", role.get("name")) + if not isinstance(role_name, str): # pragma: no cover + msg = "Role name is not a string." + raise TypeError(msg) results.extend( self._look_for_role_files( basedir, - role.get("role", role.get("name")), + role_name, ), ) elif k != "dependencies": @@ -661,10 +668,25 @@ def _sanitize_task(task: MutableMapping[str, Any]) -> MutableMapping[str, Any]: result = copy.deepcopy(task) # task is an AnsibleMapping which inherits from OrderedDict, so we need # to use `del` to remove unwanted keys. - for k in [SKIPPED_RULES_KEY, FILENAME_KEY, LINE_NUMBER_KEY]: - if k in result: - del result[k] - return result + + def remove_keys(obj: MutableMapping[str, Any]) -> MutableMapping[str, Any]: + """Recursively removes specified keys from a nested dictionary or list. + + :param obj: The input dictionary or list to process. + :param forbidden_keys: List of keys to remove from dictionaries. + :return: A new object with forbidden keys removed. + """ + if isinstance(obj, MutableMapping): + for key in [SKIPPED_RULES_KEY, FILENAME_KEY, LINE_NUMBER_KEY]: + if key in obj: + del obj[key] + for value in obj.values(): + if isinstance(value, MutableMapping): + remove_keys(value) + + return obj # Base case: return non-dict, non-list values unchanged + + return remove_keys(result) def _extract_ansible_parsed_keys_from_task( @@ -744,6 +766,13 @@ def normalize_task_v2(task: Task) -> dict[str, Any]: "__ansible_module__": action, "__ansible_module_original__": action_unnormalized, } + # Inject back original line number information into the task + if ( + action_unnormalized in task.raw_task + and isinstance(task.raw_task[action_unnormalized], Mapping) + and "__line__" in task.raw_task[action_unnormalized] + ): + result["action"]["__line__"] = task.raw_task[action_unnormalized]["__line__"] result["action"].update(arguments) return result @@ -808,6 +837,7 @@ class Task(Mapping[str, Any]): ) error: MatchError | None = None position: str = "" + kind: str = "tasks" def __post_init__(self) -> None: """Ensures that the task is valid.""" @@ -887,13 +917,7 @@ def skip_tags(self) -> list[str]: def is_handler(self) -> bool: """Return true for tasks that are handlers.""" - is_handler_file = False - if isinstance(self._normalized_task, dict): - file_name = str(self._normalized_task["action"].get(FILENAME_KEY, None)) - if file_name: - paths = file_name.split("/") - is_handler_file = "handlers" in paths - return is_handler_file or ".handlers[" in self.position + return self.kind == "handlers" def __str__(self) -> str: """Return a string representation of the task.""" @@ -949,7 +973,9 @@ def __iter__(self) -> Iterator[str]: def line(self) -> int: """Return the line number of the task.""" result: int = 0 - result = int(self.get(LINE_NUMBER_KEY, 0)) + if "get_line_column" not in globals(): + from ansiblelint.yaml_utils import get_line_column + result, _ = get_line_column(self.raw_task) # pylint: disable=possibly-used-before-assignment if not result: # pragma: no cover x = self.get("action", {}) result = int(x.get(LINE_NUMBER_KEY, 0)) @@ -957,8 +983,10 @@ def line(self) -> int: def get_error_line(self, path: list[str | int]) -> int: """Return error line number.""" - ctx = self.normalized_task - line = self.normalized_task[LINE_NUMBER_KEY] + ctx: Mapping[Any, Any] = self.normalized_task + line = 1 + if LINE_NUMBER_KEY in self.normalized_task: + line = self.normalized_task[LINE_NUMBER_KEY] for _ in path: if ( isinstance(ctx, collections.abc.Container) and _ in ctx @@ -966,7 +994,7 @@ def get_error_line(self, path: list[str | int]) -> int: value = ctx.get( # pyright: ignore[reportAttributeAccessIssue] _ # pyright: ignore[reportArgumentType] ) - if isinstance(value, dict): + if isinstance(value, Mapping): ctx = value if ( isinstance(ctx, collections.abc.Container) @@ -988,7 +1016,7 @@ def task_in_list( """Get action tasks from block structures.""" def each_entry( - data: AnsibleSequence | AnsibleMapping, file: Lintable, position: str + data: Sequence[Any] | AnsibleMapping, file: Lintable, kind: str, position: str ) -> Iterator[Task]: if not data or not isinstance(data, Iterable): return @@ -1000,6 +1028,7 @@ def each_entry( yield Task( entry, filename=file.filename, + kind=kind, position=pos_, ) for block in [k for k in entry if k in NESTED_TASK_KEYS]: @@ -1008,30 +1037,31 @@ def each_entry( yield from task_in_list( data=v, file=file, - kind="tasks", + kind=kind, position=f"{pos_}.{block}", ) - if not isinstance(data, list): + if not isinstance(data, Sequence): return if kind == "playbook": attributes = ["tasks", "pre_tasks", "post_tasks", "handlers"] for item_index, item in enumerate(data): for attribute in attributes: - if not isinstance(item, dict): + if not isinstance(item, Mapping): continue if attribute in item: - if isinstance(item[attribute], list): + if isinstance(item[attribute], Sequence): yield from each_entry( item[attribute], file=file, + kind="tasks" if "tasks" in attribute else "handlers", position=f"{position}[{item_index}].{attribute}", ) elif item[attribute] is not None: # pragma: no cover msg = f"Key '{attribute}' defined, but bad value: '{item[attribute]!s}'" raise RuntimeError(msg) - elif isinstance(data, AnsibleSequence): - yield from each_entry(data, file=file, position=position) + elif isinstance(data, Sequence): + yield from each_entry(data, file=file, position=position, kind=kind) def add_action_type( @@ -1189,11 +1219,14 @@ def is_playbook(filename: str) -> bool: exc, ) else: - if ( - isinstance(f, AnsibleSequence) - and hasattr(next(iter(f), {}), "keys") - and playbooks_keys.intersection(next(iter(f), {}).keys()) - ): + # A playbook is a sequence of dictionaries that contain at least one + # of the playbooks_keys each. + if isinstance(f, Sequence): + for item in f: + if not isinstance(item, Mapping) or not playbooks_keys.intersection( + item.keys() + ): + return False return True return False diff --git a/src/ansiblelint/yaml_utils.py b/src/ansiblelint/yaml_utils.py index 6bb928c678..463642df95 100644 --- a/src/ansiblelint/yaml_utils.py +++ b/src/ansiblelint/yaml_utils.py @@ -28,11 +28,20 @@ from ansiblelint.constants import ( ANNOTATION_KEYS, + LINE_NUMBER_KEY, NESTED_TASK_KEYS, PLAYBOOK_TASK_KEYWORDS, ) from ansiblelint.utils import Task +try: # ansible 2.19 + data tagging + # cspell: ignore datatag + from ansible._internal._datatag._tags import ( # type: ignore[import-not-found] # pyright: ignore[reportMissingImports] + Origin, + ) +except ImportError: # pragma: no cover + Origin = None + if TYPE_CHECKING: # noinspection PyProtectedMember from collections.abc import Callable, Iterator, Sequence @@ -1266,3 +1275,27 @@ def clean_json( # neither a dict nor a list, do nothing pass return obj + + +def get_line_column(data: object, default_line: int = 1) -> tuple[int, int | None]: + """Return the line and column of the given data. + + Args: + data: Object for which to introspect line number. + default_line: fallback default line number to return if no line number is found. + """ + line = 0 + column = None + if isinstance(data, Mapping) and LINE_NUMBER_KEY in data: + line = int(data[LINE_NUMBER_KEY]) + if not line: + # ansible 2.19+ + if Origin: # pragma: no cover + tag = Origin.get_tag(data) + line = tag.line_num + column = tag.col_num + else: # pre-ansible 2.19 + if hasattr(data, "ansible_pos"): # AnsibleUnicode object + _, line, column = data.ansible_pos # pyright: ignore[reportAttributeAccessIssue] + + return (line or default_line, column) diff --git a/test/rules/test_syntax_check.py b/test/rules/test_syntax_check.py index 8b2e527516..b8a7cfb71a 100644 --- a/test/rules/test_syntax_check.py +++ b/test/rules/test_syntax_check.py @@ -17,7 +17,7 @@ [ ( "syntax-check[specific]", - 4, + [4], 7, "conflicting action statements: ansible.builtin.debug, ansible.builtin.command", ), @@ -29,7 +29,7 @@ [ ( "syntax-check[specific]", - 5, + [5, 6], 7, "'include_role' is not a valid attribute for a Block", ), @@ -41,7 +41,7 @@ def test_get_ansible_syntax_check_matches( default_rules_collection: RulesCollection, filename: str, - expected_results: list[tuple[str, int, int, str]], + expected_results: list[tuple[str, list[int], int, str]], ) -> None: """Validate parsing of ansible output.""" lintable = Lintable( @@ -54,7 +54,7 @@ def test_get_ansible_syntax_check_matches( assert len(result) == len(expected_results) for index, expected in enumerate(expected_results): assert result[index].tag == expected[0] - assert result[index].lineno == expected[1] + assert result[index].lineno in expected[1] assert result[index].column == expected[2] assert str(expected[3]) in result[index].message # We internally convert absolute paths returned by ansible into paths diff --git a/test/test_app.py b/test/test_app.py index f2a7755105..a0d510e78a 100644 --- a/test/test_app.py +++ b/test/test_app.py @@ -10,7 +10,7 @@ def test_generate_ignore(tmp_path: Path) -> None: """Validate that --generate-ignore dumps expected ignore to the file.""" lintable = Lintable(tmp_path / "vars.yaml") - lintable.content = "foo: bar\nfoo: baz\n" + lintable.content = "foo: 1\nbar: baz\n" lintable.write(force=True) ignore_file = tmp_path / ".ansible-lint-ignore" assert not ignore_file.exists() @@ -19,7 +19,7 @@ def test_generate_ignore(tmp_path: Path) -> None: assert ignore_file.exists() with ignore_file.open(encoding="utf-8") as f: - assert "vars.yaml yaml[key-duplicates]\n" in f.readlines() + assert "vars.yaml yaml[colons]\n" in f.readlines() # Run again and now we expect to succeed as we have an ignore file. result = run_ansible_lint(lintable.filename, cwd=tmp_path) assert result.returncode == 0 diff --git a/test/test_main.py b/test/test_main.py index 74151551ec..9603426f5b 100644 --- a/test/test_main.py +++ b/test/test_main.py @@ -153,9 +153,13 @@ def test_broken_ansible_cfg() -> None: cwd="test/fixtures/broken-ansible.cfg", ) assert proc.returncode == RC.INVALID_CONFIG, proc - assert ( - "Invalid type for configuration option setting: CACHE_PLUGIN_TIMEOUT" - in proc.stderr + # 2.19 had different errors + assert any( + x in proc.stderr + for x in ( + "Invalid type for configuration option setting: CACHE_PLUGIN_TIMEOUT", + "has an invalid value: Invalid type provided for 'int': 'invalid-value'", + ) ) diff --git a/test/test_utils.py b/test/test_utils.py index 3537039380..4612a25e1c 100644 --- a/test/test_utils.py +++ b/test/test_utils.py @@ -324,7 +324,10 @@ def test_cli_auto_detect(capfd: CaptureFixture[str]) -> None: out, err = capfd.readouterr() # An expected rule match from our examples - assert "playbook.yml:6: name[casing]" in out + assert any( + x in out + for x in ("playbook.yml:6: name[casing]", "playbook.yml:6:13: name[casing]") + ) # assures that our ansible-lint config exclude was effective in excluding github files assert "Identified: .github/" not in out # assures that we can parse playbooks as playbooks diff --git a/test/test_vulture.py b/test/test_vulture.py index 704340e74e..9f25814a8b 100644 --- a/test/test_vulture.py +++ b/test/test_vulture.py @@ -10,6 +10,7 @@ from ansiblelint.rules.meta_video_links import MetaVideoLinksRule from ansiblelint.rules.no_handler import UseHandlerRatherThanWhenChangedRule from ansiblelint.rules.no_relative_paths import RoleRelativePath +from ansiblelint.rules.no_tabs import NoTabsRule from ansiblelint.rules.risky_file_permissions import MissingFilePermissionsRule from ansiblelint.rules.syntax_check import AnsibleSyntaxCheckRule from test.custom_rules.example_com.example_com_rule import ExampleComRule @@ -27,6 +28,7 @@ "MetaChangeFromDefaultRule", "MetaVideoLinksRule", "MissingFilePermissionsRule", + "NoTabsRule", "RoleRelativePath", "TaskNoLocalActionRule", "UnsetVariableMatcherRule",