jinja[spacing] false positive tracker for v6.5.1

[ssbarnea]

Summary

We know that even 6.5.1 produce some bad jinja formatting suggestions and we plan to remediate them as soon as possible. We will use this ticket to document them, so check the current list and if you have other examples, please minimize them first and add them in a comment.

If you are curious about currently covered cases, you can have a look at https://github.com/ansible/ansible-lint/blob/main/src/ansiblelint/rules/jinja.py#L396 where we have more than 40 examples of before and expected results.

Reproducing template

---
- name: Reproducer
  hosts: localhost
  tasks:
    - name: "Bad: {{ {a: foo[0:-1]} }}"
      ansible.builtin.debug:
        msg: foo

Known false positives

• Undesired space is added inside arrays {{ {a: foo[0:-1]} }} -> {{ {a: foo[0: -1]} }}, original was already correct

[max06]

As requested:

---

- name: Spacing Demo
  ansible.builtin.debug:
    msg: "{{ foo[bar | baz] }}"

-> Linter doesn't like the space after the pipe.

---

- name: Spacing Demo
  ansible.builtin.debug:
    msg: "{{ [{'foo': bar, 'baz': blah}] }}" # The array-[] confuses the linter

-> Linter wants to have spaces after colons removed, if it's in an array.

[ssbarnea]

That is because at this moment it does struggle to differentiate between a list and an array-slicing operation. I need to change implementation to clearly distinguish between them as the rules are quite different for one an another.

a_list = [a, b, c]
slicing[a:b:c]

If I am correct if there is an operator before [ I can assume a list, but if there is a string, name (variable) or ], it must be a slicing.

a[a][b] -- two slices one after another
"sss"[3] -- still a slice
+[a] -- array

