Skip to content

Commit

Permalink
var-spacing: fix multiline nested JSON false positive (#1672)
Browse files Browse the repository at this point in the history
* E206: Increase testing accuracy

VariableHasSpacesRule testing was limited to ensuring that the number of
error was matching an expected number. This could have potential
limitation where for example a commit prevents an real linting error
from being raised and in the same time raises an invalid one.

This commit aims at improving this rule's testing by ensuring that
errors are raised only on a controlled set of lines.

Related to: #1671

* E206: Add testing for false positive nested JSON

This commit adds 4 new tasks in example playbook used for
VariableHasSpacesRule testing.

One of these rule is a nested multiline JSON object which has its
opening and closing brackets spanned on different lines. And this rule
raises false positive error.

Related to #1671

* E206: Fix multiline nested JSON false positive

When using a nested JSON object spanned on multiple lines in a Jinja2
context ansible-lint raises a false positive E206 error.

This commit extends the regex in charge of excluding false positive JSON
objects so that it handles multiline matching.

Fixes #1671

Co-authored-by: Sorin Sbarnea <ssbarnea@redhat.com>
  • Loading branch information
simonkeyd and ssbarnea committed Jul 18, 2021
1 parent 790aeac commit 04d68a0
Show file tree
Hide file tree
Showing 2 changed files with 53 additions and 10 deletions.
24 changes: 23 additions & 1 deletion examples/playbooks/var-spacing.yml
@@ -1,4 +1,4 @@
# Should raise var-spacing at lines 25, 28, 31
# Should raise var-spacing at tasks line 23, 26, 29, 54, 65
- hosts: all
tasks:
- name: good variable format
Expand Down Expand Up @@ -46,3 +46,25 @@
case2 }}
debug:
var: cases

- name: Valid single line nested JSON false positive
debug:
msg: "{{ {'dummy_2': {'nested_dummy_1': 'value_1', 'nested_dummy_2': value_2}} | combine(dummy_1) }}"

- name: Invalid single line nested JSON
debug:
msg: "{{ {'dummy_2': {'nested_dummy_1': 'value_1', 'nested_dummy_2': value_2}} | combine(dummy_1)}}"

- name: Valid multiline nested JSON false positive
debug:
msg: >-
{{ {'dummy_2': {'nested_dummy_1': value_1,
'nested_dummy_2': value_2}} |
combine(dummy_1) }}
- name: Invalid multiline nested JSON
debug:
msg: >-
{{ {'dummy_2': {'nested_dummy_1': value_1,
'nested_dummy_2': value_2}} |
combine(dummy_1)}}
39 changes: 30 additions & 9 deletions src/ansiblelint/rules/VariableHasSpacesRule.py
Expand Up @@ -3,7 +3,7 @@

import re
import sys
from typing import Any, Dict, Optional, Union
from typing import Any, Dict, List, Optional, Union

from ansiblelint.file_utils import Lintable
from ansiblelint.rules import AnsibleLintRule
Expand All @@ -20,7 +20,7 @@ class VariableHasSpacesRule(AnsibleLintRule):
version_added = 'v4.0.0'

bracket_regex = re.compile(r"{{[^{\n' -]|[^ '\n}-]}}", re.MULTILINE | re.DOTALL)
exclude_json_re = re.compile(r"[^{]{'\w+': ?[^{]{.*?}}")
exclude_json_re = re.compile(r"[^{]{'\w+': ?[^{]{.*?}}", re.MULTILINE | re.DOTALL)

def matchtask(
self, task: Dict[str, Any], file: Optional[Lintable] = None
Expand All @@ -35,15 +35,36 @@ def matchtask(

if 'pytest' in sys.modules:

from ansiblelint.rules import RulesCollection
from ansiblelint.runner import Runner
import pytest

def test_var_spacing() -> None:
"""Verify rule."""
from ansiblelint.rules import RulesCollection # pylint: disable=ungrouped-imports
from ansiblelint.runner import Runner # pylint: disable=ungrouped-imports

@pytest.fixture
def error_expected_lines() -> List[int]:
"""Return list of expected error lines."""
return [23, 26, 29, 54, 65]

@pytest.fixture
def test_playbook() -> str:
"""Return test cases playbook path."""
return "examples/playbooks/var-spacing.yml"

@pytest.fixture
def lint_error_lines(test_playbook: str) -> List[int]:
"""Get VarHasSpacesRules linting results on test_playbook."""
collection = RulesCollection()
collection.register(VariableHasSpacesRule())

lintable = Lintable("examples/playbooks/var-spacing.yml")
lintable = Lintable(test_playbook)
results = Runner(lintable, rules=collection).run()
return list(map(lambda item: item.linenumber, results))

assert len(results) == 3
def test_var_spacing(
error_expected_lines: List[int], lint_error_lines: List[int]
) -> None:
"""Ensure that expected error lines are matching found linting error lines."""
# list unexpected error lines or non-matching error lines
error_lines_difference = list(
set(error_expected_lines).symmetric_difference(set(lint_error_lines))
)
assert len(error_lines_difference) == 0

0 comments on commit 04d68a0

Please sign in to comment.