Skip to content

Commit

Permalink
Reimplement jinja[spacing] to avoid use of regex
Browse files Browse the repository at this point in the history
This change rewrites jinja2 rule to make it use jinaja lex parser and
avoid use of regexes for parsing, as they cause too many false
positives.

Fixes: ansible#2301 ansible#2285 ansible#2241 ansible#2209 ansible#2208 ansible#2120
  • Loading branch information
ssbarnea committed Aug 18, 2022
1 parent 382a15a commit 7c6879c
Show file tree
Hide file tree
Showing 5 changed files with 173 additions and 111 deletions.
7 changes: 4 additions & 3 deletions examples/playbooks/jinja-spacing.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
---
# Should raise jinja[spacing] at tasks line 23, 26, 29, 54, 65
- hosts: all
- name: Fixture for testing jinja2[spacing]
hosts: all
tasks:
- name: Good variable format
ansible.builtin.debug:
Expand Down Expand Up @@ -56,10 +57,10 @@
vars:
cases:
case1: >-
http://example.com/{{
http://foo.com/{{
case1 }}
case2: >-
http://example.com/{{
http://bar.com/{{
case2 }}
ansible.builtin.debug:
var: cases
Expand Down
18 changes: 9 additions & 9 deletions examples/playbooks/vars/jinja-spacing.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
---
# Should raise jinja[spacing] at line [12, 13, 14, 27, 33], at following variables.
# Should raise jinja[spacing] at line [14, 15, 16, 17, 18, 19, 22, 32, 38], at following variables.
# ".bad_var_1", ".bad_var_2", ".bad_var_3", ".invalid_multiline_nested_json", ".invalid_nested_json"
good_var_1: "{{ good_format }}"
good_var_2: "Value: {{ good_format }}"
Expand All @@ -11,12 +11,12 @@ jinja_whitespace_control: |
{{ good_format }}/
{{- good_format }}
{{- good_format -}}
bad_var_1: "{{bad_format}}"
bad_var_2: "Value: {{ bad_format}}"
bad_var_3: "{{bad_format }}"
bad_var_4: "{{ bad_format|filter }}"
bad_var_5: "Value: {{ bad_format |filter }}"
bad_var_6: "{{ bad_format| filter }}"
bad_var_1: "{{bad_format}}" # <-- 1
bad_var_2: "Value: {{ bad_format}}" # <-- 2
bad_var_3: "{{bad_format }}" # <-- 3
bad_var_4: "{{ bad_format|filter }}" # <-- 4
bad_var_5: "Value: {{ bad_format |filter }}" # <-- 5
bad_var_6: "{{ bad_format| filter }}" # <-- 6
# spell-checker: disable-next-line
non_jinja_var: "data = ${lookup{$local_part}lsearch{/etc/aliases}}" # noqa: jinja[spacing]
json_inside_jinja: "{{ {'test': {'subtest': variable}} }}"
Expand All @@ -29,13 +29,13 @@ multiline_vars: # Assert that no false positive on multiline exists
http://example.com/{{
case2 }}
valid_nested_json: "{{ {'dummy_2': {'nested_dummy_1': 'value_1', 'nested_dummy_2': value_2}} | combine(dummy_1) }}"
invalid_nested_json: "{{ {'dummy_2': {'nested_dummy_1': 'value_1', 'nested_dummy_2': value_2}} | combine(dummy_1)}}"
invalid_nested_json: "{{ {'dummy_2': {'nested_dummy_1': 'value_1', 'nested_dummy_2': value_2}} | combine(dummy_1)}}" # <-- 7

valid_multiline_nested_json: >-
{{ {'dummy_2': {'nested_dummy_1': value_1,
'nested_dummy_2': value_2}} |
combine(dummy_1) }}
invalid_multiline_nested_json: >-
invalid_multiline_nested_json: >- # <-- 8
{{ {'dummy_2': {'nested_dummy_1': value_1,
'nested_dummy_2': value_2}} |
combine(dummy_1)}}
9 changes: 6 additions & 3 deletions src/ansiblelint/rules/command_instead_of_shell.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,11 +40,11 @@
changed_when: false
- name: Shell with jinja filter
ansible.builtin.shell: echo {{ "hello"|upper }}
ansible.builtin.shell: echo {{ "hello" | upper }}
changed_when: false
- name: Sshell with jinja filter (fqcn)
ansible.builtin.shell: echo {{ "hello"|upper }}
ansible.builtin.shell: echo {{ "hello" | upper }}
changed_when: false
"""

Expand Down Expand Up @@ -143,7 +143,10 @@ def matchtask(

from ansiblelint.testing import RunFromText # pylint: disable=ungrouped-imports

@pytest.mark.parametrize(("text", "expected"), ((SUCCESS_PLAY, 0), (FAIL_PLAY, 3)))
@pytest.mark.parametrize(
("text", "expected"),
(pytest.param(SUCCESS_PLAY, 0, id="0"), pytest.param(FAIL_PLAY, 3, id="1")),
)
def test_rule_command_instead_of_shell(
default_text_runner: RunFromText, text: str, expected: int
) -> None:
Expand Down

0 comments on commit 7c6879c

Please sign in to comment.