Make MatchError a dataclass

src/ansiblelint/_internal/rules.py

@@ -156,6 +156,7 @@ class RuntimeErrorRule(BaseRule):
     """Unexpected internal error."""
 
     id = "internal-error"
+    shortdesc = "Unexpected internal error"
     severity = "VERY_HIGH"
     tags = ["core"]
     version_added = "v5.0.0"

src/ansiblelint/errors.py

@@ -2,11 +2,12 @@
 from __future__ import annotations
 
 import functools
+from dataclasses import dataclass, field
 from typing import Any
 
 from ansiblelint._internal.rules import BaseRule, RuntimeErrorRule
 from ansiblelint.config import options
-from ansiblelint.file_utils import Lintable, normpath
+from ansiblelint.file_utils import Lintable
 
 
 class StrictModeError(RuntimeError):
@@ -21,6 +22,7 @@ def __init__(
 
 
 # pylint: disable=too-many-instance-attributes
+@dataclass(unsafe_hash=True)
 @functools.total_ordering
 class MatchError(ValueError):
     """Rule violation detected during linting.
@@ -32,70 +34,61 @@ class MatchError(ValueError):
     instance.
     """
 
-    tag = ""
-
-    # IMPORTANT: any additional comparison protocol methods must return
-    # IMPORTANT: `NotImplemented` singleton to allow the check to use the
-    # IMPORTANT: other object's fallbacks.
-    # Ref: https://docs.python.org/3/reference/datamodel.html#object.__lt__
+    # order matters for these:
+    message: str = field(init=True, repr=False, default="")
+    lintable: Lintable = field(init=True, repr=False, default=Lintable(name=""))
+    filename: str = field(init=True, repr=False, default="")
+
+    tag: str = field(init=True, repr=False, default="")
+    lineno: int = 1
+    details: str = ""
+    column: int | None = None
+    # rule is not included in hash because we might have different instances
+    # of the same rule, but we use the 'tag' to identify the rule.
+    rule: BaseRule = field(hash=False, default=RuntimeErrorRule())
+    ignored: bool = False
+    fixed: bool = False  # True when a transform has resolved this MatchError
+
+    def __post_init__(self) -> None:
+        # This can be used by rules that can report multiple errors type, so
+        # we can still filter by them.
 
-    # pylint: disable=too-many-arguments
-    def __init__(
-        self,
-        message: str | None = None,
-        # most linters report use (1,1) base, including yamllint and flake8
-        # we should never report line 0 or column 0 in output.
-        lineno: int = 1,
-        column: int | None = None,
-        details: str = "",
-        filename: Lintable | None = None,
-        rule: BaseRule = RuntimeErrorRule(),
-        tag: str | None = None,  # optional fine-graded tag
-        ignored: bool = False,
-    ) -> None:
-        """Initialize a MatchError instance."""
-        super().__init__(message)
+        if not self.lintable and self.filename:
+            self.lintable = Lintable(self.filename)
+        elif self.lintable and not self.filename:
+            self.filename = self.lintable.name
+
+        # We want to catch accidental MatchError() which contains no useful
+        # information. When no arguments are passed, the '_message' field is
+        # set to 'property', only if passed it becomes a string.
+        if self.rule.__class__ is RuntimeErrorRule:
+            # so instance was created without a rule
+            if not self.message:
+                msg = f"{self.__class__.__name__}() missing a required argument: one of 'message' or 'rule'"
+                raise TypeError(msg)
+            if not isinstance(self.tag, str):
+                msg = "MatchErrors must be created with either rule or tag specified."
+                raise TypeError(msg)
+        if not self.message:
+            self.message = self.rule.shortdesc
 
-        if rule.__class__ is RuntimeErrorRule and not message:
-            msg = f"{self.__class__.__name__}() missing a required argument: one of 'message' or 'rule'"
-            raise TypeError(msg)
+        self.match_type: str | None = None
+        # for task matches, save the normalized task object (useful for transforms)
+        self.task: dict[str, Any] | None = None
+        # path to the problem area, like: [0,"pre_tasks",3] for [0].pre_tasks[3]
+        self.yaml_path: list[int | str] = []
 
-        self.message = str(message or getattr(rule, "shortdesc", ""))
+        if not self.tag:
+            self.tag = self.rule.id
 
         # Safety measure to ensure we do not end-up with incorrect indexes
-        if lineno == 0:  # pragma: no cover
+        if self.lineno == 0:  # pragma: no cover
             msg = "MatchError called incorrectly as line numbers start with 1"
             raise RuntimeError(msg)
-        if column == 0:  # pragma: no cover
+        if self.column == 0:  # pragma: no cover
             msg = "MatchError called incorrectly as column numbers start with 1"
             raise RuntimeError(msg)
 
-        self.lineno = lineno
-        self.column = column
-        self.details = details
-        self.filename = ""
-        if filename:
-            if isinstance(filename, Lintable):
-                self.lintable = filename
-                self.filename = normpath(str(filename.path))
-            else:
-                self.filename = normpath(filename)
-                self.lintable = Lintable(self.filename)
-        self.rule = rule
-        self.ignored = ignored  # If set it will be displayed but not counted as failure
-        # This can be used by rules that can report multiple errors type, so
-        # we can still filter by them.
-        self.tag = tag or rule.id
-
-        # optional indicator on how this error was found
-        self.match_type: str | None = None
-        # for task matches, save the normalized task object (useful for transforms)
-        self.task: dict[str, Any] | None = None
-        # path to the problem area, like: [0,"pre_tasks",3] for [0].pre_tasks[3]
-        self.yaml_path: list[int | str] = []
-        # True when a transform has resolved this MatchError
-        self.fixed = False
-
     @functools.cached_property
     def level(self) -> str:
         """Return the level of the rule: error, warning or notice."""
@@ -147,10 +140,6 @@ def __lt__(self, other: object) -> bool:
             return NotImplemented
         return bool(self._hash_key < other._hash_key)
 
-    def __hash__(self) -> int:
-        """Return a hash value of the MatchError instance."""
-        return hash(self._hash_key)
-
     def __eq__(self, other: object) -> bool:
         """Identify whether the other object represents the same rule match."""
         if not isinstance(other, self.__class__):

src/ansiblelint/rules/__init__.py

@@ -73,7 +73,7 @@ def unjinja(text: str) -> str:
     # pylint: disable=too-many-arguments
     def create_matcherror(
         self,
-        message: str | None = None,
+        message: str = "",
         lineno: int = 1,
         details: str = "",
         filename: Lintable | None = None,
@@ -84,11 +84,10 @@ def create_matcherror(
             message=message,
             lineno=lineno,
             details=details,
-            filename=filename,
+            lintable=filename or Lintable(""),
             rule=copy.copy(self),
+            tag=tag,
         )
-        if tag:
-            match.tag = tag
         # search through callers to find one of the match* methods
         frame = inspect.currentframe()
         match_type: str | None = None
@@ -128,7 +127,7 @@ def matchlines(self, file: Lintable) -> list[MatchError]:
             result = self.match(line)
             if not result:
                 continue
-            message = None
+            message = ""
             if isinstance(result, str):
                 message = result
             matcherror = self.create_matcherror(
@@ -198,7 +197,7 @@ def matchtasks(self, file: Lintable) -> list[MatchError]:  # noqa: C901
                     continue
                 match = result
             else:  # bool or string
-                message = None
+                message = ""
                 if isinstance(result, str):
                     message = result
                 match = self.create_matcherror(
@@ -223,7 +222,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
         if isinstance(yaml, str):
             if yaml.startswith("$ANSIBLE_VAULT"):
                 return []
-            return [MatchError(filename=file, rule=LoadingFailureRule())]
+            return [MatchError(lintable=file, rule=LoadingFailureRule())]
         if not yaml:
             return matches
 
@@ -458,7 +457,7 @@ def run(  # : max-complexity: 12
                 return [
                     MatchError(
                         message=str(exc),
-                        filename=file,
+                        lintable=file,
                         rule=LoadingFailureRule(),
                         tag=f"{LoadingFailureRule.id}[{exc.__class__.__name__.lower()}]",
                     ),

src/ansiblelint/rules/schema.py

@@ -67,7 +67,7 @@ def matchtask(
                 result.append(
                     MatchError(
                         message=msg,
-                        filename=file,
+                        lintable=file or Lintable(""),
                         rule=ValidateSchemaRule(),
                         details=ValidateSchemaRule.description,
                         tag=f"schema[{tag}]",
@@ -93,7 +93,7 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
             result.append(
                 MatchError(
                     message=errors[0],
-                    filename=file,
+                    lintable=file,
                     rule=ValidateSchemaRule(),
                     details=ValidateSchemaRule.description,
                     tag=f"schema[{file.kind}]",

src/ansiblelint/rules/syntax_check.py

@@ -136,7 +136,6 @@ def _get_ansible_syntax_check_matches(  # noqa: C901
             filename = lintable
             lineno = 1
             column = None
-            tag = None
 
             stderr = strip_ansi_escape(run.stderr)
             stdout = strip_ansi_escape(run.stdout)
@@ -164,7 +163,7 @@ def _get_ansible_syntax_check_matches(  # noqa: C901
                     results.append(
                         MatchError(
                             message=title,
-                            filename=filename,
+                            lintable=filename,
                             lineno=lineno,
                             column=column,
                             rule=rule,
@@ -182,12 +181,12 @@ def _get_ansible_syntax_check_matches(  # noqa: C901
                 results.append(
                     MatchError(
                         message=message,
-                        filename=filename,
+                        lintable=filename,
                         lineno=lineno,
                         column=column,
                         rule=rule,
                         details=details,
-                        tag=tag,
+                        tag="",
                     ),
                 )
 

src/ansiblelint/runner.py

@@ -124,7 +124,7 @@ def run(self) -> list[MatchError]:  # noqa: C901
             if isinstance(lintable.data, States) and lintable.exc:
                 matches.append(
                     MatchError(
-                        filename=lintable,
+                        lintable=lintable,
                         message=str(lintable.exc),
                         details=str(lintable.exc.__cause__),
                         rule=LoadingFailureRule(),
@@ -208,7 +208,7 @@ def _emit_matches(self, files: list[Lintable]) -> Generator[MatchError, None, No
                     exc.rule = LoadingFailureRule()
                     yield exc
                 except AttributeError:
-                    yield MatchError(filename=lintable, rule=LoadingFailureRule())
+                    yield MatchError(lintable=lintable, rule=LoadingFailureRule())
                 visited.add(lintable)
 
 

src/ansiblelint/utils.py

@@ -270,7 +270,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=lintable, rule=LoadingFailureRule())
+        raise MatchError(lintable=lintable, rule=LoadingFailureRule())
     for item in _playbook_items(playbook_ds):
         # if lintable.kind not in ["playbook"]:
         for child in play_children(basedir, item, lintable.kind, playbook_dir):

test/test_formatter.py

@@ -38,7 +38,7 @@ def test_format_coloured_string() -> None:
         message="message",
         lineno=1,
         details=DETAILS,
-        filename=Lintable("filename.yml", content=""),
+        lintable=Lintable("filename.yml", content=""),
         rule=rule,
     )
     formatter.format(match)
@@ -50,7 +50,7 @@ def test_unicode_format_string() -> None:
         message="\U0001f427",
         lineno=1,
         details=DETAILS,
-        filename=Lintable("filename.yml", content=""),
+        lintable=Lintable("filename.yml", content=""),
         rule=rule,
     )
     formatter.format(match)
@@ -62,7 +62,7 @@ def test_dict_format_line() -> None:
         message="xyz",
         lineno=1,
         details={"hello": "world"},  # type: ignore
-        filename=Lintable("filename.yml", content=""),
+        lintable=Lintable("filename.yml", content=""),
         rule=rule,
     )
     formatter.format(match)

test/test_formatter_json.py

@@ -32,7 +32,7 @@ def setup_class(self) -> None:
                 message="message",
                 lineno=1,
                 details="hello",
-                filename=Lintable("filename.yml", content=""),
+                lintable=Lintable("filename.yml", content=""),
                 rule=self.rule,
             ),
         )
@@ -41,7 +41,7 @@ def setup_class(self) -> None:
                 message="message",
                 lineno=2,
                 details="hello",
-                filename=Lintable("filename.yml", content=""),
+                lintable=Lintable("filename.yml", content=""),
                 rule=self.rule,
                 ignored=True,
             ),
@@ -110,7 +110,7 @@ def test_validate_codeclimate_schema_with_positions(self) -> None:
                         lineno=1,
                         column=42,
                         details="hello",
-                        filename=Lintable("filename.yml", content=""),
+                        lintable=Lintable("filename.yml", content=""),
                         rule=self.rule,
                     ),
                 ],

test/test_formatter_sarif.py

@@ -38,7 +38,7 @@ def setup_class(self) -> None:
                 lineno=1,
                 column=10,
                 details="details",
-                filename=Lintable("filename.yml", content=""),
+                lintable=Lintable("filename.yml", content=""),
                 rule=self.rule,
                 tag="yaml[test]",
             ),
@@ -48,7 +48,7 @@ def setup_class(self) -> None:
                 message="message",
                 lineno=2,
                 details="",
-                filename=Lintable("filename.yml", content=""),
+                lintable=Lintable("filename.yml", content=""),
                 rule=self.rule,
                 tag="yaml[test]",
             ),

test/test_matcherrror.py

@@ -64,11 +64,8 @@ def test_matcherror_compare(
 
 def test_matcherror_invalid() -> None:
     """Ensure that MatchError requires message or rule."""
-    expected_err = (
-        r"^MatchError\(\) missing a required argument: one of 'message' or 'rule'$"
-    )
-    with pytest.raises(TypeError, match=expected_err):
-        raise MatchError
+    with pytest.raises(TypeError):
+        MatchError()  # pylint: disable=pointless-exception-statement
 
 
 @pytest.mark.parametrize(
@@ -78,8 +75,8 @@ def test_matcherror_invalid() -> None:
         (MatchError("z"), MatchError("a")),
         # filenames takes priority in sorting
         (
-            MatchError("a", filename=Lintable("b", content="")),
-            MatchError("a", filename=Lintable("a", content="")),
+            MatchError("a", lintable=Lintable("b", content="")),
+            MatchError("a", lintable=Lintable("a", content="")),
         ),
         # rule id partial-become > rule id no-changed-when
         (
@@ -182,7 +179,7 @@ def test_matcherror_compare_with_other_fallback(
     expected_value: bool,
 ) -> None:
     """Check that MatchError comparison runs other types fallbacks."""
-    assert operation(MatchError("foo"), other) is expected_value
+    assert operation(MatchError(message="foo"), other) is expected_value
 
 
 @pytest.mark.parametrize(