Skip to content

Commit

Permalink
Avoid same false positives with no-tabs rule (#1373)
Browse files Browse the repository at this point in the history
As we identified some particular cases where use of tabs is perfectly
justified, we add exceptions for these.

Fixes: #1334
  • Loading branch information
ssbarnea committed Feb 18, 2021
1 parent 0598777 commit 1dd1df6
Show file tree
Hide file tree
Showing 6 changed files with 62 additions and 18 deletions.
10 changes: 10 additions & 0 deletions examples/playbooks/rule-no-tabs.yml
@@ -0,0 +1,10 @@
- hosts: localhost
tasks:
- name: should not trigger no-tabs rules
lineinfile:
path: some.txt
regexp: '^\t$'
line: 'string with \t inside'
- name: foo
debug:
msg: "Presence of \t should trigger no-tabs here."
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/ComparisonToLiteralBoolRule.py
Expand Up @@ -22,7 +22,7 @@ class ComparisonToLiteralBoolRule(AnsibleLintRule):
literal_bool_compare = re.compile("[=!]= ?(True|true|False|false)")

def matchtask(self, task: Dict[str, Any]) -> Union[bool, str]:
for k, v in nested_items(task):
for k, v, _ in nested_items(task):
if k == 'when':
if isinstance(v, str):
if self.literal_bool_compare.search(v):
Expand Down
38 changes: 36 additions & 2 deletions src/ansiblelint/rules/NoTabsRule.py
@@ -1,5 +1,6 @@
# Copyright (c) 2016, Will Thames and contributors
# Copyright (c) 2018, Ansible Project
import sys
from typing import Any, Dict, Union

from ansiblelint.rules import AnsibleLintRule
Expand All @@ -13,11 +14,44 @@ class NoTabsRule(AnsibleLintRule):
severity = 'LOW'
tags = ['formatting']
version_added = 'v4.0.0'
allow_list = [
("lineinfile", "insertafter"),
("lineinfile", "insertbefore"),
("lineinfile", "regexp"),
("lineinfile", "line"),
]

def matchtask(self, task: Dict[str, Any]) -> Union[bool, str]:
for k, v in nested_items(task):
for k, v, parent in nested_items(task):
if isinstance(k, str) and '\t' in k:
return True
if isinstance(v, str) and '\t' in v:
if (parent, k) not in self.allow_list and isinstance(v, str) and '\t' in v:
return True
return False


RULE_EXAMPLE = r'''---
- hosts: localhost
tasks:
- name: should not trigger no-tabs rules
lineinfile:
path: some.txt
regexp: '^\t$'
line: 'string with \t inside'
- name: foo
debug:
msg: "Presence of \t should trigger no-tabs here."
'''

# testing code to be loaded only with pytest or when executed the rule file
if "pytest" in sys.modules:

import pytest

@pytest.mark.parametrize('rule_runner', (NoTabsRule,), indirect=['rule_runner'])
def test_no_tabs_rule(rule_runner: "Any") -> None:
"""Test rule matches."""
results = rule_runner.run_playbook(RULE_EXAMPLE)
assert results[0].linenumber == 9
assert results[0].message == NoTabsRule.shortdesc
assert len(results) == 1
2 changes: 1 addition & 1 deletion src/ansiblelint/rules/VariableHasSpacesRule.py
Expand Up @@ -22,7 +22,7 @@ class VariableHasSpacesRule(AnsibleLintRule):
exclude_json_re = re.compile(r"[^{]{'\w+': ?[^{]{.*?}}")

def matchtask(self, task: Dict[str, Any]) -> Union[bool, str]:
for k, v in nested_items(task):
for k, v, _ in nested_items(task):
if isinstance(v, str):
cleaned = self.exclude_json_re.sub("", v)
if bool(self.bracket_regex.search(cleaned)):
Expand Down
16 changes: 8 additions & 8 deletions src/ansiblelint/utils.py
Expand Up @@ -855,16 +855,16 @@ def convert_to_boolean(value: Any) -> bool:


def nested_items(
data: Union[Dict[Any, Any], List[Any]]
) -> Generator[Tuple[Any, Any], None, None]:
data: Union[Dict[Any, Any], List[Any]], parent: str = ""
) -> Generator[Tuple[Any, Any, str], None, None]:
"""Iterate a nested data structure."""
if isinstance(data, dict):
for k, v in data.items():
yield k, v
for k, v in nested_items(v):
yield k, v
yield k, v, parent
for k, v, p in nested_items(v, k):
yield k, v, p
if isinstance(data, list):
for item in data:
yield "list-item", item
for k, v in nested_items(item):
yield k, v
yield "list-item", item, parent
for k, v, p in nested_items(item):
yield k, v, p
12 changes: 6 additions & 6 deletions test/TestUtils.py
Expand Up @@ -435,11 +435,11 @@ def test_nested_items():
data = {"foo": "text", "bar": {"some": "text2"}, "fruits": ["apple", "orange"]}

items = [
("foo", "text"),
("bar", {"some": "text2"}),
("some", "text2"),
("fruits", ["apple", "orange"]),
("list-item", "apple"),
("list-item", "orange"),
("foo", "text", ""),
("bar", {"some": "text2"}, ""),
("some", "text2", "bar"),
("fruits", ["apple", "orange"], ""),
("list-item", "apple", "fruits"),
("list-item", "orange", "fruits"),
]
assert list(utils.nested_items(data)) == items

0 comments on commit 1dd1df6

Please sign in to comment.