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.
ansible · Sep 1, 2022 · 5d580f3 · 5d580f3
1 parent a9bf5ca
commit 5d580f3
Show file tree

Hide file tree

Showing 13 changed files with 28 additions and 23 deletions.
diff --git a/src/ansiblelint/errors.py b/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:

diff --git a/src/ansiblelint/rules/__init__.py b/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
 

diff --git a/src/ansiblelint/rules/partial_become.py b/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],
                     )
                 ]

diff --git a/src/ansiblelint/rules/playbook_extension.py b/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
diff --git a/src/ansiblelint/rules/role_name.py b/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),
                     )
                 )

diff --git a/src/ansiblelint/rules/syntax_check.py b/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:

diff --git a/src/ansiblelint/rules/yaml_rule.py b/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}]",
                 )
             )

diff --git a/src/ansiblelint/runner.py b/src/ansiblelint/runner.py
@@ -119,7 +119,7 @@ def run(self) -> list[MatchError]:  # noqa: C901
             except RuntimeError as exc:
                 matches.append(
                     MatchError(
-                        filename=str(lintable.path),
+                        filename=lintable,
                         message=str(exc),
                         details=str(exc.__cause__),
                         rule=LoadingFailureRule(),
@@ -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)
 
 

diff --git a/src/ansiblelint/utils.py b/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

diff --git a/test/test_formatter.py b/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)
diff --git a/test/test_formatter_json.py b/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,
             )
         )

diff --git a/test/test_formatter_sarif.py b/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,
             )
         )

diff --git a/test/test_matcherrror.py b/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()),