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

.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,8 @@
 rules:
+  document-start: disable
   indentation:
     level: error
     indent-sequences: consistent
+# ignore added because this file includes on-purpose errors
+ignore: |
+  examples/example.yml

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,12 +29,12 @@ def __init__(
             linenumber: int = 0,
             details: str = "",
             filename: Optional[str] = None,
-            rule: Type[BaseRule] = RuntimeErrorRule
+            rule: BaseRule = RuntimeErrorRule()
             ) -> 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'",

lib/ansiblelint/rules/YamllintRule.py

@@ -0,0 +1,69 @@
+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
+  line-length: disable
+"""
+
+
+class YamllintRule(AnsibleLintRule):
+    id = 'yamllint'
+    shortdesc = 'Reported by yamllint'
+    description = 'Rule violations reported by YamlLint when this is available.'
+    severity = 'VERY_LOW'
+    tags = ['formatting', 'experimental']
+    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.debug("Effective yamllint rules used: %s", config.rules)
+
+    def __init__(self):
+        """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'],
+                        id=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
@@ -35,14 +36,18 @@ def create_matcherror(
             message: str = None,
             linenumber: int = 0,
             details: str = "",
-            filename: str = None) -> MatchError:
-        return MatchError(
+            filename: str = None,
+            id: str = "") -> MatchError:
+        match = MatchError(
             message=message,
             linenumber=linenumber,
             details=details,
             filename=filename,
-            rule=self.__class__
+            rule=copy.copy(self)
             )
+        if id:
+            match.rule.id += "-" + id
+        return match
 
     def matchlines(self, file, text) -> List[MatchError]:
         matches: List[MatchError] = []
@@ -246,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):

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) == 18
 
 
 def test_example_plain_string(default_rules_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

test/TestNestedJinjaRule.py

@@ -24,15 +24,15 @@
 
 from ansiblelint.file_utils import Lintable
 
-FAIL_TASK_1LN = Lintable('playbook.yml', '''
+FAIL_TASK_1LN = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: one-level nesting
       set_fact:
         var_one: "2*(1+2) is {{ 2 * {{ 1 + 2 }} }}"
 ''')
 
-FAIL_TASK_1LN_M = Lintable('playbook.yml', '''
+FAIL_TASK_1LN_M = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: one-level multiline nesting
@@ -43,15 +43,15 @@
           }}
 ''')
 
-FAIL_TASK_2LN = Lintable('playbook.yml', '''
+FAIL_TASK_2LN = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: two-level nesting
       set_fact:
         var_two: "2*(1+(3-1)) is {{ 2 * {{ 1 + {{ 3 - 1 }} }} }}"
 ''')
 
-FAIL_TASK_2LN_M = Lintable('playbook.yml', '''
+FAIL_TASK_2LN_M = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: two-level multiline nesting
@@ -63,15 +63,15 @@
           }} }}
 ''')
 
-FAIL_TASK_W_5LN = Lintable('playbook.yml', '''
+FAIL_TASK_W_5LN = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: five-level wild nesting
       set_fact:
         var_three_wld: "{{ {{ {{ {{ {{ 234 }} }} }} }} }}"
 ''')
 
-FAIL_TASK_W_5LN_M = Lintable('playbook.yml', '''
+FAIL_TASK_W_5LN_M = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: five-level wild multiline nesting
@@ -88,15 +88,15 @@
           }}
 ''')
 
-SUCCESS_TASK_P = Lintable('playbook.yml', '''
+SUCCESS_TASK_P = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: non-nested example
       set_fact:
         var_one: "number for 'one' is {{ 2 * 1 }}"
 ''')
 
-SUCCESS_TASK_P_M = Lintable('playbook.yml', '''
+SUCCESS_TASK_P_M = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: multiline non-nested example
@@ -106,15 +106,15 @@
           2 * 1 }}
 ''')
 
-SUCCESS_TASK_2P = Lintable('playbook.yml', '''
+SUCCESS_TASK_2P = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: nesting far from each other
       set_fact:
         var_two: "number for 'two' is {{ 2 * 1 }} and number for 'three' is {{ 4 - 1 }}"
 ''')
 
-SUCCESS_TASK_2P_M = Lintable('playbook.yml', '''
+SUCCESS_TASK_2P_M = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: multiline nesting far from each other
@@ -125,15 +125,15 @@
           4 - 1 }}
 ''')
 
-SUCCESS_TASK_C_2P = Lintable('playbook.yml', '''
+SUCCESS_TASK_C_2P = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: nesting close to each other
       set_fact:
         var_three: "number for 'ten' is {{ 2 - 1 }}{{ 3 - 3 }}"
 ''')
 
-SUCCESS_TASK_C_2P_M = Lintable('playbook.yml', '''
+SUCCESS_TASK_C_2P_M = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: multiline nesting close to each other
@@ -144,7 +144,7 @@
           }}{{ 3 - 3 }}
 ''')
 
-SUCCESS_TASK_PRINT = Lintable('playbook.yml', '''
+SUCCESS_TASK_PRINT = Lintable('playbook.yml', '''\
 - hosts: all
   tasks:
     - name: print curly braces

test/TestRunner.py

@@ -30,7 +30,7 @@
 
 @pytest.mark.parametrize(('playbook', 'exclude', 'length'), (
     ('test/nomatchestest.yml', [], 0),
-    ('test/unicode.yml', [], 1),
+    ('test/unicode.yml', [], 3),
     (LOTS_OF_WARNINGS_PLAYBOOK, [LOTS_OF_WARNINGS_PLAYBOOK], 0),
     ('test/block.yml', [], 1),
     ('test/block-null.yml', [], 1),