Improve yamllint messages and documentation (#2148)

.github/workflows/tox.yml

@@ -155,7 +155,7 @@ jobs:
       WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
       # Number of expected test passes, safety measure for accidental skip of
       # tests. Update value if you add/remove tests.
-      PYTEST_REQPASS: 649
+      PYTEST_REQPASS: 652
 
     steps:
       - name: Activate WSL1

.pre-commit-config.yaml

@@ -28,6 +28,7 @@ repos:
             examples/playbooks/templates/not-valid.yaml|
             examples/playbooks/with-umlaut-.*|
             examples/playbooks/with-skip-tag-id.yml|
+            examples/yamllint/.*|
             test/fixtures/formatting-before/.*|
             src/ansiblelint/schemas/.*
           )$
@@ -53,6 +54,7 @@ repos:
         exclude: >
           (?x)^(
             test/eco/.*.result|
+            examples/yamllint/.*|
             test/fixtures/formatting-before/.*|
             test/fixtures/formatting-prettier/.*
           )$
@@ -61,6 +63,7 @@ repos:
           (?x)^(
             examples/playbooks/(with-skip-tag-id|unicode).yml|
             examples/playbooks/example.yml|
+            examples/yamllint/.*|
             test/eco/.*.result|
             test/fixtures/formatting-before/.*
           )$
@@ -89,6 +92,7 @@ repos:
         exclude: >
           (?x)^(
             examples/playbooks/templates/.*|
+            examples/yamllint/.*|
             examples/other/some.j2.yaml|
             test/fixtures/formatting-before/.*
           )$

examples/playbooks/with-skip-tag-id.yml

@@ -2,6 +2,6 @@
 - hosts: all
   tasks:
     - name: trailing whitespace on this line         
-      git:
+      ansible.builtin.git:
         repo: "{{ archive_services_repo_url }}"
         dest: /home/www

examples/yamllint/invalid.yml

@@ -0,0 +1,9 @@
+# missing document-start
+foo: ...
+foo: ...  # <-- key-duplicates
+bar: ...       # <-- wrong comment indentation
+
+# next line has trailing-spaces:
+other: aaa   
+
+# ^ empty-lines

src/ansiblelint/_internal/rules.py

@@ -20,6 +20,7 @@ class BaseRule:
     id: str = ""
     tags: List[str] = []
     description: str = ""
+    help: str = ""  # markdown help (automatically loaded from `<rule>.md`)
     version_added: str = ""
     severity: str = ""
     link: str = ""

src/ansiblelint/generate_docs.py

@@ -42,11 +42,18 @@ def rules_as_md(rules: RulesCollection) -> str:
         # and we do not have to insert `(target_header)=`
         title = f"{rule.id}"
 
-        description = rule.description
-        if rule.link:
-            description += f" [more]({rule.link})"
-
-        result += f"\n\n## {title}\n\n**{rule.shortdesc}**\n\n{description}"
+        if rule.help:
+            if not rule.help.startswith(f"## {rule.id}"):
+                raise RuntimeError(
+                    f"Rule {rule.__class__} markdown help does not start with `## {rule.id}` header.\n{rule.help}"
+                )
+            result += f"\n\n{rule.help}"
+        else:
+            description = rule.description
+            if rule.link:
+                description += f" [more]({rule.link})"
+
+            result += f"\n\n## {title}\n\n**{rule.shortdesc}**\n\n{description}"
 
     return result
 
@@ -59,7 +66,8 @@ def rules_as_rich(rules: RulesCollection) -> Iterable[Table]:
         table = Table(show_header=True, header_style="title", box=box.MINIMAL)
         table.add_column(rule.id, style="dim", width=width)
         table.add_column(Markdown(rule.shortdesc))
-        description = rule.description
+
+        description = rule.help or rule.description
         if rule.link:
             description += f" [(more)]({rule.link})"
         table.add_row("description", Markdown(description))

src/ansiblelint/rules/__init__.py

@@ -296,19 +296,32 @@ def is_valid_rule(rule: Any) -> bool:
     return issubclass(rule, AnsibleLintRule) and bool(rule.id) and bool(rule.shortdesc)
 
 
+# pylint: disable=too-many-nested-blocks
 def load_plugins(directory: str) -> Iterator[AnsibleLintRule]:
     """Yield a rule class."""
     for pluginfile in glob.glob(os.path.join(directory, "[A-Za-z]*.py")):
 
         pluginname = os.path.basename(pluginfile.replace(".py", ""))
         spec = importlib.util.spec_from_file_location(pluginname, pluginfile)
+
         # https://github.com/python/typeshed/issues/2793
         if spec and isinstance(spec.loader, Loader):
