Refactor line number identification

.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

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:

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

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]

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
 ]
 

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),

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
 

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

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,
                 ),

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,
                 ),

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,
                 ),
             )

src/ansiblelint/rules/jinja.py

@@ -6,14 +6,14 @@
 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
 
 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,14 +216,16 @@ def matchtask(
                         lintable=file,
                     )
                     if reformatted != v:
+                        lineno = task.get_error_line([*path, key])
                         result.append(
                             self.create_matcherror(
                                 message=self._msg(
                                     tag=tag,
                                     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."""

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