refactored runner.is_exclude and MatchError to use Lintable (#2379)

* refactored runner.is_exclude use Lintable
* refactoring: make MatchError use only lintable

This refactoring will help us simplify the codebase and enable us to make use of some extra data that is specific to each file, like implementing per file (lintable) skip lists.

src/ansiblelint/errors.py

@@ -34,7 +34,7 @@ def __init__(
         linenumber: int = 1,
         column: int | None = None,
         details: str = "",
-        filename: str | Lintable | None = None,
+        filename: Lintable | None = None,
         rule: BaseRule = RuntimeErrorRule(),
         tag: str | None = None,  # optional fine-graded tag
     ) -> None:

src/ansiblelint/rules/__init__.py

@@ -86,7 +86,7 @@ def create_matcherror(
         message: str | None = None,
         linenumber: int = 1,
         details: str = "",
-        filename: str | Lintable | None = None,
+        filename: Lintable | None = None,
         tag: str = "",
     ) -> MatchError:
         """Instantiate a new MatchError."""
@@ -207,7 +207,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
         if isinstance(yaml, str):
             if yaml.startswith("$ANSIBLE_VAULT"):
                 return []
-            return [MatchError(filename=str(file.path), rule=LoadingFailureRule())]
+            return [MatchError(filename=file, rule=LoadingFailureRule())]
         if not yaml:
             return matches
 

src/ansiblelint/rules/partial_become.py

@@ -104,7 +104,7 @@ def matchplay(self, file: Lintable, data: odict[str, Any]) -> list[MatchError]:
                 return [
                     self.create_matcherror(
                         message=self.shortdesc,
-                        filename=str(file.path),
+                        filename=file,
                         linenumber=data[LINE_NUMBER_KEY],
                     )
                 ]

src/ansiblelint/rules/playbook_extension.py

@@ -28,5 +28,5 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
         ext = os.path.splitext(path)
         if ext[1] not in [".yml", ".yaml"] and path not in self.done:
             self.done.append(path)
-            result.append(self.create_matcherror(filename=path))
+            result.append(self.create_matcherror(filename=file))
         return result

src/ansiblelint/rules/role_name.py

@@ -82,7 +82,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
             if role_name and not self._re.match(role_name):
                 result.append(
                     self.create_matcherror(
-                        filename=str(file.path),
+                        filename=file,
                         message=self.shortdesc.format(role_name),
                     )
                 )

src/ansiblelint/rules/syntax_check.py

@@ -75,7 +75,7 @@ def _get_ansible_syntax_check_matches(lintable: Lintable) -> list[MatchError]:
             result = []
         if run.returncode != 0:
             message = None
-            filename = str(lintable.path)
+            filename = lintable
             linenumber = 1
             column = None
             tag = None
@@ -93,12 +93,12 @@ def _get_ansible_syntax_check_matches(lintable: Lintable) -> list[MatchError]:
             if match:
                 message = match.groupdict()["title"]
                 # Ansible returns absolute paths
-                filename = match.groupdict()["filename"]
+                filename = Lintable(match.groupdict()["filename"])
                 linenumber = int(match.groupdict()["line"])
                 column = int(match.groupdict()["column"])
             elif _empty_playbook_re.search(stderr):
                 message = "Empty playbook, nothing to do"
-                filename = str(lintable.path)
+                filename = lintable
                 tag = "empty-playbook"
 
             if run.returncode == 4:

src/ansiblelint/rules/yaml_rule.py

@@ -51,7 +51,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
                     message=problem.desc,
                     linenumber=problem.line,
                     details="",
-                    filename=str(file.path),
+                    filename=file,
                     tag=f"yaml[{problem.rule}]",
                 )
             )

src/ansiblelint/runner.py

@@ -7,7 +7,6 @@
 import os
 from dataclasses import dataclass
 from fnmatch import fnmatch
-from pathlib import Path
 from typing import TYPE_CHECKING, Any, Generator
 
 import ansiblelint.skip_utils
@@ -84,25 +83,24 @@ def _update_exclude_paths(self, exclude_paths: list[str]) -> None:
         else:
             self.exclude_paths = []
 
-    def is_excluded(self, file_path: str) -> bool:
+    def is_excluded(self, lintable: Lintable) -> bool:
         """Verify if a file path should be excluded."""
         # Any will short-circuit as soon as something returns True, but will
         # be poor performance for the case where the path under question is
         # not excluded.
 
         # Exclusions should be evaluated only using absolute paths in order
         # to work correctly.
-        if not file_path:
+        if not lintable:
             return False
 
-        abs_path = os.path.abspath(file_path)
-        _file_path = Path(file_path)
+        abs_path = str(lintable.abspath)
 
         return any(
             abs_path.startswith(path)
-            or _file_path.match(path)
+            or lintable.path.match(path)
             or fnmatch(str(abs_path), path)
-            or fnmatch(str(_file_path), path)
+            or fnmatch(str(lintable), path)
             for path in self.exclude_paths
         )
 
@@ -113,15 +111,15 @@ def run(self) -> list[MatchError]:  # noqa: C901
 
         # remove exclusions
         for lintable in self.lintables.copy():
