Refactor the rendering of errors and warnings (#2566)

- Display warnings different than errors on all output formats
- Document YAML specific errors
- Display warning using yellow AND adding (warning) suffix
- Corrected cases where warning was determined incorrectly
- Display only the detailed rule tag on matches (reduce clutter)
- Capitalize YAML error messages

Fixes: #1670

src/ansiblelint/color.py

@@ -54,11 +54,10 @@
 _theme = Theme(
     {
         "info": "cyan",
-        "warning": "dim yellow",
+        "warning": "yellow",
         "danger": "bold red",
         "title": "yellow",
-        "error_code": "bright_red",
-        "error_title": "red",
+        "error": "bright_red",
         "filename": "blue",
     }
 )

src/ansiblelint/errors.py

@@ -5,6 +5,7 @@
 from typing import Any
 
 from ansiblelint._internal.rules import BaseRule, RuntimeErrorRule
+from ansiblelint.config import options
 from ansiblelint.file_utils import Lintable, normpath
 
 
@@ -87,6 +88,13 @@ def __init__(
         # 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."""
+        if {self.tag, self.rule.id, *self.rule.tags}.isdisjoint(options.warn_list):
+            return "error"
+        return "warning"
+
     def __repr__(self) -> str:
         """Return a MatchError instance representation."""
         formatstr = "[{0}] ({1}) matched {2}:{3} {4}"

src/ansiblelint/formatters/__init__.py

@@ -58,9 +58,9 @@ class Formatter(BaseFormatter):  # type: ignore
 
     def format(self, match: MatchError) -> str:
         _id = getattr(match.rule, "id", "000")
-        result = f"[error_code][link={match.rule.url}]{_id}[/link][/][dim]:[/] [error_title]{self.escape(match.message)}[/]"
-        if match.tag:
-            result += f" [dim][error_code]({self.escape(match.tag)})[/][/]"
+        result = f"[{match.level}][bold][link={match.rule.url}]{self.escape(match.tag)}[/link][/][/][dim]:[/] [{match.level}]{self.escape(match.message)}[/]"
+        if match.level != "error":
+            result += f" [dim][{match.level}]({match.level})[/][/]"
         result += (
             "\n"
             f"[filename]{self._format_path(match.filename or '')}[/]:{match.position}"
@@ -76,7 +76,7 @@ class QuietFormatter(BaseFormatter[Any]):
 
     def format(self, match: MatchError) -> str:
         return (
-            f"[error_code]{match.rule.id}[/] "
+            f"[{match.level}]{match.rule.id}[/] "
             f"[filename]{self._format_path(match.filename or '')}[/]:{match.position}"
         )
 
@@ -86,20 +86,20 @@ class ParseableFormatter(BaseFormatter[Any]):
 
     def format(self, match: MatchError) -> str:
         result = (
-            f"[filename]{self._format_path(match.filename or '')}[/]:{match.position}: "
-            f"[error_code]{match.rule.id}[/]"
+            f"[filename]{self._format_path(match.filename or '')}[/][dim]:{match.position}:[/] "
+            f"[{match.level}][bold]{self.escape(match.tag)}[/bold]"
+            f": {match.message}"
+            if not options.quiet
+            else "[/]"
         )
+        if match.level != "error":
+            result += f" [dim][{match.level}]({match.level})[/][/]"
 
-        if not options.quiet:
-            result += f": [dim]{match.message}[/]"
-
-        if match.tag:
-            result += f" [dim][error_code]({self.escape(match.tag)})[/][/]"
         return result
 
 
 class AnnotationsFormatter(BaseFormatter):  # type: ignore
-    # https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions#setting-a-warning-message
+    # https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-warning-message
     """Formatter for emitting violations as GitHub Workflow Commands.
 
     These commands trigger the GHA Workflow runners platform to post violations
@@ -115,30 +115,19 @@ class AnnotationsFormatter(BaseFormatter):  # type: ignore
 
     def format(self, match: MatchError) -> str:
         """Prepare a match instance for reporting as a GitHub Actions annotation."""
-        level = self._severity_to_level(match.rule.severity)
         file_path = self._format_path(match.filename or "")
         line_num = match.linenumber
-        rule_id = match.rule.id
         severity = match.rule.severity
         violation_details = self.escape(match.message)
         if match.column:
             col = f",col={match.column}"
         else:
             col = ""
         return (
-            f"::{level} file={file_path},line={line_num}{col},severity={severity}"
-            f"::{rule_id} {violation_details}"
+            f"::{match.level} file={file_path},line={line_num}{col},severity={severity},title={match.tag}"
+            f"::{violation_details}"
         )
 
-    @staticmethod
-    def _severity_to_level(severity: str) -> str:
-        if severity in ["VERY_LOW", "LOW"]:
-            return "warning"
-        if severity in ["INFO"]:
-            return "debug"
-        # ['MEDIUM', 'HIGH', 'VERY_HIGH'] or anything else
-        return "error"
-
 
 class CodeclimateJSONFormatter(BaseFormatter[Any]):
     """Formatter for emitting violations in Codeclimate JSON report format.
@@ -161,7 +150,7 @@ def format_result(self, matches: list[MatchError]) -> str:
             issue["type"] = "issue"
             issue["check_name"] = match.tag or match.rule.id  # rule-id[subrule-id]
             issue["categories"] = match.rule.tags
-            issue["severity"] = self._severity_to_level(match.rule.severity)
+            issue["severity"] = self._severity_to_level(match)
             issue["description"] = self.escape(str(match.message))
             issue["fingerprint"] = hashlib.sha256(
                 repr(match).encode("utf-8")
@@ -184,7 +173,11 @@ def format_result(self, matches: list[MatchError]) -> str:
         return json.dumps(result)
 
     @staticmethod
-    def _severity_to_level(severity: str) -> str:
+    def _severity_to_level(match: MatchError) -> str:
+        if match.level != "error":
+            return "info"
+        severity = match.rule.severity
+
         if severity in ["LOW"]:
             return "minor"
         if severity in ["MEDIUM"]:
@@ -205,7 +198,7 @@ class SarifFormatter(BaseFormatter[Any]):
     """
 
     BASE_URI_ID = "SRCROOT"
-    TOOL_NAME = "Ansible-lint"
+    TOOL_NAME = "ansible-lint"
     TOOL_URL = "https://github.com/ansible/ansible-lint"
     SARIF_SCHEMA_VERSION = "2.1.0"
     SARIF_SCHEMA = (
@@ -267,12 +260,12 @@ def _extract_results(
     def _to_sarif_rule(self, match: MatchError) -> dict[str, Any]:
         rule: dict[str, Any] = {
             "id": match.rule.id,
-            "name": match.rule.id,
+            "name": match.tag,
             "shortDescription": {
                 "text": self.escape(str(match.message)),
             },
             "defaultConfiguration": {
-                "level": self._to_sarif_level(match.rule.severity),
+                "level": self._to_sarif_level(match),
             },
             "help": {
                 "text": str(match.rule.description),
@@ -311,10 +304,6 @@ def _to_sarif_result(self, match: MatchError) -> dict[str, Any]:
         return result
 
     @staticmethod
-    def _to_sarif_level(severity: str) -> str:
-        if severity in ["VERY_HIGH", "HIGH", "MEDIUM"]:
-            return "error"
-        if severity in ["LOW"]:
-            return "warning"
-        # VERY_LOW, INFO or anything else
-        return "note"
+    def _to_sarif_level(match: MatchError) -> str:
+        # sarif accepts only 4 levels: error, warning, note, none
+        return match.level

src/ansiblelint/rules/yaml.md

@@ -29,6 +29,22 @@ warn_list:
 
 See the [list of yamllint rules](https://yamllint.readthedocs.io/en/stable/rules.html) for more information.
 
+Some of the detailed error codes that you might see are:
+
+- `yaml[brackets]` - _too few spaces inside empty brackets_, or _too many spaces inside brackets_
+- `yaml[colons]` - _too many spaces before colon_, or _too many spaces after colon_
+- `yaml[commas]` - _too many spaces before comma_, or _too few spaces after comma_
+- `yaml[comments-indentation]` - _Comment not indented like content_
+- `yaml[comments]` - _Too few spaces before comment_, or _Missing starting space in comment_
+- `yaml[document-start]` - _missing document start "---"_ or _found forbidden document start "---"_
+- `yaml[empty-lines]` - _too many blank lines (...> ...)_
+- `yaml[indentation]` - _Wrong indentation: expected ... but found ..._
+- `yaml[key-duplicates]` - _Duplication of key "..." in mapping_
+- `yaml[new-line-at-end-of-file]` - _No new line character at the end of file_
+- `yaml[syntax]` - YAML syntax is broken
+- `yaml[trailing-spaces]` - Spaces are found at the end of lines
+- `yaml[truthy]` - _Truthy value should be one of ..._
+
 ## Problematic code
 
 ```yaml

src/ansiblelint/rules/yaml_rule.py

@@ -50,7 +50,8 @@ def matchyaml(self, file: Lintable) -> list[MatchError]:
                 self.severity = "VERY_HIGH"
             matches.append(
                 self.create_matcherror(
-                    message=problem.desc,
+                    # yamllint does return lower-case sentences
+                    message=problem.desc.capitalize(),
                     linenumber=problem.line,
                     details="",
                     filename=file,
@@ -129,9 +130,9 @@ def _fetch_skips(data: Any, collector: dict[int, set[str]]) -> dict[int, set[str
                 "examples/yamllint/invalid.yml",
                 "yaml",
                 [
-                    'missing document start "---"',
-                    'duplication of key "foo" in mapping',
-                    "trailing spaces",
+                    'Missing document start "---"',
+                    'Duplication of key "foo" in mapping',
+                    "Trailing spaces",
                 ],
             ),
             (

test/test_cli_role_paths.py

@@ -169,7 +169,7 @@ def test_run_playbook_github(result: bool, env: dict[str, str]) -> None:
     result_gh = run_ansible_lint(role_path, cwd=cwd, env=env)
 
     expected = (
-        "::warning file=examples/playbooks/example.yml,line=44,severity=VERY_LOW::package-latest "
+        "::error file=examples/playbooks/example.yml,line=44,severity=VERY_LOW,title=package-latest::"
         "Package installs should not use latest"
     )
     assert (expected in result_gh.stdout) is result

test/test_utils.py

@@ -320,7 +320,7 @@ def test_cli_auto_detect(capfd: CaptureFixture[str]) -> None:
     # An expected rule match from our examples
     assert (
         "examples/playbooks/empty_playbook.yml:1:1: "
-        "warning: Empty playbook, nothing to do" in out
+        "warning[empty-playbook]: Empty playbook, nothing to do" in out
     )
     # assures that our ansible-lint config exclude was effective in excluding github files
     assert "Identified: .github/" not in out