+
+            # load rule markdown documentation if found
+            help_md = ""
+            if spec.origin:
+                md_filename = os.path.splitext(spec.origin)[0] + ".md"
+                if os.path.exists(md_filename):
+                    with open(md_filename, encoding="utf-8") as f:
+                        help_md = f.read()
+
             module = importlib.util.module_from_spec(spec)
             spec.loader.exec_module(module)
             try:
                 for _, obj in inspect.getmembers(module):
                     if inspect.isclass(obj) and is_valid_rule(obj):
+                        if help_md:
+                            obj.help = help_md
                         yield obj()
 
             except (TypeError, ValueError, AttributeError) as exc:

src/ansiblelint/rules/yaml.md

@@ -0,0 +1,29 @@
+## yaml
+
+Our linter also includes violations reported by [yamllint](https://github.com/adrienverge/yamllint)
+but it uses a slightly different default configuration. We will still load
+custom yamllint configuration files, but the defaults come from
+ansible-lint, not from yamllint.
+
+You can fully disable all yamllint violations by adding `yaml` to the `skip_list`.
+
+Specific tag identifiers that are printed at the end of rule name,
+like `yaml[trailing-spaces]` or `yaml[indentation]` can also be be skipped, allowing
+you to have a more fine control.
+
+### Problematic code
+
+```yaml
+# missing document-start
+foo: ...
+foo: ...  # <-- key-duplicates
+bar: ...       # <-- wrong comment indentation
+```
+
+### Correct code
+
+```yaml
+---
+foo: ...
+bar: ... # comment
+```

src/ansiblelint/rules/yaml.py

@@ -1,5 +1,6 @@
 """Implementation of yaml linting rule (yamllint integration)."""
 import logging
+import sys
 from typing import TYPE_CHECKING, List
 
 from yamllint.linter import run as run_yamllint
@@ -14,30 +15,17 @@
 
 _logger = logging.getLogger(__name__)
 
-DESCRIPTION = """\
-Rule violations reported by YamlLint when this is installed.
-
-Yamllint configuration will be loaded following yamllint's configuration file
-resolution order.
-
-You can fully disable all of them by adding 'yaml' to the 'skip_list'.
-
-Specific tag identifiers that are printed at the end of rule name,
-like 'trailing-spaces' or 'indentation' can also be be skipped, allowing
-you to have a more fine control.
-"""
-
 
 class YamllintRule(AnsibleLintRule):
     """Violations reported by yamllint."""
 
     id = "yaml"
-    description = DESCRIPTION
     severity = "VERY_LOW"
     tags = ["formatting", "yaml"]
     version_added = "v5.0.0"
     config = load_yamllint_config()
     has_dynamic_tags = True
+    link = "https://yamllint.readthedocs.io/en/stable/rules.html"
 
     def __init__(self) -> None:
         """Construct a rule instance."""
@@ -65,7 +53,7 @@ def matchyaml(self, file: Lintable) -> List["MatchError"]:
                     linenumber=problem.line,
                     details="",
                     filename=str(file.path),
-                    tag=problem.rule,
+                    tag=f"yaml[{problem.rule}]",
                 )
             )
 
@@ -78,3 +66,63 @@ def matchyaml(self, file: Lintable) -> List["MatchError"]:
                 if match.rule.id not in skip_list and match.tag not in skip_list:
                     filtered_matches.append(match)
         return filtered_matches
+
+
+# testing code to be loaded only with pytest or when executed the rule file
+if "pytest" in sys.modules:
+    import pytest
+
+    # pylint: disable=ungrouped-imports
+    from ansiblelint.config import options
+    from ansiblelint.rules import RulesCollection
+    from ansiblelint.runner import Runner
+
+    @pytest.mark.parametrize(
+        ("file", "expected_kind", "expected"),
+        (
+            (
+                "examples/yamllint/invalid.yml",
+                "yaml",
+                [
+                    'missing document start "---"',
+                    'duplication of key "foo" in mapping',
+                    "trailing spaces",
+                ],
+            ),
+            (
+                "examples/yamllint/valid.yml",
+                "yaml",
+                [],
+            ),
+        ),
+        ids=(
+            "invalid",
+            "valid",
+        ),
+    )
+    def test_yamllint(file: str, expected_kind: str, expected: List[str]) -> None:
+        """Validate parsing of ansible output."""
+        lintable = Lintable(file)
+        assert lintable.kind == expected_kind
+
+        rules = RulesCollection(options=options)
+        rules.register(YamllintRule())
+        results = Runner(lintable, rules=rules).run()
+
+        assert len(results) == len(expected), results
+        for idx, result in enumerate(results):
+            assert result.filename.endswith(file)
+            assert expected[idx] in result.message
+            assert isinstance(result.tag, str)
+            assert result.tag.startswith("yaml[")
+
+    def test_yamllint_has_help(default_rules_collection: RulesCollection) -> None:
+        """Asserts that we loaded markdown documentation in help property."""
+        for collection in default_rules_collection:
+            if collection.id == "yaml":
+                print(collection.id)
+                assert collection.help is not None
+                assert len(collection.help) > 100
+                break
+        else:
+            pytest.fail("No yaml collection found")