-            if self.is_excluded(str(lintable.path.resolve())):
+            if self.is_excluded(lintable):
                 _logger.debug("Excluded %s", lintable)
                 self.lintables.remove(lintable)
             try:
                 lintable.data
             except RuntimeError as exc:
                 matches.append(
                     MatchError(
-                        filename=str(lintable.path),
+                        filename=lintable,
                         message=str(exc),
                         details=str(exc.__cause__),
                         rule=LoadingFailureRule(),
@@ -180,7 +178,9 @@ def worker(lintable: Lintable) -> list[MatchError]:
 
         # remove any matches made inside excluded files
         matches = list(
-            filter(lambda match: not self.is_excluded(match.filename), matches)
+            filter(
+                lambda match: not self.is_excluded(Lintable(match.filename)), matches
+            )
         )
 
         return sorted(set(matches))
@@ -191,7 +191,7 @@ def _emit_matches(self, files: list[Lintable]) -> Generator[MatchError, None, No
             for lintable in self.lintables - visited:
                 try:
                     for child in ansiblelint.utils.find_children(lintable):
-                        if self.is_excluded(str(child.path)):
+                        if self.is_excluded(child):
                             continue
                         self.lintables.add(child)
                         files.append(child)
@@ -201,9 +201,7 @@ def _emit_matches(self, files: list[Lintable]) -> Generator[MatchError, None, No
                     exc.rule = LoadingFailureRule()
                     yield exc
                 except AttributeError:
-                    yield MatchError(
-                        filename=str(lintable.path), rule=LoadingFailureRule()
-                    )
+                    yield MatchError(filename=lintable, rule=LoadingFailureRule())
                 visited.add(lintable)
 
 

src/ansiblelint/utils.py

@@ -185,7 +185,7 @@ def find_children(lintable: Lintable) -> list[Lintable]:  # noqa: C901
     basedir = os.path.dirname(str(lintable.path))
     # playbook_ds can be an AnsibleUnicode string, which we consider invalid
     if isinstance(playbook_ds, str):
-        raise MatchError(filename=str(lintable.path), rule=LoadingFailureRule())
+        raise MatchError(filename=lintable, rule=LoadingFailureRule())
     for item in _playbook_items(playbook_ds):
         # if lintable.kind not in ["playbook"]:
         #     continue

test/test_formatter.py

@@ -21,6 +21,7 @@
 import pathlib
 
 from ansiblelint.errors import MatchError
+from ansiblelint.file_utils import Lintable
 from ansiblelint.formatters import Formatter
 from ansiblelint.rules import AnsibleLintRule
 
@@ -35,7 +36,7 @@ def test_format_coloured_string() -> None:
         message="message",
         linenumber=1,
         details="hello",
-        filename="filename.yml",
+        filename=Lintable("filename.yml"),
         rule=rule,
     )
     formatter.format(match)
@@ -47,7 +48,7 @@ def test_unicode_format_string() -> None:
         message="\U0001f427",
         linenumber=1,
         details="hello",
-        filename="filename.yml",
+        filename=Lintable("filename.yml"),
         rule=rule,
     )
     formatter.format(match)
@@ -59,7 +60,7 @@ def test_dict_format_line() -> None:
         message="xyz",
         linenumber=1,
         details={"hello": "world"},  # type: ignore
-        filename="filename.yml",
+        filename=Lintable("filename.yml"),
         rule=rule,
     )
     formatter.format(match)

test/test_formatter_json.py

@@ -9,6 +9,7 @@
 import pytest
 
 from ansiblelint.errors import MatchError
+from ansiblelint.file_utils import Lintable
 from ansiblelint.formatters import CodeclimateJSONFormatter
 from ansiblelint.rules import AnsibleLintRule
 
@@ -31,7 +32,7 @@ def setup_class(self) -> None:
                 message="message",
                 linenumber=1,
                 details="hello",
-                filename="filename.yml",
+                filename=Lintable("filename.yml"),
                 rule=self.rule,
             )
         )
@@ -40,7 +41,7 @@ def setup_class(self) -> None:
                 message="message",
                 linenumber=2,
                 details="hello",
-                filename="filename.yml",
+                filename=Lintable("filename.yml"),
                 rule=self.rule,
             )
         )

test/test_formatter_sarif.py

@@ -9,6 +9,7 @@
 import pytest
 
 from ansiblelint.errors import MatchError
+from ansiblelint.file_utils import Lintable
 from ansiblelint.formatters import SarifFormatter
 from ansiblelint.rules import AnsibleLintRule
 
@@ -35,7 +36,7 @@ def setup_class(self) -> None:
                 linenumber=1,
                 column=10,
                 details="hello",
-                filename="filename.yml",
+                filename=Lintable("filename.yml"),
                 rule=self.rule,
             )
         )
@@ -44,7 +45,7 @@ def setup_class(self) -> None:
                 message="message",
                 linenumber=2,
                 details="hello",
-                filename="filename.yml",
+                filename=Lintable("filename.yml"),
                 rule=self.rule,
             )
         )

test/test_matcherrror.py

@@ -6,6 +6,7 @@
 import pytest
 
 from ansiblelint.errors import MatchError
+from ansiblelint.file_utils import Lintable
 from ansiblelint.rules.no_changed_when import CommandHasChangesCheckRule
 from ansiblelint.rules.partial_become import BecomeUserWithoutBecomeRule
 
@@ -75,7 +76,10 @@ def test_matcherror_invalid() -> None:
         # sorting by message
         (MatchError("z"), MatchError("a")),
         # filenames takes priority in sorting
-        (MatchError("a", filename="b"), MatchError("a", filename="a")),
+        (
+            MatchError("a", filename=Lintable("b")),
+            MatchError("a", filename=Lintable("a")),
+        ),
         # rule id partial-become > rule id no-changed-when
         (
             MatchError(rule=BecomeUserWithoutBecomeRule()),