Add rule violations from Yamllint

Also detect yamllint rule violations when this is found as
installed. This will allow us to remove a set of rules that
overlapped with general YAML syntax ones.

Fixes: #953

.ansible-lint

@@ -1,2 +1,9 @@
+extends: default
+rules:
+  document-start: disable
+  # 160 chars was the default used by old E204 rule, but
+  # you can easily change it or disable in your .yamllint file.
+  line-length:
+    max: 160
 exclude_paths:
 - .github/

.pre-commit-config.yaml

@@ -65,6 +65,7 @@ repos:
     args: []
     additional_dependencies:
     - Sphinx>=3.1.2
+    - yamllint
 - repo: https://github.com/pre-commit/mirrors-pylint
   rev: v2.6.0
   hooks:

.yamllint

@@ -1,4 +1,9 @@
 rules:
+  document-start: disable
   indentation:
     level: error
     indent-sequences: consistent
+ignore: |
+  .tox
+  examples/example.yml
+# ignore added because this file includes on-purpose errors

examples/example.yml

@@ -33,8 +33,8 @@
     - name: passing git as an argument to another task
       action: debug msg="{{item}}"
       with_items: 
-        - git
-        - bobbins
+      - git  # yamllint wrong indentation
+      - bobbins
 
     - name: yum latest
       yum: state=latest name=httpd

lib/ansiblelint/errors.py

@@ -1,7 +1,7 @@
 """Exceptions and error representations."""
 import functools
 import os
-from typing import Any, Optional, Type
+from typing import Any, Optional
 
 from ansiblelint._internal.rules import BaseRule, RuntimeErrorRule
 from ansiblelint.file_utils import normpath
@@ -29,13 +29,13 @@ def __init__(
             linenumber: int = 0,
             details: str = "",
             filename: Optional[str] = None,
-            rule: Type[BaseRule] = RuntimeErrorRule,
+            rule: BaseRule = RuntimeErrorRule(),
             tag: Optional[str] = None  # optional fine-graded tag
             ) -> None:
         """Initialize a MatchError instance."""
         super().__init__(message)
 