I do have a method called in_expression() that currently returns the last open bracket, {, [ or (, but I guess that I need to make it return something else for a splice, as the rules are very different inside slices.

[SLoeuillet]

New false positives :;

jinja: Jinja2 spacing could be improved: {{ [directory.hosts + "/" + inventory_hostname] | product(group_names) | map("join", "/") | list }} -> {{ [directory.hosts +"/" +inventory_hostname] | product(group_names) | map("join", "/") | list }} (jinja[spacing])
inventories/group_vars/all.yml:41 Jinja2 template rewrite recommendation: `{{  | product(group_names) | map("join", "/") | list }}`.

jinja: Jinja2 spacing could be improved: {{ [kafka_servers | length, 3] | min }} -> {{ [kafka_servers |length, 3] | min }} (jinja[spacing])
roles/kafka/vars/main.yml:11 Jinja2 template rewrite recommendation: `{{  | min }}`.

jinja: Jinja2 spacing could be improved: ansible_facts['distribution'] | lower not in scylla_support or scylla_support[ansible_facts['distribution'] | lower][ansible_facts['distribution_major_version']] is not defined or ansible_facts['distribution_version'] is version(scylla_support[ansible_facts['distribution'] | lower][ansible_facts['distribution_major_version']]['minimum_version'],'<')
 -> ansible_facts['distribution'] | lower not in scylla_support or scylla_support[ansible_facts['distribution']|lower][ansible_facts['distribution_major_version']] is not defined or ansible_facts['distribution_version'] is version(scylla_support[ansible_facts['distribution'] | lower][ansible_facts['distribution_major_version']]['minimum_version'], '<')
 (jinja[spacing])
roles/scylla-node/tasks/upgrade/main.yml:107 Jinja2 template rewrite recommendation: `ansible_facts['distribution'] | lower not in scylla_support or scylla_support[ansible_facts['distribution']|lower][ansible_facts['distribution_major_version']] is not defined or ansible_facts['distribution_version'] is version(scylla_support[ansible_facts['distribution'] | lower][ansible_facts['distribution_major_version']]['minimum_version'], '<')
`.

jinja: Jinja2 spacing could be improved: scylla_detected['major_version'] not in scylla_support[ansible_facts['distribution'] | lower][ansible_facts['distribution_major_version']][scylla_detected['edition']] -> scylla_detected['major_version'] not in scylla_support[ansible_facts['distribution']|lower][ansible_facts['distribution_major_version']][scylla_detected['edition']] (jinja[spacing])
roles/scylla-node/tasks/upgrade/main.yml:115 Jinja2 template rewrite recommendation: `scylla_detected['major_version'] not in scylla_support[ansible_facts['distribution']|lower][ansible_facts['distribution_major_version']][scylla_detected['edition']]`.

jinja: Jinja2 spacing could be improved: {{ scylla_version if scylla_version != 'latest' else scylla_support[ansible_facts['distribution'] | lower][ansible_facts['distribution_major_version']][scylla_edition] | first }} -> {{ scylla_version if scylla_version != 'latest' else scylla_support[ansible_facts['distribution']|lower][ansible_facts['distribution_major_version']][scylla_edition] | first }} (jinja[spacing])
roles/scylla-node/tasks/upgrade/main.yml:123 Jinja2 template rewrite recommendation: `{{ scylla_version if scylla_version != 'latest' else scylla_support[ansible_facts['distribution']|lower][ansible_facts['distribution_major_version']] | first }}`.

jinja: Jinja2 spacing could be improved: {{ scylla_support[distro_name][distro_version][scylla_upgrade['edition']][next_available_upgrade_version_index | int:] | first }} -> {{ scylla_support[distro_name][distro_version][scylla_upgrade['edition']][next_available_upgrade_version_index |int:] | first }} (jinja[spacing])
roles/scylla-node/tasks/upgrade/main.yml:168 Jinja2 template rewrite recommendation: `{{ scylla_support[scylla_upgrade['edition']] | first }}`.

jinja: Jinja2 spacing could be improved: {{ scylla_support[distro_name][distro_version][scylla_upgrade['edition']][next_available_upgrade_version_index | int:] | first }} -> {{ scylla_support[distro_name][distro_version][scylla_upgrade['edition']][next_available_upgrade_version_index |int:] | first }} (jinja[spacing])
roles/scylla-node/tasks/upgrade/main.yml:224 Jinja2 template rewrite recommendation: `{{ scylla_support[scylla_upgrade['edition']] | first }}`.

jinja: Jinja2 spacing could be improved: node_drain_status.json.find("Drained") != -1 -> node_drain_status.json.find("Drained") != - 1 (jinja[spacing])
roles/scylla-node/tasks/upgrade/pre_upgrade.yml:94 Jinja2 template rewrite recommendation: `node_drain_status.json.find("Drained") != - 1`.

jinja: Jinja2 spacing could be improved: node_drain_status.json.find("Drained") != -1 -> node_drain_status.json.find("Drained") != - 1 (jinja[spacing])
roles/scylla-node/tasks/upgrade/pre_upgrade.yml:96 Jinja2 template rewrite recommendation: `node_drain_status.json.find("Drained") != - 1`.

[pgassmann]

suggesting |default() instead of correct | default()

WARNING  Listing 1 violation(s) that are fatal
jinja: Jinja2 spacing could be improved: {{ [zfs_extra_zfs_properties_default, item.extra_zfs_properties | default({})] | combine }} -> {{ [zfs_extra_zfs_properties_default, item.extra_zfs_properties |default({})] | combine }} (jinja[spacing])
roles/zfs/tasks/main.yml:46 Jinja2 template rewrite recommendation: `{{  | combine }}`.

[nkakouros]

This might be a ridiculous idea, but how about using black to format jinja templates? Black's format is anyway pointed to as the reference format for jinja formatting. Regarding dependencies, it seems that black only requires typed-ast and dataclasses that ansible-lint does not, so it would not add much to ansible-lint (I haven't looked into version requirements though). Of course, ansible-lint would lose the granularity of reporting exactly what is off (e.g. spacing), just that the line would be reformatted, unless black modules and functions could be used by ansible-lint to do specific checks (e.g. functions from this file.

[ssbarnea]

@nkakouros In fact you suggestion is not ridiculous at all. Reporting what was off is less important than getting it right. In the end, each human should be able to figure it out if we print both texts. I really like the idea and I trust black far more than my own makeshift code.

[isuftin]

Also got this.

jinja: Jinja2 spacing could be improved: (rundeck_startup_test.status == 403 or rundeck_startup_test.status == 200) and (rundeck_startup_test.content.find("Please wait while") == -1)
 -> (rundeck_startup_test.status == 403 or rundeck_startup_test.status == 200) and (rundeck_startup_test.content.find("Please wait while") == - 1)

... == - 1 would be incorrect

[ssbarnea]

@isuftin Yep, that bit with - 1 is called unary expression, one with only a single side. Is fixed locally but likely tomorrow I will be ready to finish it and make a new bugfix release.