test/eco/colsystem.result

@@ -17,7 +17,7 @@ warn_list:  # or 'skip_list' to silence them completely
 
 STDOUT:
 .ansible-lint:1: load-failure: [Errno 2] No such file or directory: '~/.cache/ansible-lint-eco/colsystem/tests/ansible-lint.yml' (load-failure[filenotfounderror])
-playbooks/molecule/sudo/molecule.yml:17: yaml: line too long (576 > 160 characters) (line-length)
+playbooks/molecule/sudo/molecule.yml:17: yaml: line too long (576 > 160 characters) (yaml[line-length])
 roles/authorized_key/meta/main.yml:1: schema: 7 is not one of ['6.1', '7.1', '7.2', 'all'] (schema[meta])
 roles/common/meta/main.yml:1: schema: 2.8 is not of type 'string' (schema[meta])
 roles/common/tasks/main.yml:1: unnamed-task: All tasks should be named.
@@ -28,8 +28,8 @@ roles/copy_or_link/meta/main.yml:1: schema: 2.4 is not of type 'string' (schema[
 roles/dev/meta/main.yml:1: schema: 2.4 is not of type 'string' (schema[meta])
 roles/dotfiles/meta/main.yml:1: schema: 2.8 is not of type 'string' (schema[meta])
 roles/epel/meta/main.yml:1: schema: 7 is not one of ['6.1', '7.1', '7.2', 'all'] (schema[meta])
-roles/firewalld/defaults/main.yml:11: yaml: missing starting space in comment (comments)
-roles/firewalld/defaults/main.yml:28: yaml: missing starting space in comment (comments)
+roles/firewalld/defaults/main.yml:11: yaml: missing starting space in comment (yaml[comments])
+roles/firewalld/defaults/main.yml:28: yaml: missing starting space in comment (yaml[comments])
 roles/firewalld/meta/main.yml:1: schema: 7 is not one of ['6.1', '7.1', '7.2', 'all'] (schema[meta])
 roles/flatpak/meta/main.yml:1: schema: 7 is not one of ['6.1', '7.1', '7.2', 'all'] (schema[meta])
 roles/flatpak/tasks/main.yml:1: unnamed-task: All tasks should be named.

test/eco/hardening.result

@@ -14,15 +14,15 @@ warn_list:  # or 'skip_list' to silence them completely
 
 
 STDOUT:
-defaults/main/sshd.yml:20: yaml: line too long (143 > 120 characters) (line-length)
+defaults/main/sshd.yml:20: yaml: line too long (143 > 120 characters) (yaml[line-length])
 meta/main.yml:1: schema: 8 is not one of ['6.1', '7.1', '7.2', 'all'] (schema[meta])
 molecule/default/verify.yml:1: schema: 'yes' is not of type 'boolean' (schema[playbook])
 tasks/adduser.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/aide.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/apparmor.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/apport.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/auditd.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
-tasks/auditd.yml:21: yaml: line too long (122 > 120 characters) (line-length)
+tasks/auditd.yml:21: yaml: line too long (122 > 120 characters) (yaml[line-length])
 tasks/compilers.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/cron.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/ctrlaltdel.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
@@ -43,8 +43,8 @@ tasks/logindefs.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/motdnews.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/mount.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/packagemgmt.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
-tasks/packagemgmt.yml:164: yaml: line too long (129 > 120 characters) (line-length)
-tasks/packagemgmt.yml:192: yaml: line too long (162 > 120 characters) (line-length)
+tasks/packagemgmt.yml:164: yaml: line too long (129 > 120 characters) (yaml[line-length])
+tasks/packagemgmt.yml:192: yaml: line too long (162 > 120 characters) (yaml[line-length])
 tasks/packages.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/password.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])
 tasks/path.yml:1: schema: 'yes' is not of type 'boolean' (schema[tasks])

test/test_with_skip_tagid.py

@@ -20,15 +20,15 @@ def test_negative_with_id() -> None:
     with_id = "yaml"
     bad_runner = Runner(FILE, rules=collection, tags=frozenset([with_id]))
     errs = bad_runner.run()
-    assert len(errs) > 0
+    assert len(errs) == 1
 
 
 def test_negative_with_tag() -> None:
     """Negative test with_tag."""
     with_tag = "trailing-spaces"
     bad_runner = Runner(FILE, rules=collection, tags=frozenset([with_tag]))
     errs = bad_runner.run()
-    assert len(errs) > 0
+    assert len(errs) == 1
 
 
 def test_positive_skip_id() -> None:
@@ -40,6 +40,6 @@ def test_positive_skip_id() -> None:
 
 def test_positive_skip_tag() -> None:
     """Positive test skip_tag."""
-    skip_tag = "trailing-spaces"
+    skip_tag = "yaml[trailing-spaces]"
     good_runner = Runner(FILE, rules=collection, skip_list=[skip_tag])
     assert [] == good_runner.run()