Skip to content

Commit

Permalink
Add rule violations from Yamllint
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ssbarnea committed Dec 30, 2020
1 parent d257716 commit 082b510
Show file tree
Hide file tree
Showing 17 changed files with 162 additions and 101 deletions.
7 changes: 7 additions & 0 deletions .ansible-lint
Original file line number Diff line number Diff line change
@@ -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/
1 change: 1 addition & 0 deletions .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ repos:
args: []
additional_dependencies:
- Sphinx>=3.1.2
- yamllint
- repo: https://github.com/pre-commit/mirrors-pylint
rev: v2.6.0
hooks:
Expand Down
5 changes: 5 additions & 0 deletions .yamllint
Original file line number Diff line number Diff line change
@@ -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
4 changes: 2 additions & 2 deletions examples/example.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 0 additions & 19 deletions lib/ansiblelint/rules/LineTooLongRule.py

This file was deleted.

34 changes: 0 additions & 34 deletions lib/ansiblelint/rules/TrailingWhitespaceRule.py

This file was deleted.

94 changes: 94 additions & 0 deletions lib/ansiblelint/rules/YamllintRule.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
import logging
import os
import sys
from typing import TYPE_CHECKING, List

from ansiblelint.rules import AnsibleLintRule
from ansiblelint.skip_utils import get_rule_skips_from_line

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.debug("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 = []
filtered_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))

if matches:
lines = text.splitlines()
for match in matches:
# rule.linenumber starts with 1, not zero
skip_list = get_rule_skips_from_line(lines[match.linenumber-1])
# print(skip_list)
if match.rule.id not in skip_list and match.tag not in skip_list:
filtered_matches.append(match)
return filtered_matches
3 changes: 3 additions & 0 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,6 @@ ignore_missing_imports = True

[mypy-sphinx_ansible_theme]
ignore_missing_imports = True

[mypy-yamllint.*]
ignore_missing_imports = True
3 changes: 3 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -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
2 changes: 1 addition & 1 deletion test/TestAnsibleSyntax.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
when encountering what counts as valid Ansible syntax.
"""

PB_WITH_NULL_TASKS = '''
PB_WITH_NULL_TASKS = '''\
- hosts: all
tasks:
'''
Expand Down
2 changes: 1 addition & 1 deletion test/TestExamples.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down
6 changes: 3 additions & 3 deletions test/TestLineTooLong.py
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand All @@ -14,7 +14,7 @@

class TestLineTooLongRule(unittest.TestCase):
collection = RulesCollection()
collection.register(LineTooLongRule())
collection.register(YamllintRule())

def setUp(self):
self.runner = RunFromText(self.collection)
Expand Down
18 changes: 9 additions & 9 deletions test/TestMissingFilePermissionsRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@

from ansiblelint.rules.MissingFilePermissionsRule import MissingFilePermissionsRule

SUCCESS_TASKS = '''
SUCCESS_TASKS = '''\
---
- hosts: hosts
tasks:
Expand Down Expand Up @@ -56,7 +56,7 @@
recurse: yes
'''

FAIL_TASKS = '''
FAIL_TASKS = '''\
---
- hosts: hosts
tasks:
Expand Down Expand Up @@ -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
6 changes: 3 additions & 3 deletions test/TestSkipInsideYaml.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,13 +49,13 @@
become_user: alice
action: git
- name: test 204 and 206
- name: test YAML and 206
get_url:
url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf
dest: "{{dest_proj_path}}/foo.conf"
- name: test 204 and 206 (skipped)
- name: test YAML and 206 (skipped)
get_url:
url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa 204
url: http://example.com/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/really_long_path/file.conf # noqa YAML
dest: "{{dest_proj_path}}/foo.conf" # noqa 206
- name: test 302
Expand Down
50 changes: 25 additions & 25 deletions test/TestSkipPlaybookItems.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import pytest

PLAYBOOK_PRE_TASKS = '''
PLAYBOOK_PRE_TASKS = '''\
- hosts: all
tasks:
- name: bad git 1 # noqa 401
Expand All @@ -14,7 +14,7 @@
action: git a=b c=d
'''

PLAYBOOK_POST_TASKS = '''
PLAYBOOK_POST_TASKS = '''\
- hosts: all
tasks:
- name: bad git 1 # noqa 401
Expand All @@ -28,7 +28,7 @@
action: git a=b c=d
'''

PLAYBOOK_HANDLERS = '''
PLAYBOOK_HANDLERS = '''\
- hosts: all
tasks:
- name: bad git 1 # noqa 401
Expand All @@ -42,7 +42,7 @@
action: git a=b c=d
'''

PLAYBOOK_TWO_PLAYS = '''
PLAYBOOK_TWO_PLAYS = '''\
- hosts: all
tasks:
- name: bad git 1 # noqa 401
Expand All @@ -58,29 +58,29 @@
action: git a=b c=d
'''

PLAYBOOK_WITH_BLOCK = '''
PLAYBOOK_WITH_BLOCK = '''\
- hosts: all
tasks:
- name: bad git 1 # noqa 401
action: git a=b c=d
- name: bad git 2
action: git a=b c=d
- name: Block with rescue and always section
block:
- name: bad git 3 # noqa 401
action: git a=b c=d
- name: bad git 4
action: git a=b c=d
rescue:
- name: bad git 5 # noqa 401
action: git a=b c=d
- name: bad git 6
action: git a=b c=d
always:
- name: bad git 7 # noqa 401
action: git a=b c=d
- name: bad git 8
action: git a=b c=d
- name: bad git 1 # noqa 401
action: git a=b c=d
- name: bad git 2
action: git a=b c=d
- name: Block with rescue and always section
block:
- name: bad git 3 # noqa 401
action: git a=b c=d
- name: bad git 4
action: git a=b c=d
rescue:
- name: bad git 5 # noqa 401
action: git a=b c=d
- name: bad git 6
action: git a=b c=d
always:
- name: bad git 7 # noqa 401
action: git a=b c=d
- name: bad git 8
action: git a=b c=d
'''


Expand Down

0 comments on commit 082b510

Please sign in to comment.