Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor the rendering of errors and warnings #2566

Merged
merged 1 commit into from
Oct 6, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 2 additions & 3 deletions src/ansiblelint/color.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
}
)
Expand Down
8 changes: 8 additions & 0 deletions src/ansiblelint/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down Expand Up @@ -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}"
Expand Down
63 changes: 26 additions & 37 deletions src/ansiblelint/formatters/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}"
Expand All @@ -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}"
)

Expand All @@ -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
Expand All @@ -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.
Expand All @@ -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")
Expand All @@ -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"]:
Expand All @@ -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 = (
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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
16 changes: 16 additions & 0 deletions src/ansiblelint/rules/yaml.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions src/ansiblelint/rules/yaml_rule.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
],
),
(
Expand Down
2 changes: 1 addition & 1 deletion test/test_cli_role_paths.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down