-        if rule is RuntimeErrorRule and not message:
+        if rule.__class__ is RuntimeErrorRule and not message:
             raise TypeError(
                 f'{self.__class__.__name__}() missing a '
                 "required argument: one of 'message' or 'rule'",
@@ -50,6 +50,8 @@ def __init__(
             self.filename = os.getcwd()
         self.rule = rule
         self.ignored = False  # 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
 
     def __repr__(self) -> str:

lib/ansiblelint/rules/YamllintRule.py

@@ -0,0 +1,82 @@
+import logging
+import os
+import sys
+from typing import TYPE_CHECKING, List
+
+from ansiblelint.rules import AnsibleLintRule
+
+if TYPE_CHECKING:
+    from ansiblelint.errors import MatchError
+
+_logger = logging.getLogger(__name__)
+
+# yamllint is a soft-dependency (not installed by default)
+try:
+    from yamllint.config import YamlLintConfig
+    from yamllint.linter import run as run_yamllint
+except ImportError:
+    pass
+
+
+YAMLLINT_CONFIG = """
+extends: default
+rules:
+  document-start: disable
+  # 160 chars was the default used by old E204 rule, but
+  # you can easily change it or disable in your .yamllint file.
+  line-length:
+    max: 160
+"""
+
+DESCRIPTION = """\
+Rule violations reported by YamlLint when this is installed.
+
+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):
+    id = 'YAML'
+    shortdesc = 'Violations reported by yamllint'
+    description = DESCRIPTION
+    severity = 'VERY_LOW'
+    tags = ['formatting', 'experimental', 'yamllint']
+    version_added = 'v5.0.0'
+    config = None
+    if "yamllint.config" in sys.modules:
+        config = YamlLintConfig(content=YAMLLINT_CONFIG)
+        # if we detect local yamllint config we use it but raise a warning
+        # as this is likely to get out of sync with our internal config.
+        for file in ['.yamllint', '.yamllint.yaml', '.yamllint.yml']:
+            if os.path.isfile(file):
+                _logger.warning(
+                    "Loading custom %s config file, this extends our "
+                    "internal yamllint config.",
+                    file)
+                config_override = YamlLintConfig(file=file)
+                config_override.extend(config)
+                break
+        _logger.warning("Effective yamllint rules used: %s", config.rules)
+
+    def __init__(self) -> None:
+        """Construct a rule instance."""
+        # customize id by adding the one reported by yamllint
+        self.id = self.__class__.id
+
+    def matchyaml(self, file, text: str) -> List["MatchError"]:
+        """Return matches found for a specific YAML text."""
+        matches = []
+        if YamllintRule.config:
+            for p in run_yamllint(text, YamllintRule.config):
+                matches.append(
+                    self.create_matcherror(
+                        message=p.desc,
+                        linenumber=p.line,
+                        details="",
+                        filename=file['path'],
+                        tag=p.rule))
+        return matches

lib/ansiblelint/rules/__init__.py

@@ -1,4 +1,5 @@
 """All internal ansible-lint rules."""
+import copy
 import glob
 import importlib.util
 import logging
@@ -42,7 +43,7 @@ def create_matcherror(
             linenumber=linenumber,
             details=details,
             filename=filename,
-            rule=self.__class__
+            rule=copy.copy(self)
             )
         if tag:
             match.tag = tag
@@ -250,7 +251,7 @@ def run(self, playbookfile, tags=set(), skip_list=frozenset()) -> List:
             return [MatchError(
                 message=str(error),
                 filename=playbookfile['path'],
-                rule=LoadingFailureRule)]
+                rule=LoadingFailureRule())]
 
         for rule in self.rules:
             if not tags or not set(rule.tags).union([rule.id]).isdisjoint(tags):
@@ -261,6 +262,10 @@ def run(self, playbookfile, tags=set(), skip_list=frozenset()) -> List:
                     matches.extend(rule.matchtasks(playbookfile, text))
                     matches.extend(rule.matchyaml(playbookfile, text))
 
+        # some rules can produce matches with tags that are inside our
+        # skip_list, so we need to cleanse the matches
+        matches = [m for m in matches if m.tag not in skip_list]
+
         return matches
 
     def __repr__(self) -> str:

lib/ansiblelint/runner.py

@@ -119,6 +119,6 @@ def _emit_matches(self, files: List) -> Generator[MatchError, None, None]:
                         self.playbooks.add((child['path'], child['type']))
                         files.append(child)
                 except MatchError as e:
-                    e.rule = LoadingFailureRule
+                    e.rule = LoadingFailureRule()
                     yield e
                 visited.add(arg)

lib/ansiblelint/testing/__init__.py

@@ -5,16 +5,13 @@
 import subprocess
 import sys
 import tempfile
-from typing import TYPE_CHECKING, Dict, List
+from typing import Dict, List
 
 from ansible import __version__ as ansible_version_str
 
+from ansiblelint.errors import MatchError
 from ansiblelint.runner import Runner
 
-if TYPE_CHECKING:
-    from ansiblelint.errors import MatchError
-
-
 ANSIBLE_MAJOR_VERSION = tuple(map(int, ansible_version_str.split('.')[:2]))
 
 
@@ -29,8 +26,10 @@ def _call_runner(self, path) -> List["MatchError"]:
         runner = Runner(self.collection, path)
         return runner.run()
 
-    def run_playbook(self, playbook_text):
+    def run_playbook(self, playbook_text: str) -> List[MatchError]:
         """Lints received text as a playbook."""
+        # remove newlines from beging and end of examples
+        playbook_text = playbook_text.lstrip("\n")
         with tempfile.NamedTemporaryFile(mode="w", suffix=".yml", prefix="playbook") as fp:
             fp.write(playbook_text)
             fp.flush()

lib/ansiblelint/utils.py

@@ -273,7 +273,7 @@ def _taskshandlers_children(basedir, k, v, parent_type: FileType) -> List:
     if v is None:
         raise MatchError(
             message="A malformed block was encountered while loading a block.",
-            rule=RuntimeErrorRule)
+            rule=RuntimeErrorRule())
     for th in v:
 
         # ignore empty tasks, `-`
@@ -458,7 +458,7 @@ def normalize_task_v2(task: dict) -> dict:  # noqa: C901
         action, arguments, result['delegate_to'] = mod_arg_parser.parse()
     except AnsibleParserError as e:
         raise MatchError(
-            rule=AnsibleParserErrorRule,
+            rule=AnsibleParserErrorRule(),
             message=e.message,
             filename=task.get(FILENAME_KEY, "Unknown"),
             linenumber=task.get(LINE_NUMBER_KEY, 0),

mypy.ini

@@ -35,3 +35,6 @@ ignore_missing_imports = True
 
 [mypy-sphinx_ansible_theme]
 ignore_missing_imports = True
+
+[mypy-yamllint.*]
+ignore_missing_imports = True

setup.cfg

@@ -95,6 +95,9 @@ core =
 # will install ansible from devel branch, may break at any moment.
 devel =
   ansible-core @ git+https://github.com/ansible/ansible.git
+# yamllint must remain optional
+yamllint =
+  yamllint >= 1.25.0  # GPL
 
 [options.packages.find]
 where = lib

test/TestExamples.py

@@ -5,7 +5,7 @@
 def test_example(default_rules_collection):
     """example.yml is expected to have 15 match errors inside."""
     result = Runner(default_rules_collection, 'examples/example.yml', [], [], []).run()
-    assert len(result) == 15
+    assert len(result) == 16
 
 
 def test_example_plain_string(default_rules_collection):

test/TestLineTooLong.py

@@ -2,10 +2,10 @@
 import unittest
 
 from ansiblelint.rules import RulesCollection
-from ansiblelint.rules.LineTooLongRule import LineTooLongRule
+from ansiblelint.rules.YamllintRule import YamllintRule
 from ansiblelint.testing import RunFromText
 
-LONG_LINE = '''
+LONG_LINE = '''\
 - name: task example
   debug:
     msg: 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua tempor incididunt ut labore et dolore'
@@ -14,7 +14,7 @@
 
 class TestLineTooLongRule(unittest.TestCase):
     collection = RulesCollection()
-    collection.register(LineTooLongRule())
+    collection.register(YamllintRule())
 
     def setUp(self):
         self.runner = RunFromText(self.collection)

test/TestMatchError.py

@@ -76,9 +76,9 @@ def test_matcherror_invalid():
         # filenames takes priority in sorting
         (MatchError("a", filename="b"), MatchError("a", filename="a")),
         # rule id 501 > rule id 101
-        (MatchError(rule=BecomeUserWithoutBecomeRule), MatchError(rule=AlwaysRunRule)),
+        (MatchError(rule=BecomeUserWithoutBecomeRule()), MatchError(rule=AlwaysRunRule())),
         # rule id "200" > rule id 101
-        (MatchError(rule=AnsibleLintRuleWithStringId), MatchError(rule=AlwaysRunRule)),
+        (MatchError(rule=AnsibleLintRuleWithStringId()), MatchError(rule=AlwaysRunRule())),
         # details are taken into account
         (MatchError("a", details="foo"), MatchError("a", details="bar")),
     ))

test/TestMissingFilePermissionsRule.py

@@ -105,10 +105,10 @@ def test_fail(rule_runner):
     """Validate that missing mode triggers the rule."""
     results = rule_runner.run_playbook(FAIL_TASKS)
     assert len(results) == 5
-    assert results[0].linenumber == 5
-    assert results[1].linenumber == 9
-    assert results[2].linenumber == 13
-    # assert results[3].linenumber == 17
-    assert results[3].linenumber == 21
-    assert results[4].linenumber == 26
-    # assert results[6].linenumber == 30
+    assert results[0].linenumber == 4
+    assert results[1].linenumber == 8
+    assert results[2].linenumber == 12
+    # assert results[3].linenumber == 16
+    assert results[3].linenumber == 20
+    assert results[4].linenumber == 25
+    # assert results[6].linenumber == 29