Skip to content

Commit

Permalink
Make MatchError a dataclass
Browse files Browse the repository at this point in the history
  • Loading branch information
ssbarnea committed Apr 25, 2023
1 parent 3536bfd commit 6d940b0
Show file tree
Hide file tree
Showing 11 changed files with 80 additions and 94 deletions.
1 change: 1 addition & 0 deletions src/ansiblelint/_internal/rules.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
112 changes: 51 additions & 61 deletions src/ansiblelint/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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.
Expand All @@ -32,74 +34,66 @@ 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:
raise TypeError(
f"{self.__class__.__name__}() missing a "
"required argument: one of 'message' or 'rule'",
)
if not isinstance(self.tag, str):
raise TypeError(
"MatchErrors must be created with either rule or tag specified.",
)
if not self.message:
self.message = self.rule.shortdesc

if rule.__class__ is RuntimeErrorRule and not message:
raise TypeError(
f"{self.__class__.__name__}() missing a "
"required argument: one of 'message' or 'rule'",
)
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
raise RuntimeError(
"MatchError called incorrectly as line numbers start with 1",
)
if column == 0: # pragma: no cover
if self.column == 0: # pragma: no cover
raise RuntimeError(
"MatchError called incorrectly as column numbers start with 1",
)

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."""
Expand Down Expand Up @@ -151,10 +145,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__):
Expand Down
15 changes: 7 additions & 8 deletions src/ansiblelint/rules/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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

Expand Down Expand Up @@ -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()}]",
),
Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/rules/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}]",
Expand All @@ -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}]",
Expand Down
7 changes: 3 additions & 4 deletions src/ansiblelint/rules/syntax_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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="",
),
)

Expand Down
4 changes: 2 additions & 2 deletions src/ansiblelint/runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -207,7 +207,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)


Expand Down
2 changes: 1 addition & 1 deletion src/ansiblelint/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,7 +225,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):
Expand Down
6 changes: 3 additions & 3 deletions test/test_formatter.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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)
6 changes: 3 additions & 3 deletions test/test_formatter_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
),
)
Expand All @@ -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,
),
Expand Down Expand Up @@ -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,
),
],
Expand Down
4 changes: 2 additions & 2 deletions test/test_formatter_sarif.py
Original file line number Diff line number Diff line change
Expand Up @@ -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]",
),
Expand All @@ -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]",
),
Expand Down
Loading

0 comments on commit 6d940b0

Please sign in to comment.