Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reimplement jinja[spacing] to avoid use of regex #2306

Merged
merged 1 commit into from
Aug 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions .config/dictionary.txt
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,10 @@ unignored
unimported
unindented
unjinja
unlex
unnormalized
unskippable
unspaced
unsubscriptable
untemplated
uwsgi
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 660
PYTEST_REQPASS: 693

steps:
- name: Activate WSL1
Expand Down
9 changes: 5 additions & 4 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 All @@ -18,7 +19,7 @@
ansible.builtin.debug:
msg: "{{ '{{' }}"
- name: Jinja escaping allowed
# noqa: 306
# noqa: risky-shell-pipe
ansible.builtin.shell: docker info --format '{{ '{{' }}json .Swarm.LocalNodeState{{ '}}' }}' | tr -d '"'
changed_when: false
- name: Jinja whitespace control allowed
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)}}
20 changes: 16 additions & 4 deletions src/ansiblelint/rules/jinja.md
Original file line number Diff line number Diff line change
@@ -1,20 +1,32 @@
## jinja

This rule can report problems related to jinja2 string templates. The current
version can report `jinja[spacing]` when there are no spaces between variables
and operators, including filters, like `{{ var_name | filter }}`. This
improves readability and makes it less likely to introduce typos.
version can report:

- `jinja[spacing]` when there are no spaces between variables
and operators, including filters, like `{{ var_name | filter }}`. This
improves readability and makes it less likely to introduce typos.
- `jinja[invalid]` when the jinja2 template is invalid

As jinja2 syntax is closely following Python one we aim to follow
[black](https://black.readthedocs.io/en/stable/) formatting rules. If you are
curious how black would reformat a small sniped feel free to visit
[online black formatter](https://black.vercel.app/) site. Keep in mind to not
include the entire jinja2 template, so instead of `{{ 1+2==3 }}`, do paste
only `1+2==3`.

### Problematic code

```yaml
---
foo: "{{some|dict2items}}"
foo: "{{some|dict2items}}" # <-- jinja[spacing]
bar: "{{ & }}" # <-- jinja[invalid]
```

### Correct code

```yaml
---
foo: "{{ some | dict2items }}"
bar: "{{ '&' }}"
```